[OPIK-8197] [BE] fix: unhang PythonEvaluatorServiceTest and bound every test with a timeout - #8108
Conversation
…ry test The CI Unit Tests job was dying at its 10-minute timeout-minutes wall on main and most backend PRs, while successful runs of the same job took ~3 minutes. That bimodality was the tell: not a slow suite, a deadlocked test. OPIK_8082 generalised RetriableHttpClient.performHttpRequest to serve GET as well as POST, so the outbound call moved from builder.async().post(body, cb) to builder.async().method(method, body, cb). This test still stubbed .post(...), so the stub no longer matched, the doAnswer that invokes callback.completed() never ran, and the Mono.create sink never received a terminal signal -- .block() parked on a CountDownLatch forever. A jstack of the hung fork showed main in Mono.block with boundedElastic idle and no thread in Opik code at all. Mockito's strict stubbing would normally flag the never-matched stub, but the test hangs before the strictness check gets to run. surefire.rerunFailingTestsCount could not help either: the test never fails, so there is no result to retry. Retarget the 16 stubs and 9 verifies onto the method(...) overload and shift the callback argument index 1 -> 2. Match on eq(HttpMethod.POST) rather than any string so the mock pins the verb -- accepting any method is what let this drift through unnoticed. Also add a suite-wide per-test timeout, so the next test to block is failed and named in the Surefire report instead of consuming a whole CI job anonymously. 5m is sized off the slowest legitimate test (~2m: two chained 60s Awaitility waits), and scoped to testable methods so Testcontainers startup in lifecycle callbacks is untouched. Verified: the class now passes 20 tests in ~3s; the full CI unit list runs 3716 tests green in 1:32; and an injected never-terminating test is reported as "Timeout ... timed out after" against its own name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
java
⏱️ pre-commit per-hook timing
⏭️ 41 skipped (no matching files changed)
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Review caught a real defect in the guard added by the previous commit. CI runs -Dsurefire.rerunFailingTestsCount=3, and a JUnit timeout is an ordinary failure to Surefire, so a deterministic hang is attempted 4 times. At the 5m bound that is 20m against a 10m Unit Tests wall: the job would be cancelled before Surefire could publish the named failure report -- losing precisely the diagnostic the timeout exists to provide, and leaving the original "job cancelled with no attributable test" symptom in place. Measured rather than assumed: a probe test blocking on a CountDownLatch with a 5s bound and rerun=3 produced Run 1..4, four attempts, ~4x the bound in wall time. The old value was also sized off the wrong population. It was derived from the slowest test overall (two chained 60s Awaitility waits), but that test and every other multi-second waiter is container-backed and therefore classified as an integration test. The slowest Awaitility wait in any unit-classified test is 10s. A single bound sized for integration was being applied to both tiers. So make the bound per-tier, carried through the existing matrix: 2m for unit (4 x 2m = 8m inside the 10m wall) and 4m for integration (4 x 4m = 16m inside the 20m wall). Both stay far above what their tier actually needs. Also add TestTimeoutGuardTest, which the review correctly noted was missing: the safeguard lived only in a properties file, so a rename or a scope change could silently disable it while the suite still passed green. It asserts the blocked thread is interrupted and reported, that the shipped bound survives the retry budget, and that the property stays scoped to testable methods so container startup in lifecycle callbacks is not bounded. It is fast and cannot hang -- the blocking case is bounded preemptively. Verified it fails on a revert to 5m. Full unit job re-run as CI runs it (207 classes, rerun=3, 2m override): 3719 tests green in 1:29, zero guard-triggered timeouts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review found two defects in TestTimeoutGuardTest, both of which made it weaker than advertised. Fixing them matters more than usual: this class exists to stop a wrong timeout reaching CI, so a hole in it is a hole in the safeguard itself. First, the shipped default was never validated where it counts. configuredTimeout() preferred the -D override when present, and CI always sets one -- so CI only ever checked the override, and junit-platform.properties could drift to any value undetected. Worse, the "verified it fails on a revert to 5m" claim on the previous commit exercised the override path, not the file. The shipped default and the override are now separate, unconditional assertions: the default is read straight off the classpath and never consults System.getProperty. Confirmed by breaking the file to 20m while passing a valid -D 2m -- the CI situation exactly -- which now fails where it previously passed silently. Second, toSeconds() stripped non-digits and only honoured a lowercase trailing 'm', so 2h, 2d, 2ms and 2H all collapsed to their bare number: a 2h bound (8h across four attempts, against a 10m wall) sailed through. It now parses JUnit's documented grammar case-insensitively across ns/us/ms/s/m/h/d and REJECTS anything outside it, rather than coercing it to a number that happens to pass. Silent coercion is the failure mode that let 2h through, so the rejection is the point. Covered by parameterized cases for both valid and invalid forms. Also pinned both tiers in discover-backend-tests.sh. The override is per-tier but the tier is not knowable from inside the JVM, so the script is the only place the per-test values and their job walls are stated together; asserting the pairing keeps a change to one from silently outgrowing the other. Verified it fails when UNIT_TEST_TIMEOUT is bumped to 5m. 19 tests, ~0.3s, still cannot hang. Full unit job as CI runs it (207 classes, rerun=3, 2m override): 3735 tests green in 1:29. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Backend Tests - Integration Group 14 43 files 43 suites 3m 36s ⏱️ Results for commit 85259c4. ♻️ This comment has been updated with latest results. |
…o end Third review round, three more real holes in the guard -- all verified by breaking each one and watching the assertion stay green before the fix. Overflow was the worst of them. `toSeconds(value) * ATTEMPTS_PER_HANG` used plain long math, so a grammar-valid but absurd bound wrapped negative and satisfied isLessThan(wall): `-D ...=106751991167300d` PASSED, meaning the assertion written to reject oversized timeouts accepted the most oversized values of all. Now Math.multiplyExact with an explicit fail-closed AssertionError, plus isPositive() so a negative budget can never read as "fits", and the same treatment for the microsecond conversion, which could wrap before the budget check ever saw it. Covered by failsClosedOnOverflow. Second, overrideFitsRetryBudget returned early when no -D was present, so deleting or renaming the override in backend_tests.yml would leave every CI job silently on the shipped default with the guard still green -- the guard could not detect its own removal. It now requires the property under GITHUB_ACTIONS (set by the runner, so it cannot drift from the workflow) while staying skippable locally, where there is no override and the shipped default is asserted unconditionally. Third, pinning the shell variables proved nothing about what reaches JUnit: the matrix has to emit them and the workflow has to pass them through, and each link can be renamed or dropped on its own. ciMatrixWiresBothTiers now asserts the whole chain -- the -D on the mvn step, timeout-minutes sourced from matrix.timeout, and both emitted entry shapes pairing their wall with their per-test bound. Verified each path fails after the fix: -D line removed from the workflow, unit testTimeout omitted from the matrix, and GITHUB_ACTIONS=true with no override. 22 tests, ~0.33s, still cannot hang. Full unit job as CI runs it: 3738 tests green in 1:42. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Note on Evidence it is independent of this branch:
The Aug 31 run predates this branch, is on an unrelated branch, and had no per-test timeout configuration in play at all. The timeout guard added here is specifically not implicated: these are Awaitility Backend jobs on the latest commit: 🤖 Reply posted via /address-github-pr-comments |
The isPositive() check added last commit to stop overflow wrapping negative had a mirror-image bug: toSeconds() floored every sub-second value to 0, and 0 is not positive, so the guard rejected the smallest safe bounds as if they were no bound at all. Measured: -D 500ms, 999ms, 100us and 5000000ns were all rejected, each of which costs at most a couple of seconds across four attempts against a 10m wall. 1500ms passed only because it truncated to 1s, which is the tell that the unit and not the value was the problem. Parse to a Duration and compare Durations, so magnitude survives below a second. The overflow protection it has to coexist with is unchanged in effect -- Duration.multipliedBy still throws ArithmeticException, so absurd bounds keep failing closed -- and the grammar rejection is untouched. parsesJUnitTimeoutGrammar now asserts in millis rather than seconds, so a future truncation regression is visible in the expectation itself rather than hidden by the unit. acceptsSubSecondBudgets covers the five values that regressed. Re-verified nothing else moved: absurd bounds still rejected (9999999999d, 999999999999999999ms, 106751991167300d), the workflow -D line and matrix testTimeout key are still required, and CI still fails without an override. 29 tests, ~0.36s. Full unit job as CI runs it: 3745 tests green in 1:46. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No test needed here. Test-and-CI only: all five files are under Advisory, from the QA test radar. Nothing here blocks this PR, and anything it proposes is a draft for review. Re-checked after a push on 02 Sep 16:33 UTC — nothing the verdict depends on changed. |
thiagohora
left a comment
thiagohora
left a comment
There was a problem hiding this comment.
Automated code review findings (excluding style-only nits).
| # Scoped to testable methods rather than timeout.default so @BeforeAll/@BeforeEach are exempt -- | ||
| # Testcontainers startup is slow and highly variable, and bounding it here would make container | ||
| # setup, not the test, the thing that fails. | ||
| junit.jupiter.execution.timeout.testable.method.default = 2m |
There was a problem hiding this comment.
The shipped 2-minute default here applies globally to every testable method, but it's sized for the unit tier. A developer running mvn test -Dtest=SomeIntegrationTest locally (without CI's per-tier -D override) inherits this 2m default instead of the intended 4m integration bound. The PR description itself notes the slowest known integration test already chains two 60s Awaitility waits (~120s) — right at this cap — so any JVM/Testcontainers overhead on top pushes it over, spuriously failing a previously-passing test outside CI.
There was a problem hiding this comment.
Fixed in b192512 — good catch, and I had this backwards. I sized the shipped value for the unit tier while it is actually what local runs inherit, and locally nothing knows which tier a test belongs to.
Confirmed the numbers you cite: TraceThreadOnlineScoringAgenticToolsE2ETest chains two 60s Awaitility waits in a single method at lines 218 and 228, so ~120s against a 2m cap. Any Testcontainers or JVM overhead on top fails a previously-passing test for anyone running it outside CI.
The file now ships 4m, which still fits the integration wall under retries (4 × 4m = 16m < 20m). CI does know the tier and keeps overriding per job from the matrix, so the tighter 2m unit bound still applies exactly where it matters — the value here only governs local runs, which is now stated in the comment so the next person does not have to infer it.
I also noted JUnit's real grammar in the file, since the valid-value question came up on the other thread.
🤖 Reply posted via /address-github-pr-comments
There was a problem hiding this comment.
Commit b192512 addressed this comment by changing the local default timeout from 2m to 4m, accommodating integration tests with two 60-second waits plus overhead. The comments also clarify that CI still overrides the timeout per test tier.
| * case-insensitively. A bare number means seconds. Anything else must be rejected rather than | ||
| * silently coerced -- the whole point of this class is that a wrong bound cannot pass unnoticed. | ||
| */ | ||
| private static final Pattern TIMEOUT_GRAMMAR = Pattern.compile("(?i)^(\\d+)\\s*(ns|μs|us|ms|s|m|h|d)?$"); |
There was a problem hiding this comment.
TIMEOUT_GRAMMAR doesn't actually match JUnit's real TimeoutDurationParser regex. Disassembling the actual JUnit bytecode shows the real pattern is ([1-9]\d*) ?((?:[nμm]?s)|m|h|d)? — it only accepts the Unicode μs and rejects leading zeros / a bare 0. This grammar here also accepts ASCII us and values like 0m/007m. If a value like 100us were ever set as the real -D override, JUnit would throw DateTimeParseException at test-engine startup — the exact silent-coercion failure this guard exists to catch, just for a unit alias the guard itself invented.
There was a problem hiding this comment.
Commit b192512 addressed this comment by removing TIMEOUT_GRAMMAR and its custom JUnit grammar parser entirely. The replacement validates only the shipped whole-minute/second format, avoiding the incorrect aliases and semantics.
There was a problem hiding this comment.
Fixed in b192512 — you are right, and I verified it against the engine bytecode rather than taking it on trust. From TimeoutDurationParser in junit-jupiter-engine:
String ([1-9]\d*) ?((?:[nμm]?s)|m|h|d)?
String Timeout duration is not in the expected format (<number> [ns|μs|ms|s|m|h|d])
Exactly as you describe: leading digit must be 1–9 (so 0m and 007m are rejected), and the microsecond unit is the Unicode μs only — no ASCII us.
The consequence is worse than a cosmetic mismatch, and I confirmed it by running it: -D ...=100us throws
java.time.format.DateTimeParseException: Timeout duration is not in the expected format (<number> [ns|μs|ms|s|m|h|d])
while my test suite reported all 29 tests passing on that same value. So the guard was certifying as valid a value that breaks the engine at startup — the exact silent-coercion failure it was written to catch, for a unit alias I invented. A guard that green-lights something JUnit rejects is worse than no guard.
Rather than transcribe the real regex, I removed the reimplementation entirely (see the reply on your file-level comment). JUnit validates its own grammar correctly and fails loudly at engine startup, so duplicating it here only created a second, wrong source of truth. The remaining check accepts just the whole-minute/second forms the shipped value is ever expected to use, and fails rather than guesses on anything else.
🤖 Reply posted via /address-github-pr-comments
| // timeout.testable.method.default property: it abandons the blocked thread and fails. | ||
| Executable blocksForever = () -> neverCountedDown.await(); | ||
| var failure = assertThrows(AssertionError.class, | ||
| () -> assertTimeoutPreemptively(Duration.ofMillis(200), blocksForever)); |
There was a problem hiding this comment.
This guard-of-the-guard test uses a 200ms assertTimeoutPreemptively bound, which is tight enough to be flaky under CI resource contention. It spins up a new thread and waits for it to be interrupted; scheduling latency or a GC pause pushing actual completion past 200ms for reasons unrelated to the CountDownLatch could make this assertion flaky — a bit ironic in a test meant to prove the timeout mechanism is reliable.
There was a problem hiding this comment.
Commit b192512 addressed this comment by deleting the flaky 200ms preemptive-timeout test entirely.
There was a problem hiding this comment.
Fixed in b192512 — the test is gone, so the 200ms bound is too.
The irony was fair: asserting the timeout mechanism is reliable via a 200ms preemptive bound that could itself flake on scheduling latency or a GC pause is self-undermining. And it was testing JUnit's own assertTimeoutPreemptively, not any Opik behaviour — so a failure would have told us something about the JVM or the runner, not about this change.
It was removed as part of trimming the file down to the retry-budget check (see the reply on your file-level comment), rather than by loosening the bound, since a looser bound would still have been testing framework internals.
🤖 Reply posted via /address-github-pr-comments
There was a problem hiding this comment.
Not convinced this file earns its ~290 lines. It's a meta-test that re-implements JUnit's own timeout grammar (and gets it wrong — see the inline comment on TIMEOUT_GRAMMAR above) and string-greps backend_tests.yml/discover-backend-tests.sh for literal substrings, which will break on any unrelated formatting change to those files. It also adds no coverage of the actual bug in this PR (the .post(...)→.method(...) stub mismatch in PythonEvaluatorServiceTest) — it only guards against someone later misconfiguring the timeout properties/workflow, a much rarer failure mode than what caused this incident.
The properties-file addition plus the stub retargeting already fix the real problem (a hang now fails loudly with a named test instead of silently eating the CI job). Suggest dropping this file, or trimming it to just the "shipped default fits the retry budget" check without the grammar reimplementation or the YAML/script scraping.
There was a problem hiding this comment.
Commit b192512 addressed this comment by removing the timeout grammar reimplementation, CI workflow/script scraping, and unrelated tests, leaving only the shipped-default retry-budget check.
There was a problem hiding this comment.
Fixed in b192512 — took your second option: trimmed to the budget check. 291 lines → 88, 29 tests → 2.
Your read matches where this file ended up. It accreted across three earlier review rounds, and every one of those rounds found a bug in the guard rather than in the fix — overflow wraparound, then sub-second truncation, now the invented grammar. That is a strong signal the surface was not worth its size, and it is the same conclusion I had reached before your review landed.
Removed, all four of your objections:
- the hand-rolled
TIMEOUT_GRAMMAR— wrong, per your other comment, and verified so against the engine bytecode - the invented ASCII
usalias, which the guard was certifying as valid while JUnit throws on it - the
backend_tests.yml/discover-backend-tests.shsubstring greps — brittle against unrelated formatting changes there - the 200ms
assertTimeoutPreemptivelybound — flaky, and testing JUnit rather than Opik
Kept, because it is the one assertion that caught something real:
shippedDefaultFitsRetryBudget— 4 attempts at the shipped bound must fit the job wall. This is what caught the original 5m value, which would have been cancelled at 4 × 5m = 20m against the 10m unit wall. Read from the properties file rather thanSystem.getProperty, so CI's-Doverride cannot mask a bad shipped default. Verified it still fails on a 6m default, both with and without an override in play.timeoutIsScopedToTestableMethods— one line, guards against a blankettimeout.defaultbeing introduced, which would bound@BeforeAlland make Testcontainers startup the thing that fails.
Grammar validation is left to JUnit, which does it correctly.
Agreed on your framing of the real fix, and worth restating since the guard has dominated the review: the stub retargeting plus the properties file are what close the ticket. That part has been green in CI since the second commit.
🤖 Reply posted via /address-github-pr-comments
Human review, and both findings land. Acting on them removes ~200 lines and a real footgun. The shipped 2m default was sized for the unit tier but is what a LOCAL run inherits, and locally nothing knows which tier a test belongs to. The slowest integration test chains two 60s Awaitility waits in one method (~120s), so 2m sat right at that cap: any Testcontainers or JVM overhead would spuriously fail a passing test for anyone running it outside CI. The file now ships 4m, which still fits the integration wall under retries (4 x 4m = 16m < 20m). CI does know the tier and keeps overriding per job from the matrix, so the tighter 2m unit bound still applies exactly where it matters. TestTimeoutGuardTest is cut from 291 lines to 88. Review was right that it did not earn its size, and right about why -- confirmed from the engine bytecode: JUnit's real pattern is ([1-9]\d*) ?((?:[nμm]?s)|m|h|d)?, which takes only the Unicode "μs" and rejects leading zeros. My hand-rolled TIMEOUT_GRAMMAR invented an ASCII "us" alias and accepted 0m/007m, so the guard was certifying "100us" as valid while JUnit throws DateTimeParseException on it at engine startup. A guard that green-lights a value which breaks the engine is worse than no guard: it is the silent-coercion failure it was written to prevent, for a unit I made up. Also gone: the YAML/discovery-script substring greps, which would break on any unrelated formatting change to those files, and a 200ms assertTimeoutPreemptively bound tight enough to flake under CI contention -- ironic in a test asserting the timeout mechanism is reliable. What survives is the assertion that actually caught something: 4 attempts at the shipped bound must fit the job wall, read from the file rather than System.getProperty so a CI -D override cannot mask it. That is the check that caught the original 5m bound. Verified it still fails on a 6m default, with and without an override in play. Grammar validation is left to JUnit, which does it correctly and fails loudly. Full unit job as CI runs it: 3718 tests green in 1:54. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 participants
Details
The CI
Unit Testsjob was being killed at its 10-minutetimeout-minuteswall onmainand on most backend PRs, while successful runs of the same job finished in ~3 minutes. That bimodality (either ~3m or exactly the wall, with nothing in between) is the tell: this was a deadlocked test, not a slow suite. Across the last 100 workflow runs, 24 of 85Unit Testsjobs died this way — 17 of them on a single day,mainincluded.OPIK_8082generalisedRetriableHttpClient.performHttpRequestto serve GET as well as POST, moving the outbound call frombuilder.async().post(body, cb)tobuilder.async().method(method, body, cb).PythonEvaluatorServiceTeststill stubbed.post(...), so the stub stopped matching, thedoAnswerthat invokescallback.completed()never ran, and theMono.createsink never received a terminal signal —.block()parked on aCountDownLatchforever. Ajstackof the hung fork showedmaininMono.block,boundedElasticidle, and no thread anywhere in Opik code.Two things worth calling out, because they explain why nothing caught this:
-Dsurefire.rerunFailingTestsCount=3is configured on the job but is structurally unable to help: the test never fails, so there is no result to retry.This PR retargets the 16 stubs and 9 verifications onto the
method(...)overload and shifts the callback argument index from 1 to 2. The matcher iseq(HttpMethod.POST)rather than a permissive any-string, so the mock now pins the verb — accepting any method is precisely what let this drift through unnoticed.It also adds a per-test timeout, so the next test that blocks is failed and named in the Surefire report instead of consuming a whole CI job anonymously. Today that failure surfaces only as "job cancelled", and finding the culprit means reading raw job logs for the class that printed
Running ...with no matching result line.The bound has to fit the retry budget, not a single attempt: CI runs
-Dsurefire.rerunFailingTestsCount=3, and a timeout is an ordinary failure to Surefire, so a deterministic hang is attempted 4 times (measured: a probe blocking on aCountDownLatchproducedRun 1–Run 4). If 4x the bound exceeds the job wall, the job is cancelled before Surefire can publish the named report — losing the exact diagnostic the guard exists to provide.It is therefore per-tier, carried through the existing matrix rather than hardcoded —
discover-backend-tests.shemitstestTimeoutper entry and the workflow passes it as-D, so the job wall and the per-test bound sit next to each other and cannot drift apart:Both stay far above what their tier needs: the slowest Awaitility wait in any unit-classified test is 10s, and the 60s+ waits all live in container-backed integration tests. It is scoped to testable methods so Testcontainers startup in lifecycle callbacks is untouched.
TestTimeoutGuardTestkeeps the shipped bound honest: 4 attempts at it must fit the job wall, read from the properties file rather thanSystem.getPropertyso CI's-Doverride cannot mask a bad shipped default. That is the assertion that caught the original 5m value. A second one-line check guards against a blankettimeout.defaultbeing introduced, which would bound@BeforeAlland make Testcontainers startup the thing that fails.It is deliberately narrow (88 lines, 2 tests). An earlier revision reimplemented JUnit's timeout grammar and string-matched the workflow and discovery script; review pointed out the reimplementation was wrong — JUnit's real pattern is
([1-9]\d*) ?((?:[nμm]?s)|m|h|d)?, taking only the Unicodeμs, so the guard was certifying100usas valid while JUnit throwsDateTimeParseExceptionon it at engine startup. Grammar validation is now left to JUnit, which does it correctly and fails loudly; the YAML/script greps went too, since they would break on any unrelated formatting change there.Note the shipped value is 4m, not the unit tier's 2m: this file is what local runs inherit, and locally nothing knows a test's tier, so it has to accommodate the slower one (the slowest integration test chains two 60s waits, ~120s). CI overrides per job from the matrix, so the tighter unit bound still applies where it matters.
Retries are kept rather than disabled — they still serve genuine flakes — with the budget sized to fit instead.
Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
Commands run, all from
apps/opik-backend:mvn test -Dtest='PythonEvaluatorServiceTest'— the previously-hanging class now passes 20 tests in ~3s (was: blocked until the job was killed)..github/scripts/discover-backend-tests.sh(207 classes) with the same flags CI uses (-Dmaven.test.failure.ignore=true -Dsurefire.rerunFailingTestsCount=3) and the real2mper-test override: 3718 tests, 0 failures, 0 errors, 1:54.mvn spotless:check— passes.pre-commit run --files <changed files>— passes, no reformatting.Scenarios validated:
jstackthread dump showingmainparked inMono.blockat the failing assertion, with no thread in Opik code — ruling out slowness or retry backoff.CountDownLatch(the same primitive as the real bug). It is reported asGuardProbeTest.hangsForever » Timeout hangsForever() timed out, with a stack trace pointing at the blocking line — confirming the "failed and named in the report" behaviour rather than assuming the config works. Probe removed afterwards.junit-platform.propertiesitself and no-Doverride; it timed out at exactly the configured value.TimeoutExceptionstrings in the log are deliberate timeout/retry simulations from tests exercising those paths, not the guard.CountDownLatchwith a 5s bound andrerunFailingTestsCount=3producedRun 1–Run 4— confirming a hang costs 4x the per-test bound.6m(4 x 6m = 24m > the 20m wall) fails, both with and without a valid-Doverride in play. Reading the file rather thanSystem.getPropertyis what makes the override unable to mask a bad shipped default; that is the check that caught the original5mbound.TraceThreadOnlineScoringAgenticToolsE2ETestchains two 60s Awaitility waits (lines 218, 228), ~120s total — which is why the shipped value is4mand not the unit tier's2m.([1-9]\d*) ?((?:[nμm]?s)|m|h|d)?. An earlier revision's hand-rolled parser accepted an ASCIIusalias that JUnit rejects withDateTimeParseException; that reimplementation is now removed rather than corrected, since duplicating the grammar created a second, wrong source of truth.discover-backend-tests.shand confirmed every entry carriestestTimeout(unit2m, integration4m);actionlintandzizmorpass on the workflow change.Not run: the integration groups. This change touches one unit test class plus a test-scope properties file; the integration matrix is healthy and well under its 20-minute budget. CI will exercise it.
Documentation
N/A — no user-facing or API surface affected. The rationale for the bound, its coupling to the retry budget, and the per-tier split are documented inline in
junit-platform.propertiesanddiscover-backend-tests.sh.