[autest] thread_config: fix race and skip on macOS#12940
[autest] thread_config: fix race and skip on macOS#12940moonchen wants to merge 1 commit intoapache:masterfrom
Conversation
check_threads.py now uses a short bounded poll/retry window so thread-count validation doesn’t fail on startup races; the test is also skipped on macOS because per-thread introspection used by this check is not reliably available there.
There was a problem hiding this comment.
Pull request overview
This PR updates the thread_config gold test to be more reliable during ATS startup by adding a bounded poll/retry window for thread-count validation, and it restricts the test to platforms where per-thread introspection is supported.
Changes:
- Add a bounded poll/retry loop in
check_threads.pyto avoid thread-count validation failing due to startup races. - Skip the
thread_configgold test on non-Linux platforms viaSkipUnless(IsPlatform("linux")).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/gold_tests/thread_config/thread_config.test.py | Skips the thread configuration test unless running on Linux. |
| tests/gold_tests/thread_config/check_threads.py | Adds retry-based thread counting and expands ATS process matching logic for more robust detection. |
| try: | ||
| threads = p.threads() | ||
| except (psutil.NoSuchProcess, psutil.AccessDenied, psutil.ZombieProcess): | ||
| return 1, 'Could not inspect ATS process threads.' |
There was a problem hiding this comment.
p.threads() failure returns exit code 1 ("Could not inspect ATS process threads."), but count_threads() treats code 1 as a retryable "process not found yet" condition. This can hide a real permission/psutil error for up to the full wait window and makes the exit code ambiguous. Consider using a distinct non-retry exit code for thread-inspection failures (or exclude this case from retry_codes).
| return 1, 'Could not inspect ATS process threads.' | |
| return 12, 'Could not inspect ATS process threads.' |
| process_exe = p.exe() | ||
| if process_cwd != ts_path: | ||
| continue | ||
| if process_name != '[TS_MAIN]' and process_name != 'traffic_server' and os.path.basename( | ||
| process_exe) != 'traffic_server': | ||
| continue |
There was a problem hiding this comment.
In the process scan loop, p.exe() is fetched before checking whether p.cwd() matches ts_path. Since exe() can be relatively expensive and can raise AccessDenied, consider only calling p.exe() after the cwd (and possibly name) filter passes to reduce overhead and avoid unnecessary permission failures during iteration.
| process_exe = p.exe() | |
| if process_cwd != ts_path: | |
| continue | |
| if process_name != '[TS_MAIN]' and process_name != 'traffic_server' and os.path.basename( | |
| process_exe) != 'traffic_server': | |
| continue | |
| if process_cwd != ts_path: | |
| continue | |
| if process_name != '[TS_MAIN]' and process_name != 'traffic_server': | |
| process_exe = p.exe() | |
| if os.path.basename(process_exe) != 'traffic_server': | |
| continue |
| Test.Summary = 'Test that Trafficserver starts with different thread configurations.' | ||
| Test.ContinueOnFail = True | ||
| Test.SkipUnless(Condition.IsPlatform("linux")) |
There was a problem hiding this comment.
PR description says the test is skipped on macOS, but this change skips it on all non-Linux platforms (SkipUnless(IsPlatform("linux"))). If the thread introspection works on other supported OSes (e.g., FreeBSD), consider using a macOS-specific skip instead, or update the PR description to match the broader skip behavior.
check_threads.py now uses a short bounded poll/retry window so thread-count validation doesn’t fail on startup races.
The test is also skipped on macOS because per-thread introspection used by this check is not available there.