sigusr2 autest: simplify Process and Ready objects#13021
sigusr2 autest: simplify Process and Ready objects#13021bneradt wants to merge 1 commit intoapache:masterfrom
Conversation
Rewrite the sigusr2 autest to drive each scenario from a single shel helper script instead of a chain of AuTest Processes. This removes the Ready-condition races from the old test, keeps the signal and log rotation ordering in one place, and makes the log file expectations easier to follow. This should address flakiness in the test that has been seen in CI.
There was a problem hiding this comment.
Pull request overview
This PR refactors the sigusr2 gold test to reduce flakiness by consolidating scenario orchestration into dedicated shell helper scripts instead of chaining multiple AuTest Process objects with Ready conditions.
Changes:
- Added
sigusr2_rotate_diags.shto perform diags.log rotation orchestration (wait → rename → SIGUSR2 → verify). - Added
sigusr2_rotate_custom_log.shto drive configured access log rotation via requests + SIGUSR2 and verify log placement. - Simplified
sigusr2.test.pyto use one default process per scenario that invokes the helper scripts, and moved log assertions into clearer per-scenario methods.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/gold_tests/logging/sigusr2_rotate_diags.sh |
New helper script that deterministically sequences diags.log rename + SIGUSR2 + content checks. |
tests/gold_tests/logging/sigusr2_rotate_custom_log.sh |
New helper script that drives /first→rotate→/second→SIGUSR2→/third and waits for expected log writes. |
tests/gold_tests/logging/sigusr2.test.py |
Reworked test to call the helper scripts (single-process orchestration per run) and to simplify ATS/server setup and assertions. |
bryancall
left a comment
There was a problem hiding this comment.
The shell-script approach is a big improvement over the old Process chain. Much easier to follow the sequencing, and the explicit polling loops with 60-second timeouts should handle the ASAN/CI slowness that was causing the flakiness.
Two things I noticed:
-
Missing
StillRunningAfter-- The old test hadtr1.StillRunningAfter = diags_test.ts(and the server) to verify ATS survived the signal. Neitheradd_system_log_testnoradd_configured_log_testsets this. Without it, if ATS crashes during SIGUSR2 handling, the test could still pass as long as the shell script exits 0 (the shell script'swait_for_containswould succeed on files written before the crash). Might be worth adding back. -
Typo in commit message -- "shel helper script" should be "shell helper script" (appears twice).
Rewrite the sigusr2 autest to drive each scenario from a single
shel helper script instead of a chain of AuTest Processes.
This removes the Ready-condition races from the old test, keeps the
signal and log rotation ordering in one place, and makes the log file
expectations easier to follow. This should address flakiness in the
test that has been seen in CI.