fix: onhost supervisor reloading test#2681
Conversation
vjripoll
changed the title
vjripoll
force-pushed
the
fix-supervisor-reloading-test
branch
3 times, most recently
from
b7ff10e to
bed38b5
Compare
vjripoll
changed the title
sigilioso
left a comment
sigilioso
left a comment
There was a problem hiding this comment.
Two minor questions. Thanks!
|
|
||
| /// Serializes write+exec tests to avoid ETXTBSY (a concurrent fork inheriting the writable fd of a | ||
| /// binary being written makes its exec fail with "Text file busy"). | ||
| static EXEC_SERIAL_LOCK: Mutex<()> = Mutex::new(()); |
There was a problem hiding this comment.
I think we use serial_test in other tests that can be problematic when executed in parallel. Does the Mutex approach cover something that using the #[serial] macro doesn't?
There was a problem hiding this comment.
You're right, I didn't even remember the serial_test. I'll change it and add the dependency. Thank you!
| std::thread::sleep(Duration::from_secs(5)); | ||
|
|
||
| dirs |
There was a problem hiding this comment.
I'd like to confirm that I'm understanding the issue and the fix.
Before, we had a 5s. sleep keep the AC proccess running to write the logs and, sometimes, the expected logs weren't written on time and that made the test fail. Now, we keep the AC process running for as long as the test runs and we rely on the check's retries to give some time to the process to write the files.
If that's right, would it make sense to remove this 5s. sleep and just adjust the retries in the corresponding test(s)?
There was a problem hiding this comment.
Right on both counts. Removed the post-reload sleep and let the retry (up to 60s, with AC now kept alive) wait for the logs. Kept the pre-reload sleep, since it ensures the first run's log lands before the reload can disable logging.
vjripoll
force-pushed
the
fix-supervisor-reloading-test
branch
from
bed38b5 to
4b43837
Compare
|
👍 |
sigilioso
left a comment
sigilioso
left a comment
There was a problem hiding this comment.
I've left two nits, feel free to ignore them
vjripoll
force-pushed
the
fix-supervisor-reloading-test
branch
from
4b43837 to
fe8ca69
Compare
3 participants
ISSUE
File logging reload: run_file_logging_scenario dropped the StartedAgentControl handle on return, so Agent Control stopped before the test's final retry. That retry could never wait for the post-reload log to appear (AC was already gone), and the only window for the second-run log was a fixed 5s sleep, which slow CI exceeded.
Self-replacer: the workspace tests run in parallel. While one test writes a binary (fs::copy/fs::write holding a writable fd), another test fork (to spawn its process) inherits that fd, so executing the just-written binary intermittently fails with ETXTBSY ("Text file busy").
FIX
File logging: the scenario now returns the StartedAgentControl handle and the test keeps it alive, so AC stays running while the test waits. The final retry now waits for the full expected log state (both messages present/absent as expected, up to 60s) instead of checking once after AC had stopped.
Self-replacer: serialize the write+exec tests behind a shared lock so only one write+exec sequence runs at a time, and build the example binary at most once (Once) to remove the concurrent-build.