Skip to content

Commit 92198b6

Browse files
andreiborzaandreiborza
authored
chore(node-core): Fix node-core integration test assertions (#19219)
We excluded running the node-core integration tests [from ci here](#16930), but we never ensured they were ran elsewhere. This PR fixes the failing tests and adds a dedicated step in build.yml. As for impact: The failing tests were mostly centered around asserting the correct sdk name, and two failing tests around usage of top-level await and one around adding headers to outgoing http requests. None of these are critical, the last one working when scoped to node 22 (expected, since our http integration on node-core uses diagnostic channels that are only available on node 22). This is not an issue in our node sdk because we use otel's http integration for that. Closes #19220 (added automatically)
1 parent c7e6c6b commit 92198b6

File tree

6 files changed

+93
-35
lines changed

6 files changed

+93
-35
lines changed

‎.github/workflows/build.yml‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ jobs:
191191
changed_node_integration:
192192
${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected,
193193
'@sentry-internal/node-integration-tests') }}
194+
changed_node_core_integration:
195+
${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected,
196+
'@sentry-internal/node-core-integration-tests') }}
194197
changed_remix:
195198
${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected,
196199
'@sentry/remix') }}
@@ -772,6 +775,55 @@ jobs:
772775
token: ${{ secrets.GITHUB_TOKEN }}
773776
directory: dev-packages/node-integration-tests
774777

778+
job_node_core_integration_tests:
779+
name:
780+
Node (${{ matrix.node }})${{ (matrix.typescript && format(' (TS {0})', matrix.typescript)) || '' }} Node-Core
781+
Integration Tests
782+
needs: [job_get_metadata, job_build]
783+
if: needs.job_build.outputs.changed_node_core_integration == 'true' || github.event_name != 'pull_request'
784+
runs-on: ubuntu-24.04
785+
timeout-minutes: 15
786+
strategy:
787+
fail-fast: false
788+
matrix:
789+
node: [18, 20, 22, 24]
790+
typescript:
791+
- false
792+
include:
793+
# Only check typescript for latest version (to streamline CI)
794+
- node: 24
795+
typescript: '3.8'
796+
steps:
797+
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
798+
uses: actions/checkout@v6
799+
with:
800+
ref: ${{ env.HEAD_COMMIT }}
801+
- name: Set up Node
802+
uses: actions/setup-node@v6
803+
with:
804+
node-version: ${{ matrix.node }}
805+
- name: Restore caches
806+
uses: ./.github/actions/restore-cache
807+
with:
808+
dependency_cache_key: ${{ needs.job_build.outputs.dependency_cache_key }}
809+
810+
- name: Overwrite typescript version
811+
if: matrix.typescript == '3.8'
812+
run: node ./scripts/use-ts-3_8.js
813+
working-directory: dev-packages/node-core-integration-tests
814+
815+
- name: Run integration tests
816+
working-directory: dev-packages/node-core-integration-tests
817+
run: yarn test
818+
819+
- name: Parse and Upload Coverage
820+
if: cancelled() == false
821+
continue-on-error: true
822+
uses: getsentry/codecov-action@main
823+
with:
824+
token: ${{ secrets.GITHUB_TOKEN }}
825+
directory: dev-packages/node-core-integration-tests
826+
775827
job_cloudflare_integration_tests:
776828
name: Cloudflare Integration Tests
777829
needs: [job_get_metadata, job_build]
@@ -1144,6 +1196,7 @@ jobs:
11441196
job_deno_unit_tests,
11451197
job_node_unit_tests,
11461198
job_node_integration_tests,
1199+
job_node_core_integration_tests,
11471200
job_cloudflare_integration_tests,
11481201
job_browser_playwright_tests,
11491202
job_browser_loader_tests,

‎dev-packages/node-core-integration-tests/suites/public-api/logs/test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ describe('logger public API', () => {
2828
},
2929
'server.address': {
3030
type: 'string',
31-
value: 'M6QX4Q5HKV.local',
31+
value: expect.any(String),
3232
},
3333
'user.name': {
3434
type: 'string',
@@ -65,7 +65,7 @@ describe('logger public API', () => {
6565
},
6666
'server.address': {
6767
type: 'string',
68-
value: 'M6QX4Q5HKV.local',
68+
value: expect.any(String),
6969
},
7070
'user.name': {
7171
type: 'string',
@@ -102,7 +102,7 @@ describe('logger public API', () => {
102102
},
103103
'server.address': {
104104
type: 'string',
105-
value: 'M6QX4Q5HKV.local',
105+
value: expect.any(String),
106106
},
107107
'user.name': {
108108
type: 'string',

‎dev-packages/node-core-integration-tests/suites/tracing/requests/traceparent/scenario-http.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ function makeHttpRequest(url) {
1616
});
1717
}
1818

19-
await Sentry.startSpan({ name: 'outer' }, async () => {
19+
Sentry.startSpan({ name: 'outer' }, async () => {
2020
await makeHttpRequest(`${process.env.SERVER_URL}/api/v1`);
2121
});

‎dev-packages/node-core-integration-tests/suites/tracing/requests/traceparent/test.ts‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createTestServer } from '@sentry-internal/test-utils';
22
import { describe, expect } from 'vitest';
3+
import { conditionalTest } from '../../../../utils';
34
import { createEsmAndCjsTests } from '../../../../utils/runner';
45

56
describe('outgoing traceparent', () => {
@@ -29,29 +30,33 @@ describe('outgoing traceparent', () => {
2930
});
3031
});
3132

32-
createEsmAndCjsTests(__dirname, 'scenario-http.mjs', 'instrument.mjs', (createRunner, test) => {
33-
test('outgoing http requests should get traceparent headers', async () => {
34-
expect.assertions(5);
33+
// This test requires Node.js 22+ because it depends on the 'http.client.request.created'
34+
// diagnostic channel for baggage header propagation, which only exists since Node 22.12.0+ and 23.2.0+
35+
conditionalTest({ min: 22 })('node >=22', () => {
36+
createEsmAndCjsTests(__dirname, 'scenario-http.mjs', 'instrument.mjs', (createRunner, test) => {
37+
test('outgoing http requests should get traceparent headers', async () => {
38+
expect.assertions(5);
3539

36-
const [SERVER_URL, closeTestServer] = await createTestServer()
37-
.get('/api/v1', headers => {
38-
expect(headers['baggage']).toEqual(expect.any(String));
39-
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})-1$/));
40-
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0');
41-
expect(headers['traceparent']).toEqual(expect.stringMatching(/^00-([a-f\d]{32})-([a-f\d]{16})-01$/));
42-
})
43-
.start();
40+
const [SERVER_URL, closeTestServer] = await createTestServer()
41+
.get('/api/v1', headers => {
42+
expect(headers['baggage']).toEqual(expect.any(String));
43+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f\d]{32})-([a-f\d]{16})-1$/));
44+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0');
45+
expect(headers['traceparent']).toEqual(expect.stringMatching(/^00-([a-f\d]{32})-([a-f\d]{16})-01$/));
46+
})
47+
.start();
4448

45-
await createRunner()
46-
.withEnv({ SERVER_URL })
47-
.expect({
48-
transaction: {
49-
// we're not too concerned with the actual transaction here since this is tested elsewhere
50-
},
51-
})
52-
.start()
53-
.completed();
54-
closeTestServer();
49+
await createRunner()
50+
.withEnv({ SERVER_URL })
51+
.expect({
52+
transaction: {
53+
// we're not too concerned with the actual transaction here since this is tested elsewhere
54+
},
55+
})
56+
.start()
57+
.completed();
58+
closeTestServer();
59+
});
5560
});
5661
});
5762
});

‎dev-packages/node-core-integration-tests/suites/winston/test.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('winston integration', () => {
2121
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
2222
'sentry.release': { value: '1.0.0', type: 'string' },
2323
'sentry.environment': { value: 'test', type: 'string' },
24-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
24+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
2525
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
2626
'server.address': { value: expect.any(String), type: 'string' },
2727
},
@@ -36,7 +36,7 @@ describe('winston integration', () => {
3636
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
3737
'sentry.release': { value: '1.0.0', type: 'string' },
3838
'sentry.environment': { value: 'test', type: 'string' },
39-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
39+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
4040
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
4141
'server.address': { value: expect.any(String), type: 'string' },
4242
},
@@ -65,7 +65,7 @@ describe('winston integration', () => {
6565
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
6666
'sentry.release': { value: '1.0.0', type: 'string' },
6767
'sentry.environment': { value: 'test', type: 'string' },
68-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
68+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
6969
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
7070
'server.address': { value: expect.any(String), type: 'string' },
7171
},
@@ -80,7 +80,7 @@ describe('winston integration', () => {
8080
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
8181
'sentry.release': { value: '1.0.0', type: 'string' },
8282
'sentry.environment': { value: 'test', type: 'string' },
83-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
83+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
8484
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
8585
'server.address': { value: expect.any(String), type: 'string' },
8686
},
@@ -95,7 +95,7 @@ describe('winston integration', () => {
9595
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
9696
'sentry.release': { value: '1.0.0', type: 'string' },
9797
'sentry.environment': { value: 'test', type: 'string' },
98-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
98+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
9999
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
100100
'server.address': { value: expect.any(String), type: 'string' },
101101
},
@@ -110,7 +110,7 @@ describe('winston integration', () => {
110110
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
111111
'sentry.release': { value: '1.0.0', type: 'string' },
112112
'sentry.environment': { value: 'test', type: 'string' },
113-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
113+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
114114
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
115115
'server.address': { value: expect.any(String), type: 'string' },
116116
},
@@ -139,7 +139,7 @@ describe('winston integration', () => {
139139
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
140140
'sentry.release': { value: '1.0.0', type: 'string' },
141141
'sentry.environment': { value: 'test', type: 'string' },
142-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
142+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
143143
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
144144
'server.address': { value: expect.any(String), type: 'string' },
145145
},
@@ -154,7 +154,7 @@ describe('winston integration', () => {
154154
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
155155
'sentry.release': { value: '1.0.0', type: 'string' },
156156
'sentry.environment': { value: 'test', type: 'string' },
157-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
157+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
158158
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
159159
'server.address': { value: expect.any(String), type: 'string' },
160160
},
@@ -169,7 +169,7 @@ describe('winston integration', () => {
169169
'sentry.origin': { value: 'auto.log.winston', type: 'string' },
170170
'sentry.release': { value: '1.0.0', type: 'string' },
171171
'sentry.environment': { value: 'test', type: 'string' },
172-
'sentry.sdk.name': { value: 'sentry.javascript.node', type: 'string' },
172+
'sentry.sdk.name': { value: 'sentry.javascript.node-core', type: 'string' },
173173
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
174174
'server.address': { value: expect.any(String), type: 'string' },
175175
foo: { value: 'bar', type: 'string' },

‎dev-packages/node-core-integration-tests/utils/assertions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export function assertEnvelopeHeader(actual: Envelope[0], expected: Partial<Enve
9191
event_id: expect.any(String),
9292
sent_at: expect.any(String),
9393
sdk: {
94-
name: 'sentry.javascript.node',
94+
name: 'sentry.javascript.node-core',
9595
version: SDK_VERSION,
9696
},
9797
...expected,

0 commit comments

Comments
 (0)