Conversation
bm1549
added
the
AI Generated
Contributor
Overall package sizeSelf size: 4.95 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.0 | 81.15 kB | 815.98 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
This comment has been minimized.
This comment has been minimized.
bm1549
added a commit
that referenced
this pull request
Mar 6, 2026
Move the eslint-prefer-assert-object-contains rule and its auto-fix to a separate PR (#7693) for independent review and merge. Co-Authored-By: Claude Opus 4.6 <[email protected]>
2 tasks
BenchmarksBenchmark execution time: 2026-03-10 15:53:44 Comparing candidate commit 343b852 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 230 metrics, 30 unstable metrics. |
BridgeAR
requested changes
Mar 6, 2026
Member
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
I am in favor of having such a rule, while this seems a bit too eager right now 😅
With a less eager check, the overall changes would also be less. I originally intended to write such a rule by myself after migrating more entries in a few smaller intermediate PRs.
Contributor
Author
|
Addressed @BridgeAR's feedback: Rule changes (
Test file reverts:
|
Member
Claude misunderstood a comment :) It works just fine for arrays. |
bm1549
changed the title
BridgeAR
reviewed
Mar 6, 2026
| @@ -128,9 +129,13 @@ describe('AppsecFsPlugin', () => { | |||
| let store = appsecFsPlugin._onFsOperationStart() | |||
|
|
|||
| assert.ok(Object.hasOwn(store, 'fs')) | |||
Member
There was a problem hiding this comment.
Suggested change
| assert.ok(Object.hasOwn(store, 'fs')) |
| @@ -242,11 +251,15 @@ describe('vulnerability-analyzer', () => { | |||
| const location = { path: 'file-name.js', line: 15 } | |||
| const vulnerability = vulnerabilityAnalyzer._createVulnerability(type, evidence, undefined, location) | |||
| assert.notStrictEqual(vulnerability, null) | |||
Member
There was a problem hiding this comment.
Suggested change
| assert.notStrictEqual(vulnerability, null) |
| @@ -229,11 +234,15 @@ describe('vulnerability-analyzer', () => { | |||
| const spanId = 888 | |||
| const vulnerability = vulnerabilityAnalyzer._createVulnerability(type, evidence, spanId) | |||
| assert.notStrictEqual(vulnerability, null) | |||
Member
There was a problem hiding this comment.
Suggested change
| assert.notStrictEqual(vulnerability, null) |
| @@ -216,11 +217,15 @@ describe('vulnerability-analyzer', () => { | |||
| const spanId = 888 | |||
| const vulnerability = vulnerabilityAnalyzer._createVulnerability(type, evidence, spanId, location) | |||
| assert.notStrictEqual(vulnerability, null) | |||
Member
There was a problem hiding this comment.
Suggested change
| assert.notStrictEqual(vulnerability, null) |
| @@ -227,12 +240,16 @@ describe('Plugin', () => { | |||
| agent | |||
| .assertSomeTraces(traces => { | |||
Member
There was a problem hiding this comment.
We could use assertFirstTraceSpan directly. That would be a bit nicer.
| status: 'error', | ||
| }) | ||
|
|
||
| assertObjectContains(payload.tags, ['error_type:error type']) |
Member
There was a problem hiding this comment.
This can be combined with the above.
Comment on lines
256
to
257
| assert.strictEqual(formattedEvent.subject.type, undefined) | ||
| assert.strictEqual(formattedEvent.subject.attributes, undefined) |
Member
There was a problem hiding this comment.
Can these not be included with the above?
Comment on lines
366
to
368
| assert.strictEqual(trace.meta['service.name'], undefined) | ||
| assert.strictEqual(trace.meta['span.type'], undefined) | ||
| assert.strictEqual(trace.meta['resource.name'], undefined) |
Member
There was a problem hiding this comment.
Could any of these be included in the above?
Member
There was a problem hiding this comment.
I believe the undefined check here is actually an not existence check? Could we just change these cases to assert.ok(!Object.hasOwn(foo, 'property'))? That way the intent is clearer and all other undefined checks can be inlined into the assertObjectContains calls.
Comment on lines
557
to
559
| assert.strictEqual(trace.meta['nested.A'], undefined) | ||
| assert.strictEqual(trace.meta['nested.A.B'], undefined) | ||
| assert.strictEqual(trace.meta['nested.A.num'], undefined) |
| abc: '', | ||
| }) | ||
| assert.ok(!('' in carrier)) | ||
| assert.strictEqual(carrier.valid, '') |
Contributor
Author
|
Hi @BridgeAR! I've applied all the feedback from your review. Here's a summary of what was changed: Removed redundant pre-checks before
Converted
Merged split
Moved
I skipped three locations where the suggestion would require asserting CI is fully green. Thanks for the detailed review! |
BridgeAR
previously approved these changes
Mar 6, 2026
Member
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
Ignoring undefined does not seem correct by claude. This nevertheless improves a lot, so LGTM
bm1549
force-pushed
the
brian.marks/eslint-prefer-assert-object-contains
branch
from
d59f34c to
e8fffff
Compare
BridgeAR
reviewed
Mar 9, 2026
Member
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
LGTM, just left a few nits and it needs a rebase
integration-tests/jest/jest.spec.js
Outdated
Comment on lines
248
to
249
| assert.ok(test) | ||
|
|
Member
There was a problem hiding this comment.
Nit
Suggested change
| assert.ok(test) |
Comment on lines
55
to
56
| const span = payload[0][0] | ||
|
|
Member
There was a problem hiding this comment.
Nit: we could move this into assertObjectContains(payload, [[{...}]])
That way there is also no need to check for array anymore for payload and payload[0]. Only the length check would still be needed.
Comment on lines
98
to
109
| assert.deepStrictEqual(spanEvent.content.meta, { | ||
| bar: 'baz', | ||
| }) | ||
| assertObjectContains(spanEvent.content.metrics, { | ||
| float: 1.23456712345, | ||
| negativefloat: -1.23456789, | ||
| bigfloat: 12345678.9, | ||
| bignegativefloat: -12345678.9, | ||
| }) | ||
|
|
||
| assert.strictEqual(spanEvent.content.metrics.positive, 123456712345) | ||
| assert.strictEqual(spanEvent.content.metrics.negative, -123456712345) |
Member
There was a problem hiding this comment.
Nit: these could all be included above.
Comment on lines
366
to
368
| assert.strictEqual(trace.meta['service.name'], undefined) | ||
| assert.strictEqual(trace.meta['span.type'], undefined) | ||
| assert.strictEqual(trace.meta['resource.name'], undefined) |
Member
There was a problem hiding this comment.
I believe the undefined check here is actually an not existence check? Could we just change these cases to assert.ok(!Object.hasOwn(foo, 'property'))? That way the intent is clearer and all other undefined checks can be inlined into the assertObjectContains calls.
Add an ESLint rule that detects 3+ consecutive assert.strictEqual() calls on properties of the same root object and auto-fixes them to use assertObjectContains(). Enable the rule as 'error' for test files and fix all existing violations (~290 auto-fixed). The auto-fix: - Builds nested object literals from property access paths - Handles both dot notation and bracket notation with string literals - Skips groups with non-literal computed properties (variables) - Inserts the assertObjectContains import if not already present - Computes relative require paths to integration-tests/helpers Co-Authored-By: Claude Opus 4.6 <[email protected]>
Object.hasOwn only checks own properties, missing prototype getters (e.g. Histogram.min, Histogram.max). Using 'key in Object(actual)' supports inherited properties and getters.
When the expected value is undefined, the original code used assert.strictEqual(obj.key, undefined) which passes whether the property is absent or explicitly set to undefined. The key-in check should be skipped for undefined values to preserve this behavior.
…ct-contains
Address BridgeAR's review feedback:
- Rule now skips groups where any path uses a numeric computed index
(e.g. arr[0], payload[1]) since these indicate array indexing and
assertObjectContains does not work correctly when numeric string keys
are used as object properties
- Rule now skips groups where the terminal property is 'length' or 'size'
since these collection size properties do not work reliably with
partialDeepStrictEqual on arrays/maps/sets
- Revert helpers/index.js semantic change ('in' operator and undefined
guard) back to original Object.hasOwn check
- Revert all 65 test files that had invalid transformations (numeric
indices or length/size terminal properties in assertObjectContains)
- Re-run lint:fix with corrected rule; 10 files remain with valid
assertObjectContains transformations
- Add targeted test cases covering numeric index and length/size guards
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
When an expected property value is undefined, skip the Object.hasOwn check — this allows asserting that a property is absent or undefined (e.g. `path: undefined`) without requiring the property to exist on the actual object as an own property. The 'in' operator change that BridgeAR flagged is still reverted; only the undefined guard (needed for absent-property assertions in ~13 test files) is restored. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
assertObjectContains with Object.hasOwn fails on CountMetric class instances because `type` is a prototype property, not an own property. Revert to original separate assert.strictEqual calls. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Object.hasOwn is too strict for assertObjectContains — class instances like CountMetric, URL, and Histogram have properties on their prototypes (e.g. type, hostname, min/max) that are not own properties. Restore 'in' operator for the existence check, keeping the undefined guard. BridgeAR's specific concern (length/size checks on actual array/map/set objects falling through to the fallback) is already addressed by the ESLint rule fix that prevents those transformations. Also re-apply the namespaces.spec.js assertObjectContains transformation now that the helper works correctly with prototype properties. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…int rule
- Revert integration-tests/helpers/index.js to original Object.hasOwn check
(restores identical semantics to assert.partialDeepStrictEqual)
- Remove eslint-prefer-assert-object-contains rule and test files
- Revert eslint.config.mjs and CODEOWNERS to remove rule registration
- Fix transformed test files that were incompatible with Object.hasOwn:
- Revert url.spec.js, namespaces.spec.js, histogram.spec.js (prototype props)
- Replace `undefined`-valued assertObjectContains keys with separate
assert.strictEqual() calls in config, span_format, agentless-json,
opentelemetry/logs, opentelemetry/metrics, text_map, llmobs/sdk,
vulnerability-analyzer, extended-data-collection (3 files),
profiling/config, openfeature/writers/exposures
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…mpat) On Node.js < 22, assertObjectContains falls back to Object.hasOwn which only checks own properties, not prototype chain. Prototype getters and constructor-assigned-but-overridden properties fail this check. Reverted or surgically fixed assertObjectContains calls on: - PoissonProcessSamplingFilter (currentSamplingInstant, nextSamplingInstant, samplingInstantCount) - LLMObsSDK (enabled, _config) - LoggerProvider / MockedExporter (OTel class instances) - MeterProvider / MockedExporter (OTel class instances) - ExposuresWriter (writer._interval, _timeout, _bufferLimit) - DataStreams DataStreamsProcessor (hostname, enabled, env) - Profiling Config (v8ProfilerBugWorkaroundEnabled, etc.) - AppSec utils DatadogRaspAbortError (name, message, etc.) - SpanStatsProcessor (hostname, enabled, env) For logs.spec.js and metrics.spec.js, kept assertObjectContains on plain objects (log2/scope.logRecords entries, resourceAttrs built via forEach) while reverting the class instance calls. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
… instance URL.href is a prototype getter, not an own property, so Object.hasOwn fails on Node.js < 22 where assertObjectContains uses the fallback. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
- Remove redundant Object.hasOwn/notStrictEqual checks before assertObjectContains
- Merge split assertObjectContains calls on same object into single call
- Convert assertSomeTraces(traces => { const span = traces[0][0]; ... }) to assertFirstTraceSpan() in couchbase, amqplib, and vertexai plugins
- Expand assertObjectContains in jest/mocha integration tests to include more meta properties from surrounding assertions
- Move _dd.code_origin.type assertion into assertObjectContains block in code-origin spec
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
- Convert assertSomeTraces + traces[0][0] to assertFirstTraceSpan in: graphql (×2, callback form to preserve negative check), vertexai (4th case), mongodb-core/core, mongodb-core/mongodb (with inlined resource variable), undici, extended-data-collection express/fastify, body-parser - Include component: 'graphql' in assertObjectContains for graphql parse/validate spans - Merge assertObjectContains calls in config/index.spec.js (logLevel/logger/middlewareTracingEnabled + peerServiceMapping + plugins/port/... into single call) - Remove redundant assert.notStrictEqual(config.telemetry, undefined) in config spec - Include commitSHA/debug/dynamicInstrumentation into assertObjectContains in debugger/config spec - Merge two assertObjectContains in agentless-ci-visibility + include type/version from above - Include meta/metrics in assertObjectContains in agentless-json spec - Merge assertObjectContains(payload.tags, ...) into assertObjectContains(payload, ...) in llmobs - Include carrier.valid in assertObjectContains in tagger spec Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
- span_format.spec.js: change assert.strictEqual(x, undefined) to
assert.ok(!Object.hasOwn(meta, key)) for non-existence checks
- agentless-ci-visibility.spec.js: merge decodedTrace.version, metadata,
and spanEvent assertions into a single assertObjectContains(decodedTrace)
- azure-functions client.spec.js: use assertObjectContains(payload, [[{...}]])
and remove redundant Array.isArray checks
- mocha.spec.js: include ORIGIN_KEY, TEST_CODE_OWNERS, LIBRARY_VERSION
in failing test assertObjectContains (matching passing tests coverage)
- jest.spec.js: remove redundant assert.ok(test) (nit)
- exposures.spec.js: keep assert.strictEqual(X, undefined) - the writer
explicitly sets type/attributes as undefined properties (Object.hasOwn
returns true for them, so existence check would fail)
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
bm1549
force-pushed
the
brian.marks/eslint-prefer-assert-object-contains
branch
from
d86ed9c to
de09314
Compare
The current GitLab pipeline's CI job token lacks permissions to clone auto_inject.git (403). This empty commit triggers a fresh pipeline with a new token. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
BridgeAR
approved these changes
Mar 10, 2026
Member
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
LGTM, thank you for the improvement :)
I think this will end up more people using the helper because they see it in tests.
dd-octo-sts bot
pushed a commit
that referenced
this pull request
Mar 11, 2026
…ertObjectContains (#7693) * feat(eslint): add eslint-prefer-assert-object-contains with auto-fix Add an ESLint rule that detects 3+ consecutive assert.strictEqual() calls on properties of the same root object and auto-fixes them to use assertObjectContains(). Enable the rule as 'error' for test files and fix all existing violations (~290 auto-fixed). The auto-fix: - Builds nested object literals from property access paths - Handles both dot notation and bracket notation with string literals - Skips groups with non-literal computed properties (variables) - Inserts the assertObjectContains import if not already present - Computes relative require paths to integration-tests/helpers Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use 'in' operator instead of Object.hasOwn in assertObjectContains Object.hasOwn only checks own properties, missing prototype getters (e.g. Histogram.min, Histogram.max). Using 'key in Object(actual)' supports inherited properties and getters. * chore: add eslint-rules/ to CODEOWNERS for lang-platform-js * fix: skip key existence check when expected value is undefined When the expected value is undefined, the original code used assert.strictEqual(obj.key, undefined) which passes whether the property is absent or explicitly set to undefined. The key-in check should be skipped for undefined values to preserve this behavior. * fix(eslint): skip array indices and length/size in prefer-assert-object-contains Address BridgeAR's review feedback: - Rule now skips groups where any path uses a numeric computed index (e.g. arr[0], payload[1]) since these indicate array indexing and assertObjectContains does not work correctly when numeric string keys are used as object properties - Rule now skips groups where the terminal property is 'length' or 'size' since these collection size properties do not work reliably with partialDeepStrictEqual on arrays/maps/sets - Revert helpers/index.js semantic change ('in' operator and undefined guard) back to original Object.hasOwn check - Revert all 65 test files that had invalid transformations (numeric indices or length/size terminal properties in assertObjectContains) - Re-run lint:fix with corrected rule; 10 files remain with valid assertObjectContains transformations - Add targeted test cases covering numeric index and length/size guards Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: restore undefined guard in assertObjectContainsImpl When an expected property value is undefined, skip the Object.hasOwn check — this allows asserting that a property is absent or undefined (e.g. `path: undefined`) without requiring the property to exist on the actual object as an own property. The 'in' operator change that BridgeAR flagged is still reverted; only the undefined guard (needed for absent-property assertions in ~13 test files) is restored. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: revert namespaces.spec.js - CountMetric has prototype properties assertObjectContains with Object.hasOwn fails on CountMetric class instances because `type` is a prototype property, not an own property. Revert to original separate assert.strictEqual calls. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: restore 'in' operator for class instance prototype property checks Object.hasOwn is too strict for assertObjectContains — class instances like CountMetric, URL, and Histogram have properties on their prototypes (e.g. type, hostname, min/max) that are not own properties. Restore 'in' operator for the existence check, keeping the undefined guard. BridgeAR's specific concern (length/size checks on actual array/map/set objects falling through to the fallback) is already addressed by the ESLint rule fix that prevents those transformations. Also re-apply the namespaces.spec.js assertObjectContains transformation now that the helper works correctly with prototype properties. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: address BridgeAR feedback - revert helpers/index.js and drop ESLint rule - Revert integration-tests/helpers/index.js to original Object.hasOwn check (restores identical semantics to assert.partialDeepStrictEqual) - Remove eslint-prefer-assert-object-contains rule and test files - Revert eslint.config.mjs and CODEOWNERS to remove rule registration - Fix transformed test files that were incompatible with Object.hasOwn: - Revert url.spec.js, namespaces.spec.js, histogram.spec.js (prototype props) - Replace `undefined`-valued assertObjectContains keys with separate assert.strictEqual() calls in config, span_format, agentless-json, opentelemetry/logs, opentelemetry/metrics, text_map, llmobs/sdk, vulnerability-analyzer, extended-data-collection (3 files), profiling/config, openfeature/writers/exposures Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: revert assertObjectContains on class instances (Object.hasOwn compat) On Node.js < 22, assertObjectContains falls back to Object.hasOwn which only checks own properties, not prototype chain. Prototype getters and constructor-assigned-but-overridden properties fail this check. Reverted or surgically fixed assertObjectContains calls on: - PoissonProcessSamplingFilter (currentSamplingInstant, nextSamplingInstant, samplingInstantCount) - LLMObsSDK (enabled, _config) - LoggerProvider / MockedExporter (OTel class instances) - MeterProvider / MockedExporter (OTel class instances) - ExposuresWriter (writer._interval, _timeout, _bufferLimit) - DataStreams DataStreamsProcessor (hostname, enabled, env) - Profiling Config (v8ProfilerBugWorkaroundEnabled, etc.) - AppSec utils DatadogRaspAbortError (name, message, etc.) - SpanStatsProcessor (hostname, enabled, env) For logs.spec.js and metrics.spec.js, kept assertObjectContains on plain objects (log2/scope.logRecords entries, resourceAttrs built via forEach) while reverting the class instance calls. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: revert llmobs writers/base.spec.js - requestOptions.url is a URL instance URL.href is a prototype getter, not an own property, so Object.hasOwn fails on Node.js < 22 where assertObjectContains uses the fallback. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
Merged
simon-id
reviewed
Mar 11, 2026
packages/dd-trace/test/appsec/extended-data-collection.express.plugin.spec.js
Show resolved
Hide resolved
CarlesDD
pushed a commit
that referenced
this pull request
Mar 16, 2026
…ertObjectContains (#7693) * feat(eslint): add eslint-prefer-assert-object-contains with auto-fix Add an ESLint rule that detects 3+ consecutive assert.strictEqual() calls on properties of the same root object and auto-fixes them to use assertObjectContains(). Enable the rule as 'error' for test files and fix all existing violations (~290 auto-fixed). The auto-fix: - Builds nested object literals from property access paths - Handles both dot notation and bracket notation with string literals - Skips groups with non-literal computed properties (variables) - Inserts the assertObjectContains import if not already present - Computes relative require paths to integration-tests/helpers Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use 'in' operator instead of Object.hasOwn in assertObjectContains Object.hasOwn only checks own properties, missing prototype getters (e.g. Histogram.min, Histogram.max). Using 'key in Object(actual)' supports inherited properties and getters. * chore: add eslint-rules/ to CODEOWNERS for lang-platform-js * fix: skip key existence check when expected value is undefined When the expected value is undefined, the original code used assert.strictEqual(obj.key, undefined) which passes whether the property is absent or explicitly set to undefined. The key-in check should be skipped for undefined values to preserve this behavior. * fix(eslint): skip array indices and length/size in prefer-assert-object-contains Address BridgeAR's review feedback: - Rule now skips groups where any path uses a numeric computed index (e.g. arr[0], payload[1]) since these indicate array indexing and assertObjectContains does not work correctly when numeric string keys are used as object properties - Rule now skips groups where the terminal property is 'length' or 'size' since these collection size properties do not work reliably with partialDeepStrictEqual on arrays/maps/sets - Revert helpers/index.js semantic change ('in' operator and undefined guard) back to original Object.hasOwn check - Revert all 65 test files that had invalid transformations (numeric indices or length/size terminal properties in assertObjectContains) - Re-run lint:fix with corrected rule; 10 files remain with valid assertObjectContains transformations - Add targeted test cases covering numeric index and length/size guards Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: restore undefined guard in assertObjectContainsImpl When an expected property value is undefined, skip the Object.hasOwn check — this allows asserting that a property is absent or undefined (e.g. `path: undefined`) without requiring the property to exist on the actual object as an own property. The 'in' operator change that BridgeAR flagged is still reverted; only the undefined guard (needed for absent-property assertions in ~13 test files) is restored. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: revert namespaces.spec.js - CountMetric has prototype properties assertObjectContains with Object.hasOwn fails on CountMetric class instances because `type` is a prototype property, not an own property. Revert to original separate assert.strictEqual calls. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: restore 'in' operator for class instance prototype property checks Object.hasOwn is too strict for assertObjectContains — class instances like CountMetric, URL, and Histogram have properties on their prototypes (e.g. type, hostname, min/max) that are not own properties. Restore 'in' operator for the existence check, keeping the undefined guard. BridgeAR's specific concern (length/size checks on actual array/map/set objects falling through to the fallback) is already addressed by the ESLint rule fix that prevents those transformations. Also re-apply the namespaces.spec.js assertObjectContains transformation now that the helper works correctly with prototype properties. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: address BridgeAR feedback - revert helpers/index.js and drop ESLint rule - Revert integration-tests/helpers/index.js to original Object.hasOwn check (restores identical semantics to assert.partialDeepStrictEqual) - Remove eslint-prefer-assert-object-contains rule and test files - Revert eslint.config.mjs and CODEOWNERS to remove rule registration - Fix transformed test files that were incompatible with Object.hasOwn: - Revert url.spec.js, namespaces.spec.js, histogram.spec.js (prototype props) - Replace `undefined`-valued assertObjectContains keys with separate assert.strictEqual() calls in config, span_format, agentless-json, opentelemetry/logs, opentelemetry/metrics, text_map, llmobs/sdk, vulnerability-analyzer, extended-data-collection (3 files), profiling/config, openfeature/writers/exposures Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: revert assertObjectContains on class instances (Object.hasOwn compat) On Node.js < 22, assertObjectContains falls back to Object.hasOwn which only checks own properties, not prototype chain. Prototype getters and constructor-assigned-but-overridden properties fail this check. Reverted or surgically fixed assertObjectContains calls on: - PoissonProcessSamplingFilter (currentSamplingInstant, nextSamplingInstant, samplingInstantCount) - LLMObsSDK (enabled, _config) - LoggerProvider / MockedExporter (OTel class instances) - MeterProvider / MockedExporter (OTel class instances) - ExposuresWriter (writer._interval, _timeout, _bufferLimit) - DataStreams DataStreamsProcessor (hostname, enabled, env) - Profiling Config (v8ProfilerBugWorkaroundEnabled, etc.) - AppSec utils DatadogRaspAbortError (name, message, etc.) - SpanStatsProcessor (hostname, enabled, env) For logs.spec.js and metrics.spec.js, kept assertObjectContains on plain objects (log2/scope.logRecords entries, resourceAttrs built via forEach) while reverting the class instance calls. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix: revert llmobs writers/base.spec.js - requestOptions.url is a URL instance URL.href is a prototype getter, not an own property, so Object.hasOwn fails on Node.js < 22 where assertObjectContains uses the fallback. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[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
3 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 does this PR do?
Replaces 3+ consecutive
assert.strictEqual(obj.prop, value)calls on the same root object with a singleassertObjectContains(obj, { ... })call across ~50 test files.This was originally generated via an ESLint auto-fix rule (now dropped per reviewer feedback), then manually reviewed and corrected.
Motivation
assertObjectContainsproduces much better error output when assertions fail — showing the full expected/actual objects rather than one property at a time. This makes test failures easier to diagnose.What was changed
assert.strictEqualtoassertObjectContainsintegration-tests/helpers/index.jsunchanged — originalObject.hasOwnsemantics preservedWhat was NOT changed (kept as
assert.strictEqual)undefinedvalues (property absence) —Object.hasOwndoesn't match absent keysURL,Histogram,CountMetric, etc.)length/sizeproperty checksAdditional Notes
The
assertObjectContainshelper falls back to a custom partial-check implementation on Node.js < 22 (whereassert.partialDeepStrictEqualdoesn't exist), maintainingObject.hasOwnsemantics throughout.