Repository navigation
[SDTEST-4060] Add Go test automation exclusion declarations - #291
calvinbayer wants to merge 2 commits into
Conversation
|
@autotest Review |
|
Validation at signed head 14c6b92: focused end-to-end exclusion matrix passes (24.5s), including shared parallel table callbacks, declaration isolation, compiled metadata without source, pointer/value Testify methods, cleanup failures, native repetitions, EFD/ATR and both retry modes. go vet, maintained-source verification (397 dd-trace-go files), dependency boundary and all 14 CI-shard checks pass. Full compatibility validation against the repository-pinned Orchestrion and root workflows are running; CI will provide platform validation. |
There was a problem hiding this comment.
Two critical correctness gaps stand out: exclusion/protection policies are lost for quarantined Testify parents running under -race + process retry mode (findings 0/4), and the DD_CIVISIBILITY_SUBTEST_FEATURES_ENABLED=false feature gate is bypassed when annotation metadata is applied (findings 2/7). Additional distinct issues include promoted-method name mismatches dropping exclusion registrations, unconditional Attempt-to-Fix inheritance overriding explicit child assignments, and annotated methods in imported helper packages never receiving registrations.
🤖 Bits Code Review · Commit 14c6b92
There was a problem hiding this comment.
Two of the most critical issues are: (1) in race+process-retry mode, quarantined Testify parent isolation paths skip declaration-exclusion resolution, allowing protected failures to be silently suppressed; and (2) promoted Testify methods (inherited via suite embedding) lose their annotation-based exclusions because the runtime name of the wrapper does not match the registered declaration name.
🤖 Bits Code Review · Commit 14c6b92
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: b41d939 | Docs | View more details | Give us feedback! |
|
@autotest Review |
There was a problem hiding this comment.
The two most critical issues are: (1) protected-failure state is not accumulated across ATF retry attempts in in-process mode, allowing a command to exit 0 despite an earlier blocking failure; and (2) the exclusion cache uses only t.Name() as its key, enabling cross-suite collisions that can suppress failures or skip unrelated tests during retries. Additional distinct concerns cover cgo-sourced declarations being invisible to exclusion scans, ordinary nested suite methods being incorrectly rejected during process-retry validation, and exclusion payloads exceeding the 64 KiB control-file limit in process transport mode.
🤖 Bits Code Review · Commit 46123b6
|
@autotest Review Please review signed head 114e790. All five previous findings are addressed with regressions. Expanded normal, Go 1.25, and final actual race matrices passed (race 109.571s). Policy/protocol units, vet, source integrity, dependency boundaries, CI shard and strict source-frame checks passed. Independent bounded process review returned no findings. Exact-head CI is running. |
|
@autotest Review Final signed head 48f97bf adds only two per-command signing flags for synthetic fixture commits. Runtime, annotation, policy and transport code is identical to 114e790. Both affected fixture scenarios and all pinned Orchestrion composition variants pass (67.409s). Please review this exact head; final-head CI and the full local Go suite are running. |
There was a problem hiding this comment.
The most critical issue is that pointer receiver type aliases (e.g., type P = *Suite) cause the registration code generator in exclusions.go to emit an invalid (*P).TestCritical expression, breaking compilation of otherwise valid instrumented builds. A separate concern is that cleanup panics for annotated top-level tests are silently swallowed rather than re-propagated, diverging from native Go termination behavior.
🤖 Bits Code Review · Commit 48f97bf
|
@autotest Review Please review signed head62e6a8339bead34b5c7ace0c96d8b58016631d24. Both prior findings are fixed with native compiler/runtime regressions. Complete normal exclusion matrix54.570s and focused actualrace14.816s pass. Source398integrity, vet, dependency boundary,14shard checks, strictsource-frame controls pass. Independent bounded final delta review: no findings. Exact-head CI and complete local Go suite are running. |
There was a problem hiding this comment.
The two most structurally critical issues are: (1) the exclusion/replay cache is re-initialized on every nested management wrapper entry in instrumentation.go, so excluded callbacks re-execute instead of replaying their original result under ATR/EFD/ATF parent retries; and (2) a no_auto_retry Testify method with a full table (4,096 rows) pushes the process-retry cache over its entry limit, blocking unrelated ATR siblings from recovering.
🤖 Bits Code Review · Commit 62e6a83
|
@autotest Review Please review signed head a98737d. All four prior findings are fixed with real compiler/runtime regressions. Complete normal matrix passed 97.075s, focused corrected nested actual race matrix 53.068s, bounded cache/protocol units 2.116s. Vet, source integrity (398 files), dependency boundary, 14 shard checks, and strict source-frame controls pass. Independent bounded final reads found no further issues. Complete actual race matrix, full local pinned suite, and exact-head CI are running. |
|
Validation update at signed a98737d: the complete final actual race annotation matrix passed (212.075s), alongside the complete normal matrix (97.075s). This includes all four latest review fixes, declaration-isolated nested management counters, native repetitions and duplicate row names, protected cached ancestor outcomes, 4,096 generated rows, and changed ancestors with an independent shared excluded leaf. The full pinned compatibility suite and exact-head CI continue. |
There was a problem hiding this comment.
Process-isolated descendant retries can rerun excluded tests and let protected failures produce a successful command. Valid isolation-root cache entries are also rejected, and replayed excluded-test events lose TIA metadata.
🤖 Bits Code Review · Commit a98737d
|
@autotest Review Please review signed head 411bc20. All four previous findings are fixed with actual process-boundary regressions. Native filter preservation also fixes a proven selected-sibling regression: native passed, annotation-off passed while admitting the filtered callback, annotation-on failed; all controls now pass without admitting it. Focused final transport/TIA matrix passed 27.759s, native TIA declaration-tag controls 8.712s, selector regression normal 12.595s and actual race 19.970s. Units, vet, source provenance (400 files), dependency boundary and 14 shard checks pass. Independent bounded review found no additional issue. Full normal/race matrices, full pinned local suite and exact-head CI are running on frozen source. |
|
Final frozen-head validation at 411bc20: complete normal annotation matrix passed (155.030s), and complete actual race matrix passed (271.702s). Both include native filter preservation, independent descendant ATF cache/protected outcomes, selected-root cache identity, native top-level TIA declaration tags, and late worker TIA metadata/counters. Bounded units, vet, 400-file provenance, dependency and shard checks pass. Exact-head CI and the full pinned compatibility suite remain in progress. |
There was a problem hiding this comment.
Adding either exclusion annotation to a generic Testify method causes instrumentation to abort before tests execute. The receiver scanner rejects valid generic receiver syntax.
🤖 Bits Code Review · Commit 411bc20
|
@autotest Review Please review signed head ee94b72. The remaining generic receiver finding is fixed, including legal parenthesized receivers. Compiled local/imported pointer/value/multi-parameter fixtures passed on current Go (14.755s), Go 1.25.11 (15.067s) and actual race builds (16.496s); excluded callback/event and ordinary unrelated-declaration controls remain strict. Runner units, vet, unchanged 400-file provenance, dependency boundary and all 14 shard checks pass. Independent bounded review: no findings. Previous signed head had all 146 CI checks green. Final exact-head CI, full normal/race matrices and full pinned local suite are running on frozen source. |
|
Frozen head ee94b72: complete normal annotation matrix passed (120.328s), and complete actual race matrix passed (298.987s), including the final generic and parenthesized receiver regressions. Current/minimum Go compiled generic controls and all required source/dependency/shard checks pass. Exact-head CI and the full pinned compatibility suite continue on unchanged source. |
There was a problem hiding this comment.
Three distinct correctness issues were found: the protocol envelope for ExcludedTests can be exceeded when a no_auto_retry method is late-discovered during process retries; the parallel-safety flag is prematurely cleared before user cleanup functions run, allowing unsafe process-wide state mutation; and promoted suite methods generate inconsistent cache keys, causing already-excluded declarations to re-execute.
🤖 Bits Code Review · Commit ee94b72
|
@autotest Review Please review signed head 521980e. All three prior findings are fixed with real boundary regressions. Combined final new-delta matrix passed normal 46.493s and actual race 60.449s; lifecycle native guards/ordering/body terminals, promoted declaration/reporting identity, and all 4,097 late-discovered events are preserved. Bounded units, vet, exact source-frame controls, unchanged 400-file provenance and 14 shard checks pass. Independent bounded reviews found no additional issue. PR description now describes final grouped behavior and runnable regression commands. Previous signed head had all 146 CI checks green; final exact-head CI, complete normal/race matrices and full pinned compatibility are running on frozen source. |
There was a problem hiding this comment.
Three distinct correctness bugs were identified: a receiver-type mismatch that silently bypasses test exclusion lookups for Testify suites, a potential deadlock when cleanup calls SkipNow/FailNow before the scheduler slot is reacquired in parallel subtest scenarios, and a nil exclusion-cache propagation gap that causes excluded descendant tests to run again on subsequent retries. All three fixes also require refreshing the maintained-source hash in SOURCE.json.
🤖 Bits Code Review · Commit 521980e
|
@autotest Review Final signed head fe9b50b corrects only the SOURCE.json digest for final ADAPTATIONS.md documentation. Runtime, CLI and test code is identical to 521980e. All-library provenance, dependency boundary and 14 shard checks pass on this head. All three prior review findings remain addressed; combined final regressions passed normal 46.493s and actual race 60.449s, and the full normal annotation matrix passed 119.352s. Exact-head CI, full actual race and full pinned local suite are running unchanged. |
There was a problem hiding this comment.
Three distinct correctness issues were found in the CI visibility test instrumentation: a cache-key collision that can mask failing rows during in-process retries, a Goexit/panic interaction that aborts remaining package tests for annotated exclusions, and a protection-propagation gap that lets quarantined parents suppress ATF family failures across retry attempts.
🤖 Bits Code Review · Commit fe9b50b
|
@autotest Review Please review signed head Final quick validation passed: all maintained-source manifests (400/9/52/19 files), dependency boundary, 14 shard tests, runner/runtime exclusion units, vet, exact source-frame checks, native lifecycle normal/race/Go1.25, public method API normal/race/Go1.25, and cache race controls. Fresh complete annotation matrices and pinned full compatibility suite are running on this frozen source; their results and this head's CI remain pending. |
|
Validation update for |
There was a problem hiding this comment.
Two distinct issues affect NoAutoRetry/excluded-test handling: one concerns serialization budget enforcement for ExcludedTests payloads that can exceed 16 MiB, and the other concerns verdict correctness when a failing excluded declaration is never rediscovered on a passing parent retry attempt, allowing a failed test to be silently suppressed even with PreventFailureSuppression set.
🤖 Bits Code Review · Commit 2729cf7
|
@autotest Review Please review signed head Focused validation passed:28 omitted-child/Q/PFS cases normal11.393s, actualrace50.998s, Go1.25.11 38.875s; bounded mixed/excluded4096-record diagnostic units on current/minimum Go; native fuzz fatal/skip and coverage differential48.027s; strict frame controls; all401/9/52/19 provenance files, dependency boundary,14 shard tests, runner/runtime units and vet. Fresh complete annotation matrices and pinned full compatibility suite are running on this frozen source; exact-head CI remains pending. |
calvinbayer
left a comment
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with an independent reviewer auditing both expectations and raw runtime evidence.
Revision tested: babe0e351a29048d6103d5d5c4672f3fb2bb87e0
Environment: local macOS arm64, Go 1.27.3, Gin playground; DDTest official release builder bundles Mini runtime. All six source checkouts and release binaries match this exact commit, with vcs.modified=false. SDK aa51eff7d37fd57d922594def527d5e070c9c092; Orchestrion 1.14.0.
| Shepherd case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| Native Go exclusions | 20261105-103339.388367000-14431-1 |
20261105-103521.477775000-18589-1 |
17/17 PASS |
| Testify exclusions | 20261105-103413.572743000-15749-1 |
20261105-103553.062742000-19485-1 |
17/17 PASS |
| Protected callback in quarantined Testify method | 20261105-103233.670008000-10924-1 |
20261105-103444.131363000-16752-1 |
15/15 PASS |
Result: 98 assertions passed across six runs. The independent reviewer inspected raw Mockdog test/session events, management requests, reports, execution logs and binary metadata. All cases require intentional command exit 1, failed sessions, exact event counts and positive annotation tags; missing events and startup errors fail the checks.
The new callback case runs an actual -race binary in process mode with ATR explicitly disabled. The callback has failure protection alone, retains its backend quarantine assignment, emits once, fails its method/parent/session and command, and never acquires a NoRetry tag. This prevents NoRetry from masking the quarantine regression. Native Go and Testify direct-declaration cases also retain all three backend assignments, both exclusion tags, and no retry/ATF-completion tags. Each case was repeated with fresh process/fixture state.
Method: Crook's go-mini-testing-test-automation-exclusions, go-mini-testify-test-automation-exclusions and go-mini-testify-callback-protection assertions on Gin, using --dep ddtest=codex/critical-test-annotations-go --debug. Crook removes fixtures and restores go.mod/go.sum; generated Orchestrion pin data was preserved. This supersedes the earlier review for e51d13d.
Additional regression checks for this tree: complete normal exclusion matrix 266.054s, focused new regressions with actual race workers 8.629s, minimum Go 1.25.11 actual-race callback matrix 31.624s, runtime units, go vet and maintained-source/dependency checks pass. Full race-matrix and platform CI results are tracked separately. Both latest Datadog findings were reproduced, fixed and replied to; there are no remaining Shepherd-review findings.
Scope: verified real framework execution through Mockdog intake, not the production Datadog UI. The existing in-process selected-quarantine-method capability fallback is preserved; the positive callback case exercises supported race/process isolation. This test-review verdict does not substitute for the separate Datadog code review or platform CI gates.
There was a problem hiding this comment.
Three distinct correctness issues were found: Testify nested rows incorrectly inherit the enclosing method's exclusion policy instead of their callback's, process-retry worker wrappers fail to propagate management state to excluded descendants causing skips to be missed, and concurrent body+cleanup panics lose the cleanup panic's diagnostics during finalization.
🤖 Bits Code Review · Commit babe0e3
|
@autotest Review |
calvinbayer
left a comment
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with an independent reviewer auditing test expectations and raw execution evidence.
Revision: 50925c7f83845e32f46d229cb95322e71ec3d8f3
Environment: local macOS arm64, Go 1.27.3, Gin playground; official DDTest release builder with bundled Mini runtime. All eight source checkouts and tested binaries match this exact commit, vcs.modified=false. SDK aa51eff7d37fd57d922594def527d5e070c9c092; Orchestrion 1.14.0.
| Shepherd case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| Nested callback row exclusions | 20261105-120917.354020000-27339-1 |
20261105-121048.775356000-32461-1 |
17/17 PASS |
| PFS-only callback beneath quarantined method, race/process | 20261105-120946.221299000-29833-1 |
20261105-121106.345404000-33809-1 |
15/15 PASS |
| Native Go exclusions | 20261105-121009.980650000-30978-1 |
20261105-121124.958801000-34592-1 |
17/17 PASS |
| Testify exclusions | 20261105-121029.847503000-31727-1 |
20261105-121146.539337000-36319-1 |
17/17 PASS |
All 132 assertions passed. The independent reviewer inspected raw parsed test/session events, management HTTP responses, reports, debug logs and build metadata. Each case requires intentional command exit 1 and a failed session, exact event counts, positive exclusion tags, retained backend assignments, and no forbidden retry/ATF-completion tags. The protection-only case uses an actual race/process worker with ATR disabled and requires NoRetry to remain absent. Cases report exactly 4/3/1/2 tests and 2/2/1/2 suites respectively, with one module and session each.
The nested-row case reproduced the previous commit's defect: five assertions failed because the command/session passed and both row exclusion tags were missing. Both runs now fail the command as intended, execute the protected row once and preserve all disabled/quarantine/ATF assignments.
Method: Crook's four go-mini-* assertions on Gin using --dep ddtest=codex/critical-test-annotations-go --debug, each twice. Fresh fixtures/process state, restored go.mod/go.sum and removed fixtures were verified. This supersedes the review of babe0e3.
Related native/worker acceptance checks cover callback-policy inheritance, disabled+ATF inheritance through ordinary worker wrappers, and body/multiple/dynamic cleanup panic diagnostics with native Goexit and queued-child behavior. Focused worker/native/strict-frame/TestMain/manual-RunM checks passed; full actual-race exclusion matrix passed 513.092s, minimum Go 1.25.11 actual-race worker checks 40.697s; vet/lint and all source manifests/inventory checks passed. The separate full local root test command has an identical Mocha guard failure on the published baseline, independently reproduced without source changes; it is not counted as a passing run. Whole-module and platform CI completion are tracked separately.
No remaining Shepherd-review findings. Scope is real instrumented framework execution through Mockdog intake; production Datadog UI was not checked. Ordinary annotation-off event-lifetime differences were compared to the exact published baseline and preserved. Generated Orchestrion pin data and coordinator review artifacts were retained, so no claim is made that the whole workspace is pristine. This verdict does not replace the separate Datadog code review or CI gates.
There was a problem hiding this comment.
Protected tests that time out or encounter a panic-hook crash can still emit a skipped final status because crash finalization interprets retained quarantine and disabled assignment tags as effective policy. The command remains failing, but the protected test’s final result is misclassified.
🤖 Bits Code Review · Commit 50925c7
|
@autotest Review |
calvinbayer
left a comment
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with independent review of expectations and raw runtime evidence.
Revision: 492b5889994a424712a576712130d5a8692d4779
Environment: macOS arm64, Go 1.27.3, Gin playground; official release builder with bundled Mini. Every source checkout and tested binary matches this exact commit, vcs.modified=false. SDK aa51eff7d37fd57d922594def527d5e070c9c092, Orchestrion 1.14.0.
| Case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| Protected timeout finalization | 20261105-124434.988245000-99358-1 |
20261105-124636.979518000-8105-1 |
18/18 PASS |
| Nested callback row exclusions | 20261105-124458.214790000-1325-1 |
20261105-124658.645690000-9170-1 |
17/17 PASS |
| PFS-only callback in quarantined method, race/process | 20261105-124522.682133000-3707-1 |
20261105-124717.631267000-10209-1 |
15/15 PASS |
| Native Go exclusions | 20261105-124549.815414000-5091-1 |
20261105-124737.882411000-11035-1 |
17/17 PASS |
| Testify exclusions | 20261105-124613.376567000-7138-1 |
20261105-124759.682710000-11778-1 |
17/17 PASS |
All 168 assertions passed. The independent reviewer checked raw parsed tests/sessions, management HTTP responses, logs, reports, build identity and fixture restoration. All cases require intentional command exit 1, failed sessions, exact event counts, correct true-only annotation tags, retained backend assignments and no forbidden retry/ATF-completion tags. Missing events or setup errors cannot pass.
The protected timeout test has exactly one event with error.type=timeout, test.status=fail and test.final_status=fail, despite retained disabled/quarantine/ATF assignment tags. The same case on 50925c7 failed exactly one of 18 assertions: its final status was skip. Both new-head repetitions pass. The earlier nested-row before/after regression also remains green, and the PFS-only worker case stays independent of NoRetry.
Method: the five local Shepherd go-mini-* Crook assertions on Gin with PR-branch dependency injection and debug logs, each twice. The timeout preset sets Go's explicit one-second timeout against a five-second body. Crook removed all fixtures and restored module/dependency files. This supersedes the review for 50925c7.
Additional crash-policy acceptance checks passed: 18 normal cases 16.045s, actual race 31.516s, minimum Go 1.25.11 27.655s; normal/race unit controls cover legacy manual fallback, mutable-tag isolation, first-write behavior and concurrent close/crash. Vet, strict source-frame/inventory checks, dependency boundaries and all maintained-source manifests pass. Prior complete actual-race exclusion coverage passed 513.092s; the repository's official ten-group normal validation is running separately on this exact revision.
No unaddressed Shepherd-review findings. Production Datadog UI was not checked; this verifies real instrumented framework execution through Mockdog intake. Platform CI and Datadog code review remain separate gates. Local full-root Mocha and signal-test failures were independently reproduced unchanged on published baselines and are not counted as passing tests. The monolithic local integration command reached its global time budget without failed integration cases; full coverage is being run through the existing official groups without changing inventory or per-group limits.
There was a problem hiding this comment.
A controlled native-subtest panic in a process retry drops completed NoRetry declarations from the worker result, allowing them to execute again and losing their original events.
🤖 Bits Code Review · Commit 492b588
|
@autotest Review |
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with independent review of expectations and raw runtime evidence.
Revision: ccb6c493bf8257000fb08b8d69a6f23e1e6e46aa
Environment: macOS arm64, Go 1.27.3, Gin playground; official release builder with bundled Mini. All 12 source checkouts and tested binaries match this exact signed commit, vcs.modified=false. SDK 65148c8b23d01ab8d21526ce2438b625a326746d, Orchestrion 1.14.0.
| Case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| Early worker panic / original NoRetry failure | 20261105-140138.595728000-4761-1 |
20261105-140347.613083000-12066-1 |
15/15 PASS |
| Protected timeout finalization | 20261105-140158.964468000-5958-1 |
20261105-140403.355553000-12667-1 |
18/18 PASS |
| Nested callback row exclusions | 20261105-140220.317728000-7623-1 |
20261105-140420.064841000-13347-1 |
17/17 PASS |
| PFS-only callback in quarantined method, race/process | 20261105-140243.680442000-9911-1 |
20261105-140437.476817000-13776-1 |
15/15 PASS |
| Native Go exclusions | 20261105-140305.516139000-10715-1 |
20261105-140456.232778000-14420-1 |
17/17 PASS |
| Testify exclusions | 20261105-140328.266963000-11407-1 |
20261105-140513.960277000-14918-1 |
17/17 PASS |
All 198 assertions passed across six cases, each repeated from fresh state. Every case requires intentional command exit 1, a failing session, exact event counts and correct annotation tags. The management cases additionally require retained disabled/quarantine/ATF assignments and absence of forbidden retry or ATF-completion tags. Crook itself exits 0 only when the expected product failure and all intake assertions match.
The new early-panic case first discovers and fails a protected NoRetry callback in a process retry, then an ordinary child panics. Both fresh runs retain the exact original error retained protected first failure, its single failing event and final status, both exclusion flags and no retry tags. Independent inspection of the actual stashed counter files confirms three root attempts and one physical excluded callback execution (attempt=xxx, critical=x) in each run. The published 492b588 baseline instead executed it twice (critical=xx), replaced the failure with a pass and falsely returned command exit 0: four of the same 15 assertions failed.
Independent review checked raw parsed tests/sessions, original error metadata, management HTTP responses, reports, debug logs, build identity, stashed counters and fixture restoration. All six local Shepherd go-mini-* Crook assertions used PR-branch dependency injection. Crook removed the fixtures, restored module/dependency files and stashed/removed managed state between repetitions. This supersedes the Shepherd review for 492b588.
Additional worker-terminal acceptance checks passed: 17 Mini cases plus 10 native controls, normal 18.281s, actual race 46.069s, Go 1.25.11 21.163s. Admission-latch concurrency units pass normally and under race; full vet, strict source-frame checks, unchanged integration inventory, all maintained-source manifests, dependency boundaries, 14 shard tests and root lint pass. CLI helper regressions preserve exact environment/ownership checks with coverage and ordinary startup output; quiet/noisy variants and existing version/help checks pass with race and coverage enabled.
No unaddressed Shepherd-review findings. Scope: real instrumented framework execution through Mockdog intake; production Datadog UI was not checked. Exact-head platform CI and Datadog code review are separate gates. Full local root tests retain the previously reproduced unchanged Mocha environment failure and are not counted as passing. The complete two-test annotation matrix also passed with actual race instrumentation on this exact revision in 548.997s, covering native/off/on cleanup provenance, worker terminals, management, cache, EFD/ATR/ATF, payload and capability-fallback controls.
There was a problem hiding this comment.
A race between terminal result publication and cleanup registration in the process-retry path for disabled+ATF roots causes ProtectedDescendantFailure to remain false, allowing packageFailed to suppress a genuine protected failure and report a spurious success. The fix requires propagating child protection before committing the terminal result.
🤖 Bits Code Review · Commit ccb6c49
|
@autotest Review |
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with independent review of expectations and raw runtime evidence.
Revision: febe7c38fb3a114b4e940010e919500c2af64b72
Environment: macOS arm64, Go 1.27.3, Gin playground; official release builder with bundled Mini. All 18 source checkouts and tested binaries match this exact signed commit, vcs.modified=false. SDK 7ea7c9015443c9b9e00e310bdbcd61f7f33686d0; Orchestrion 1.14.0.
| Case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| PFS-only native worker terminal | 20261105-144310.503989000-72547-1 |
20261105-144611.315133000-80521-1 |
24/24 PASS |
| PFS-only parallel irreversible failure | 20261105-144332.757514000-74390-1 |
20261105-144630.957280000-81073-1 |
24/24 PASS |
| Native cleanup cancellation control | 20261105-144354.137183000-76417-1 |
20261105-144647.342285000-81509-1 |
21/21 PASS |
| Early panic / original NoRetry failure | 20261105-144411.395749000-76933-1 |
20261105-144702.570404000-81924-1 |
15/15 PASS |
| Protected timeout finalization | 20261105-144429.174272000-77606-1 |
20261105-144719.728226000-82480-1 |
18/18 PASS |
| Nested callback row exclusions | 20261105-144447.650281000-78169-1 |
20261105-144737.647725000-82931-1 |
17/17 PASS |
| PFS-only callback in quarantined method, race/process | 20261105-144503.863092000-78626-1 |
20261105-144753.947946000-83310-1 |
15/15 PASS |
| Native Go exclusions | 20261105-144524.626665000-79223-1 |
20261105-144813.079654000-84231-1 |
17/17 PASS |
| Testify exclusions | 20261105-144548.841728000-79980-1 |
20261105-144829.515015000-84780-1 |
17/17 PASS |
All 336 assertions passed across nine cases, each repeated from fresh state. Eight positive cases require intentional command exit 1 and failing sessions. The native cleanup cancellation control requires command exit 0 and a passing session. Exact event counts, assignment metadata, applicable annotation tags and forbidden retry/error tags are checked per case. Crook exits 0 only when its expected product outcome and intake assertions match.
The three new worker cases independently cover the latest Datadog finding and its concurrency/cancellation boundaries:
- Protected panic: a disabled+ATF root discovers a PFS-only callback during a worker retry. Its exact panic remains visible, and final family status and session stay failed despite the root assignments.
- Irreversible parallel failure: the protected callback returns from
t.Errorbefore closing a readiness channel and remaining blocked. An ordinary sibling waits for that channel before panicking. Explicit native-parallel=2guarantees admission. The already committed protected failure keeps the family blocking even without callback cleanup. - Native cancellation: cleanup SkipNow cancels the body panic. The retry is raw PASS/final SKIP, retains disabled/ATF assignments and
attempt_to_fix_passed=false, and has no panic error tags. The command/session stay successful. A premature raw-panic protection marker cannot pass this control.
Both repetitions of each new case have exactly two root events and no synthetic child event, preserving the existing native worker reporting capability. Actual stashed counters are attempt=xx, critical=x. None adds a NoRetry annotation. On published ccb6c49, the same positive cases falsely returned command 0/final skip/passing session (three of 24 assertions failed each). The cancellation case kept command success but reported a failed retry and stale testmain_exit_conflict errors (three of 21 assertions failed).
The earlier NoRetry case also retains its original failed callback event/error without re-execution: both actual counter stashes are attempt=xxx, critical=x. The 492b588 baseline instead reran it and replaced its failure with a pass. Protected timeout, callback ownership, true-only exclusion tags, retained management assignments and direct native/Testify cases remain green.
Independent review checked all raw reports, parsed tests/sessions, error metadata, management HTTP responses, debug logs, build identity, counter stashes and fixture restoration. These are nine local Shepherd go-mini-* Crook cases with PR-branch injection. Crook removed fixtures, restored module/dependency files and stashed/removed managed state between runs. This supersedes the Shepherd review for ccb6c49.
Additional focused acceptance checks passed: normal 42.294s, actual race 77.462s, Go 1.25.11 42.856s, including the native seven-terminal oracle, PFS/native/ordinary concurrent controls, queued/self-parallel lifecycle, permitted ATR recovery and the prior history/admission matrix. Vet, strict source-frame checks, complete unchanged integration inventory, source manifests, dependency boundaries, 14 shard tests and root lint pass.
No unaddressed Shepherd-review findings. Scope: real instrumented framework execution through Mockdog test-cycle intake; production Datadog UI and telemetry were not checked. Exact-head platform CI and Datadog code review remain separate gates. Full local root tests retain the independently reproduced unchanged Mocha environment failure and are not counted as passing. The complete two-test annotation matrix also passed with actual race instrumentation on this exact revision in 579.570s, including native lifecycle/cancellation, irreversible concurrent failures, retry history, active admission, management and payload controls.
There was a problem hiding this comment.
More details
The exact base-to-head change has no confirmed reportable regression: the specialist review found no actionable findings, and the human discussion contains no unresolved objection to this revision. Static coverage measured 49 reviewed files and one deliberately skipped generated manifest.
🤖 Bits Code Review · Commit febe7c3
There was a problem hiding this comment.
A quarantined parent can still receive test.final_status=skip after a protected child fails if a timeout or background panic interrupts normal finalization.
🤖 Bits Code Review · Commit febe7c3
|
@autotest Review |
There was a problem hiding this comment.
In in-process execution, a protected child test that calls t.Error and blocks never delivers its failure marker to the quarantined parent before finalization, so failForCrash incorrectly reports the parent's status as skip rather than fail. The fix must propagate the trusted failure to active ancestors at the point the failure is observed, not at finalizer time, since a blocked callback may never reach the finalizer.
🤖 Bits Code Review · Commit 5285ff1
calvinbayer
left a comment
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with independent review of expectations and raw runtime evidence.
Revision: 5285ff11d8f04a9c7e89399fb747a020ab81eaf9
Environment: macOS arm64, Go 1.27.3, Gin playground; official release builder with bundled Mini. All 20 source checkouts and tested binaries match this exact signed commit, vcs.modified=false. SDK 7ea7c9015443c9b9e00e310bdbcd61f7f33686d0; Orchestrion 1.14.0.
| Case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| Quarantined parent crash after protected child | 20261105-152827.325258000-27882-1 |
20261105-153148.369134000-35339-1 |
24/24 PASS |
| PFS-only native worker terminal | 20261105-152851.657095000-29728-1 |
20261105-153211.366991000-35930-1 |
24/24 PASS |
| PFS-only parallel irreversible failure | 20261105-152910.506475000-30555-1 |
20261105-153230.167456000-36387-1 |
24/24 PASS |
| Native cleanup cancellation control | 20261105-152929.334445000-31081-1 |
20261105-153248.864211000-37313-1 |
21/21 PASS |
| Early panic / original NoRetry failure | 20261105-152951.691997000-31796-1 |
20261105-153310.845591000-37799-1 |
15/15 PASS |
| Protected timeout finalization | 20261105-153009.977605000-32398-1 |
20261105-153329.552506000-38249-1 |
18/18 PASS |
| Nested callback row exclusions | 20261105-153031.278921000-33162-1 |
20261105-153347.221568000-39576-1 |
17/17 PASS |
| PFS-only callback in quarantined method, race/process | 20261105-153050.644166000-33755-1 |
20261105-153405.611826000-40380-1 |
15/15 PASS |
| Native Go exclusions | 20261105-153112.461396000-34374-1 |
20261105-153424.249682000-41183-1 |
17/17 PASS |
| Testify exclusions | 20261105-153130.324018000-34832-1 |
20261105-153442.164883000-41773-1 |
17/17 PASS |
All 384 assertions passed across ten cases, each repeated from fresh state. Nine positive cases require intentional command exit 1 and failing sessions. The native cleanup cancellation control requires command exit 0 and a passing session. Exact event counts, assignment metadata, applicable annotation tags and forbidden retry/error tags are checked per case. Crook exits 0 only when its expected product outcome and intake assertions match.
The latest parent-crash case first completes a failing PFS-only child beneath an unannotated quarantined parent, then reaches Go's native timeout. Both actual child and parent events remain FAIL/finalFAIL while retaining Q assignments; the parent does not acquire a synthetic PFS or NoRetry annotation. Actual stashed counters are child=x, after-child=x, proving completion before the crash. Published febe7c3 failed exactly one of the same 24 assertions: its parent final status was SKIP. Normal command failure, child result, assignment tags and timeout metadata already matched. Both fresh new-head repetitions pass.
The three worker cases also cover the prior Datadog finding and its concurrency/cancellation boundaries:
- Protected panic: a disabled+ATF root discovers a PFS-only callback during a worker retry. Its exact panic remains visible, and final family status and session stay failed despite the root assignments.
- Irreversible parallel failure: the protected callback returns from
t.Errorbefore closing a readiness channel and remaining blocked. An ordinary sibling waits for that channel before panicking. Explicit native-parallel=2guarantees admission. The already committed protected failure keeps the family blocking even without callback cleanup. - Native cancellation: cleanup SkipNow cancels the body panic. The retry is raw PASS/final SKIP, retains disabled/ATF assignments and
attempt_to_fix_passed=false, and has no panic error tags. The command/session stay successful. A premature raw-panic protection marker cannot pass this control.
Both repetitions of each of those three worker cases have exactly two root events and no synthetic child event, preserving the existing native worker reporting capability. Actual stashed counters are attempt=xx, critical=x. None adds a NoRetry annotation. On published ccb6c49, the same positive cases falsely returned command 0/final skip/passing session (three of 24 assertions failed each). The cancellation case kept command success but reported a failed retry and stale testmain_exit_conflict errors (three of 21 assertions failed).
The earlier NoRetry case also retains its original failed callback event/error without re-execution: both actual counter stashes are attempt=xxx, critical=x. The 492b588 baseline instead reran it and replaced its failure with a pass. Protected timeout, callback ownership, true-only exclusion tags, retained management assignments and direct native/Testify cases remain green.
Independent review checked all raw reports, parsed tests/sessions, error metadata, management HTTP responses, debug logs, build identity, counter stashes and fixture restoration. These are ten local Shepherd go-mini-* Crook cases with PR-branch injection. Crook removed fixtures, restored module/dependency files and stashed/removed managed state between runs. This supersedes the Shepherd review for febe7c3.
Additional focused acceptance checks passed: normal 55.908s, actual race 67.065s, Go 1.25.11 60.897s, including parent timeout/background-panic and supported retained ATF histories, the trusted event-fact lifecycle and atomic metadata binding, native terminal/cancellation controls, PFS/native/ordinary concurrency, queue/recovery and the prior history/admission matrix. Vet, strict source-frame checks, complete unchanged integration inventory, source manifests, dependency boundaries, 14 shard tests and root lint pass.
No unaddressed Shepherd-review findings. Scope: real instrumented framework execution through Mockdog test-cycle intake; production Datadog UI and telemetry were not checked. Exact-head platform CI and Datadog code review remain separate gates. Full local root tests retain the independently reproduced unchanged Mocha environment failure and are not counted as passing. The complete two-test annotation matrix also passed with actual race instrumentation on this exact revision in 609.732s, including parent crash/ATF history and all earlier native lifecycle, concurrency, retry, cache and payload controls.
…rotected failures
5285ff1 to
dabeaf7
Compare
|
@autotest Review |
There was a problem hiding this comment.
More details
Targeted runner, runtime, and integration validation supports the declaration-exclusion behavior. The full compatibility matrix remains unverified, and production telemetry was intentionally excluded.
🤖 Bits Code Review · Commit dabeaf7
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with independent review of expectations and raw runtime evidence.
Revision: dabeaf7ef5a18c663298c0afd028b027032e3d53
Environment: macOS arm64, Go 1.27.3, Gin playground; official release builder with bundled Mini. All 22 source checkouts and tested binaries match this exact signed commit, vcs.modified=false. SDK 1bdbf936737ab9184afd16295e4377feb58e9a52, 7ea7c9015443c9b9e00e310bdbcd61f7f33686d0; injected Orchestrion v1.14.0, with Mini selecting its bundled runtime (orchestrion=false).
| Case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| Active protected child timeout | 20261105-161719.156978000-96778-1 |
20261105-162138.209971000-7121-1 |
24/24 PASS |
| Quarantined parent crash after protected child | 20261105-161748.997323000-97897-1 |
20261105-162158.846224000-7686-1 |
24/24 PASS |
| PFS-only native worker terminal | 20261105-161811.527108000-98540-1 |
20261105-162220.100184000-8315-1 |
24/24 PASS |
| PFS-only parallel irreversible failure | 20261105-161834.057621000-99373-1 |
20261105-162243.484815000-9407-1 |
24/24 PASS |
| Native cleanup cancellation control | 20261105-161858.863245000-625-1 |
20261105-162302.397541000-10026-1 |
21/21 PASS |
| Early panic / original NoRetry failure | 20261105-161926.658132000-1761-1 |
20261105-162322.151897000-10726-1 |
15/15 PASS |
| Protected timeout finalization | 20261105-161946.938250000-2601-1 |
20261105-162343.805983000-11489-1 |
18/18 PASS |
| Nested callback row exclusions | 20261105-162008.207459000-3417-1 |
20261105-162407.558158000-13950-1 |
17/17 PASS |
| PFS-only callback in quarantined method, race/process | 20261105-162031.184911000-4077-1 |
20261105-162426.646336000-14569-1 |
15/15 PASS |
| Native Go exclusions | 20261105-162057.072743000-5658-1 |
20261105-162455.340804000-16817-1 |
17/17 PASS |
| Testify exclusions | 20261105-162120.278508000-6702-1 |
20261105-162519.590051000-17957-1 |
17/17 PASS |
All 432 assertions passed across eleven cases, each repeated from fresh state. Ten positive cases require intentional command exit 1 and failing sessions. The native cleanup cancellation control requires command exit 0 and a passing session. Exact event counts, assignment metadata, applicable annotation tags and forbidden retry/error tags are checked per case. Crook exits 0 only when its expected product outcome and intake assertions match.
The new active-child timeout case records parent entry, child entry and return from t.Error exactly once; after-child is absent. Both open child and parent events remain FAIL/finalFAIL through native timeout. The parent retains Q without acquiring PFS or NoRetry. This exercises failure propagation before child finalization, matching production finding 4481813292.
The earlier parent-crash case first completes a failing PFS-only child beneath an unannotated quarantined parent, then reaches Go's native timeout. Both actual child and parent events remain FAIL/finalFAIL while retaining Q assignments; the parent does not acquire a synthetic PFS or NoRetry annotation. Actual stashed counters are child=x, after-child=x, proving completion before the crash. Published febe7c3 failed exactly one of the same 24 assertions: its parent final status was SKIP. Normal command failure, child result, assignment tags and timeout metadata already matched. Both fresh new-head repetitions pass.
The three worker cases also cover the prior Datadog finding and its concurrency/cancellation boundaries:
- Protected panic: a disabled+ATF root discovers a PFS-only callback during a worker retry. Its exact panic remains visible, and final family status and session stay failed despite the root assignments.
- Irreversible parallel failure: the protected callback returns from
t.Errorbefore closing a readiness channel and remaining blocked. An ordinary sibling waits for that channel before panicking. Explicit native-parallel=2guarantees admission. The already committed protected failure keeps the family blocking even without callback cleanup. - Native cancellation: cleanup SkipNow cancels the body panic. The retry is raw PASS/final SKIP, retains disabled/ATF assignments and
attempt_to_fix_passed=false, and has no panic error tags. The command/session stay successful. A premature raw-panic protection marker cannot pass this control.
Both repetitions of each of those three worker cases have exactly two root events and no synthetic child event, preserving the existing native worker reporting capability. Actual stashed counters are attempt=xx, critical=x. None adds a NoRetry annotation. On published ccb6c49, the same positive cases falsely returned command 0/final skip/passing session (three of 24 assertions failed each). The cancellation case kept command success but reported a failed retry and stale testmain_exit_conflict errors (three of 21 assertions failed).
The earlier NoRetry case also retains its original failed callback event/error without re-execution: both actual counter stashes are attempt=xxx, critical=x. The 492b588 baseline instead reran it and replaced its failure with a pass. Protected timeout, callback ownership, true-only exclusion tags, retained management assignments and direct native/Testify cases remain green.
Independent review checked all raw reports, parsed tests/sessions, error metadata, management HTTP responses, debug logs, build identity, counter stashes and fixture restoration. These are eleven local Shepherd go-mini-* Crook cases with PR-branch injection. Crook removed fixtures, restored module/dependency files and stashed/removed managed state between runs. This supersedes the Shepherd review for 5285ff1.
Additional focused acceptance checks passed: normal 110.093s, actual race 157.361s, Go 1.25.11 116.743s, including parent timeout/background-panic and supported retained ATF histories, the trusted event-fact lifecycle and atomic metadata binding, native terminal/cancellation controls, PFS/native/ordinary concurrency, queue/recovery and the prior history/admission matrix. Vet, strict source-frame checks, complete unchanged integration inventory, source manifests, dependency boundaries, 14 shard tests and root lint pass.
No unaddressed Shepherd-review findings. Scope: real instrumented framework execution through Mockdog test-cycle intake; production Datadog UI and telemetry were not checked. Exact-head platform CI and Datadog code review remain separate gates. Full local root tests retain the independently reproduced unchanged Mocha environment failure and are not counted as passing. The complete two-test annotation race matrix also passed on this exact source in 665.139s (PARITY_TEST_MODE=race).
There was a problem hiding this comment.
Annotated generic callback functions can make the generated instrumentation source fail to compile: the runner emits a direct registration for an uninstantiated generic function instead of using compiled-name registration. The PR discussion addresses generic receiver declarations, but this separate callback path remains uncovered by the added runner tests.
🤖 Bits Code Review · Commit dabeaf7
|
@autotest Review |
There was a problem hiding this comment.
More details
This PR adds declaration-level Go test exclusions for native Go and Testify execution, preserving management assignments, retries, process boundaries, and native lifecycle behavior. The earlier P2 for annotated generic callbacks generating invalid uninstantiated registrations is fixed in this commit with runtime-name registration and internal/integration coverage; no open findings remain. Production-aware validation was not applicable to this static follow-up.
🤖 Bits Code Review · Commit b41d939
There was a problem hiding this comment.
More details
Focused verification covered declaration-local retry retention and failure protection through native cleanup, with no reportable defects identified. Production behavior and the full compatibility matrix remain unvalidated.
🤖 Bits Code Review · Commit b41d939
There was a problem hiding this comment.
Shepherd E2E test review: PASS
Tested by: Shepherd Agent (autonomous QA for Datadog Test Optimization), with independent review of expectations and raw runtime evidence.
Revision: b41d939f4b3247466a99265059850b45cdbf1a39
Environment: macOS arm64, Go 1.27.3, Gin playground; official release builder with bundled Mini. All 24 source checkouts and tested binaries match this exact signed commit, vcs.modified=false. Shepherd dependency checkout dd-trace-go@1bdbf936737ab9184afd16295e4377feb58e9a52; Mini uses the SDK vendored in this PR (base 870449702d0a0cea26a6223eefe2f0a198069d79, adapted hashes verified); injected Orchestrion v1.14.0, with Mini selecting its bundled runtime (orchestrion=false).
| Case | First run | Fresh repeat | Assertions per run |
|---|---|---|---|
| Generic callback declaration exclusions | 20261105-164656.611442000-51410-1 |
20261105-165120.484436000-61475-1 |
32/32 PASS |
| Active protected child timeout | 20261105-164726.184061000-52905-1 |
20261105-165143.407949000-62075-1 |
24/24 PASS |
| Quarantined parent crash after protected child | 20261105-164755.075642000-53919-1 |
20261105-165207.720802000-62925-1 |
24/24 PASS |
| PFS-only native worker terminal | 20261105-164819.385561000-54737-1 |
20261105-165231.512007000-63747-1 |
24/24 PASS |
| PFS-only parallel irreversible failure | 20261105-164838.665041000-55486-1 |
20261105-165250.425008000-64398-1 |
24/24 PASS |
| Native cleanup cancellation control | 20261105-164856.440096000-57111-1 |
20261105-165309.103329000-65063-1 |
21/21 PASS |
| Early panic / original NoRetry failure | 20261105-164919.309040000-57958-1 |
20261105-165331.448178000-65628-1 |
15/15 PASS |
| Protected timeout finalization | 20261105-164938.673977000-58445-1 |
20261105-165349.863135000-66358-1 |
18/18 PASS |
| Nested callback row exclusions | 20261105-164956.460523000-58890-1 |
20261105-165407.098546000-66796-1 |
17/17 PASS |
| PFS-only callback in quarantined method, race/process | 20261105-165019.607354000-59690-1 |
20261105-165425.728433000-67427-1 |
15/15 PASS |
| Native Go exclusions | 20261105-165043.551529000-60367-1 |
20261105-165444.549424000-68044-1 |
17/17 PASS |
| Testify exclusions | 20261105-165102.864818000-60878-1 |
20261105-165506.990834000-68523-1 |
17/17 PASS |
All 496 assertions passed across twelve cases, each repeated from fresh state. Eleven positive cases require intentional command exit 1 and failing sessions. The native cleanup cancellation control requires command exit 0 and a passing session. Exact event counts, assignment metadata, applicable annotation tags and forbidden retry/error tags are checked per case. Crook exits 0 only when its expected product outcome and intake assertions match.
The new generic callback case places the annotated generic declaration in an ordinary Go source file and invokes its concrete instantiation from a native test in a separate file. Generated registration compiles, and the actual child retains both exclusion tags and disabled/quarantine/ATF assignments without retries. The ordinary parent retains its three failing ATR attempts (two retries), with one final blocking result and no synthetic annotations. Exact counts require four test events and two source suites; actual counter stashes must be parent=xxx, child=x. The corresponding native program compiled and ran before the fix, while the original instrumented build failed on an uninstantiated generated registration (staging review finding 4482397937).
The active-child timeout case records parent entry, child entry and return from t.Error exactly once; after-child is absent. Both open child and parent events remain FAIL/finalFAIL through native timeout. The parent retains Q without acquiring PFS or NoRetry. This exercises failure propagation before child finalization, matching production finding 4481813292.
The earlier parent-crash case first completes a failing PFS-only child beneath an unannotated quarantined parent, then reaches Go's native timeout. Both actual child and parent events remain FAIL/finalFAIL while retaining Q assignments; the parent does not acquire a synthetic PFS or NoRetry annotation. Actual stashed counters are child=x, after-child=x, proving completion before the crash. Published febe7c3 failed exactly one of the same 24 assertions: its parent final status was SKIP. Normal command failure, child result, assignment tags and timeout metadata already matched. Both fresh new-head repetitions pass.
The three worker cases also cover the prior Datadog finding and its concurrency/cancellation boundaries:
- Protected panic: a disabled+ATF root discovers a PFS-only callback during a worker retry. Its exact panic remains visible, and final family status and session stay failed despite the root assignments.
- Irreversible parallel failure: the protected callback returns from
t.Errorbefore closing a readiness channel and remaining blocked. An ordinary sibling waits for that channel before panicking. Explicit native-parallel=2guarantees admission. The already committed protected failure keeps the family blocking even without callback cleanup. - Native cancellation: cleanup SkipNow cancels the body panic. The retry is raw PASS/final SKIP, retains disabled/ATF assignments and
attempt_to_fix_passed=false, and has no panic error tags. The command/session stay successful. A premature raw-panic protection marker cannot pass this control.
Both repetitions of each of those three worker cases have exactly two root events and no synthetic child event, preserving the existing native worker reporting capability. Actual stashed counters are attempt=xx, critical=x. None adds a NoRetry annotation. On published ccb6c49, the same positive cases falsely returned command 0/final skip/passing session (three of 24 assertions failed each). The cancellation case kept command success but reported a failed retry and stale testmain_exit_conflict errors (three of 21 assertions failed).
The earlier NoRetry case also retains its original failed callback event/error without re-execution: both actual counter stashes are attempt=xxx, critical=x. The 492b588 baseline instead reran it and replaced its failure with a pass. Protected timeout, callback ownership, true-only exclusion tags, retained management assignments and direct native/Testify cases remain green.
Independent review checked all raw reports, parsed tests/sessions, error metadata, management HTTP responses, debug logs, build identity, counter stashes and fixture restoration. These are twelve local Shepherd go-mini-* Crook cases with PR-branch injection. Crook removed fixtures, restored module/dependency files and stashed/removed managed state between runs. This supersedes the Shepherd review for dabeaf7.
Additional focused generic-declaration acceptance checks passed: normal 19.950s, actual race 26.558s, Go 1.25.11 20.134s, covering local/external/imported callbacks, multiple type parameters, unused declarations, ordinary callback controls and generic receivers. Vet, strict source-frame checks, unchanged 191-test integration inventory, source manifests, dependency boundaries, 14 shard tests and root lint pass.
No unaddressed Shepherd-review findings. Scope: real instrumented framework execution through Mockdog test-cycle intake; production Datadog UI and telemetry were not checked. Exact-head platform CI and Datadog code review remain separate gates. Full local root tests retain the independently reproduced unchanged Mocha environment failure and are not counted as passing. The complete two-test annotation race matrix also passed on this exact source in 638.450s (PARITY_TEST_MODE=race).
|
Validation complete for
PR remains open and unmerged. |
What
Adds per-declaration test automation exclusions to DDTest's native Go Mini instrumentation for Go testing and Testify:
//ddtest:prevent_failure_suppressionkeeps failures blocking despite quarantine or disabling.//ddtest:no_auto_retryexcludes the declaration from ATR, EFD and ATF, including retries of enclosing suites.testopt.RegisterTestExclusionsprovides the initialization-time declaration API used by generated registrations.The CLI registers annotations from compiler-selected sources, including overlays, cgo files, local/external test packages, imported helpers, aliases, and generic/parenthesized receivers. Runtime execution retains original backend assignments, source attribution and native selection while applying each declaration's effective policy. Mutable span tags do not activate exclusions.
Why
Critical tests need to remain blocking and execute only as permitted by their declaration. The exclusion must survive suite retries and process boundaries without changing unrelated tests, generated cases, native repetitions or framework lifecycle behavior.
E2E testing
Use a mock intake with independently controlled ATR/EFD, management assignments and TIA responses. Run each policy separately, together, and absent:
-count=2. Switch derived suites sharing one base declaration at the same native path: reuse that declaration's first result while preserving reflected source/suite attribution. Explicit overrides and unrelated declarations remain independent, including ancestor switches beneath ordinary process-worker wrappers. Put an annotated callback inside an unannotated Testify method, then create a native row from that callback. Assign disabled/quarantine/ATF to the row and require both inherited exclusion tags, all assignment tags, one failing row execution and a failing command/session (four test events for root, method, callback and row). Switch two annotated parent callbacks using the same native row path/callback: the second declaration must execute and retain its own failing result.go test -count=1andddtest instrument go test -count=1for body plus one/multiple cleanup panics, including a cleanup that registers another cleanup. Check the same diagnostic markers and native exit outcome, with original source attribution. Include cleanup Goexit cancellation and a later cleanup panic. Also run an assigned disabled/quarantined protected test through a Go timeout and a background panic with the CLI's--panic-hookmode. Require one executed failing event withtest.final_status=fail, retained assignment tags, and a failing command; a skipped final result is incorrect. Use ordinary and NoRetry-only negative controls: quarantine retains its existing skipped final result; disabled+ATF with NoRetry stays skipped without executing the body. Verify mutable span tags cannot activate protection. Complete a failing PFS-only child under an unannotated quarantined parent, then time out the parent or trigger a background panic with --panic-hook. Also keep a child active after t.Error until timeout or background panic: its normal native Fail hook must protect the active parent before the child finalizes. Both child and parent must retain failed final results and assignment tags; the parent must not gain a synthetic PFS annotation. Cover trusted fact delivery before/after event binding, retry-event resets and concurrent close/crash, while ordinary NoRetry-only or recovered failures keep their existing suppression behavior. Retain native command outcomes and cleanup order; do not admit queued parallel children after an unsuppressed body terminal or release scheduler slots twice.The CLI helper regressions also run with ordinary startup noise and coverage output before their result. Verify exact dispatch environment state, then kill the probe owner and require its OS lock, session, configuration copy and owned probe to be recovered while the customer test remains intact. Coverage and instrumentation remain enabled.
The executable regression matrix is in
ddto/integration/mini_exclusions*_test.go; runner registration units and maintained runtime protocol tests cover declaration and transport boundaries. Reproduce with:Automated timings, full compatibility progress and exact-head CI/review results are recorded in the validation comments. No merge or release is included.