test(guardrails): remove redundant runtime-check unit test#9098
Merged
pabloerhard merged 1 commit intoJun 26, 2026
Conversation
packages/dd-trace/test/guardrails/index.spec.js duplicated existing integration coverage. init.spec.js's "when node version is too recent" context already exercises the incompatible-runtime bailout — plus the telemetry, debug logs, and DD_INJECT_FORCE override that the unit test stubbed to no-ops — by setting pkg.nodeMaxMajor = NODE_MAJOR. The unit test was added in #9033 only to satisfy a Codecov patch-coverage flag on lines that PR added to the bailout block (integration tests run in sandboxes and don't count toward nyc). It relied on proxyquire intercepting an in-function require() of package.json, which is environment-dependent: it failed on the v5.110.0 release proposal on Node 26 even though the real bailout — verified by the passing init.spec.js integration test on that same run — works correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
Overall package sizeSelf size: 6.37 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🔗 Commit SHA: a85442e | Docs | Datadog PR Page | Give us feedback! |
rochdev
marked this pull request as ready for review
June 26, 2026 19:17
BenchmarksBenchmark execution time: 2026-06-26 19:22:04 Comparing candidate commit a85442e in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2253 metrics, 33 unstable metrics.
|
pabloerhard
approved these changes
Jun 26, 2026
dd-octo-sts Bot
pushed a commit
that referenced
this pull request
Jun 26, 2026
packages/dd-trace/test/guardrails/index.spec.js duplicated existing integration coverage. init.spec.js's "when node version is too recent" context already exercises the incompatible-runtime bailout — plus the telemetry, debug logs, and DD_INJECT_FORCE override that the unit test stubbed to no-ops — by setting pkg.nodeMaxMajor = NODE_MAJOR. The unit test was added in #9033 only to satisfy a Codecov patch-coverage flag on lines that PR added to the bailout block (integration tests run in sandboxes and don't count toward nyc). It relied on proxyquire intercepting an in-function require() of package.json, which is environment-dependent: it failed on the v5.110.0 release proposal on Node 26 even though the real bailout — verified by the passing init.spec.js integration test on that same run — works correctly. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Merged
pabloerhard
pushed a commit
that referenced
this pull request
Jun 26, 2026
packages/dd-trace/test/guardrails/index.spec.js duplicated existing integration coverage. init.spec.js's "when node version is too recent" context already exercises the incompatible-runtime bailout — plus the telemetry, debug logs, and DD_INJECT_FORCE override that the unit test stubbed to no-ops — by setting pkg.nodeMaxMajor = NODE_MAJOR. The unit test was added in #9033 only to satisfy a Codecov patch-coverage flag on lines that PR added to the bailout block (integration tests run in sandboxes and don't count toward nyc). It relied on proxyquire intercepting an in-function require() of package.json, which is environment-dependent: it failed on the v5.110.0 release proposal on Node 26 even though the real bailout — verified by the passing init.spec.js integration test on that same run — works correctly. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes
packages/dd-trace/test/guardrails/index.spec.js.Why
This unit test duplicated existing — and stronger — integration coverage, and was added only to satisfy a Codecov patch-coverage flag.
The behavior is already covered by
integration-tests/init.spec.js. Itscontext('when node version is too recent')block setspkg.nodeMaxMajor = NODE_MAJORand asserts the tracer does not initialize, plus the abort telemetry, theincompatible_runtimedebug log output, and theDD_INJECT_FORCEoverride — everything the unit test stubbed to no-ops and more.The unit test only existed for a coverage metric. #9033 refactored the bailout block in
guardrails/index.js, adding new lines. Codecov flagged those new lines as uncovered patch coverage, because the only thing exercising that block — the integration test above — runs in a sandbox and doesn't count toward nyc. The unit test was added to clear that flag. AGENTS.md explicitly discourages this ("Don't add redundant unit tests solely to improve coverage numbers").It was also fragile. It relied on proxyquire intercepting an in-function
require('../../../../package.json'), which is environment-dependent. It failed on the v5.110.0 release proposal on Node 26 — yetPlatform / integration (node-latest)(which runsinit.spec.json Node 26) passed on that same run, proving the real bailout path is correct and the failure was a mocking artifact, not a bug.Coverage note
.codecov.ymlhasproject: offandpatchonly measures added lines on PRs, so this pure deletion does not trip a Codecov gate. The bailout lines remain covered byinit.spec.js(just not nyc-visible). If a future change to the bailout block re-triggers a patch-coverage flag, the right fix is to make integration coverage count or to exclude the block in.codecov.yml— not to re-add a proxyquire unit test.The
unit-guardrailsjob still runstelemetry.spec.js, so the mocha glob remains non-empty (verified locally: 9 passing).