Fix flaky expression language probe test setup - #7570
Conversation
Co-authored-by: P403n1x87 <20231758+P403n1x87@users.noreply.github.com>
|
I can only run on private repositories. |
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e53bbfd3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not self.wait_for_all_probes(statuses=["INSTALLED"], timeout=60): | ||
| self.setup_failures.append("Probes did not reach INSTALLED status") | ||
| # Stop the test if the probes did not reach INSTALLED status since the probe won't exist | ||
| # to send a snapshot. | ||
| return |
There was a problem hiding this comment.
Initialize Ruby remote config before waiting for probes
For the enabled Ruby rails72, rails80, and uds-rails variants, this new early return runs before any request can start the Ruby Remote Configuration client. BaseDebuggerTest.initialize_weblog_remote_config() documents that Ruby starts it only from request middleware, but this class never calls that helper; therefore the INSTALLED wait times out, records a setup failure, and every expression-language test fails instead of exercising its probes. Initialize remote config before sending and waiting for probes (which also resets setup_failures per setup).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
One probe installation timeout changes a shared failure list. This makes later expression-language tests fail even when their setup succeeds.
🤖 Datadog Autotest · Commit 4e53bbf · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| self.send_rc_probes() | ||
| self.wait_for_all_probes(statuses=["INSTALLED"]) | ||
| if not self.wait_for_all_probes(statuses=["INSTALLED"], timeout=60): | ||
| self.setup_failures.append("Probes did not reach INSTALLED status") |
There was a problem hiding this comment.
Keep the setup failure on one test instance
One rare probe timeout can cause many later expression-language tests to fail and can hide their real results.
Assertion details
- Input: One expression-language setup times out before all probes reach
INSTALLED, and later tests in the same process continue. - Expected:
The timeout failure must apply only to the test instance whose probes do not install. - Actual: The append changes the class-level list. Each later test instance reads the same failure. Its
assert_setup_ok()call then fails even when its setup succeeds.
| self.setup_failures.append("Probes did not reach INSTALLED status") | |
| self.setup_failures = ["Probes did not reach INSTALLED status"] |
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Motivation
test_expression_language_comparison_operatorsis flaky (~0.2%, 1/483 runs in the last 30 days). The test creates 96 probes (48 expressions × 2 probe types for Python). Under CI load, the default 30s timeout forwait_for_all_probescan expire before all probes reachINSTALLED. Because the return value was ignored, the test proceeded to send a weblog request against uninstalled probes that never emit, producing a confusing "probes are not emitting: RECEIVED" assertion failure instead of a clear setup error.Changes
tests/debugger/test_debugger_expression_language.py:_setup: check thewait_for_all_probes(statuses=["INSTALLED"], timeout=60)return value; on failure, append toself.setup_failuresand return early so the test does not request against uninstalled probes. Also bumpedwait_for_all_snapshots()totimeout=60._assert: addedself.assert_setup_ok()as the first assertion afterself.collect()so setup failures surface as a clear error.This aligns
_setup/_assertwith the established robust pattern already used by sibling debugger tests (test_debugger_capture_expressions.py,test_debugger_probe_snapshot.py).Testing
ruff checkon the modified file — all checks pass.PR by Bits - View session in Datadog
Comment @DataDog to request changes