From 8a9e8333db38bae93f47f23b61fa7e556d95f902 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 12 Oct 2026 15:05:45 +0200 Subject: [PATCH] test(node-integration): Stop exporting `NODE_VERSION` and `RUNTIME` from the test utils Suites limit tests to a Node version or a runtime with `supports`, so the raw values stay private to the test utils. The last suite that checked `NODE_VERSION` itself now uses `test.runIf(supports({ min: 24 }))`, which skips the same Node versions. A Bugbot rule flags suites that check the Node version or the runtime in other ways. Co-Authored-By: Claude Opus 5.5 --- .cursor/BUGBOT.md | 1 + .../suites/thread-blocked-native/test.ts | 9 ++------- dev-packages/node-integration-tests/utils/index.ts | 4 ++-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index f70b4bc51da0..a394e1130d84 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -93,6 +93,7 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu - Flag usage of `expect.objectContaining` and other relaxed assertions, when a test expects something NOT to be included in a payload but there's no respective assertion. - Flag usage of conditionals in one test and recommend splitting up the test for the different paths. - Flag usage of loops testing multiple scenarios in one test and recommend using `(it)|(test).each` instead. +- In `dev-packages/node-integration-tests/suites/`, flag tests that check the Node version or the runtime themselves, for example with `process.versions`, `process.env.RUNTIME`, `ctx.skip()`, or a version or runtime condition in `skipIf` / `runIf`. Recommend `test.runIf(supports({ ... }))` or `describe.runIf(supports({ ... }))` from `utils/index.ts` instead. - Apply the `@sentry/conventions` constants rule from above to test files too. Test assertions on span attributes and ops must use the constants, not string literals. - Flag tests that are likely to introduce flakes. In our case this usually means we wait for some telemetry requests sent from an SDK. Patterns to look out for: - Only waiting for a request, after an action is performed. Instead, start waiting, perform action, await request promise. diff --git a/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts b/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts index a07cc781b3d2..bf39f9d7c1ad 100644 --- a/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts +++ b/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts @@ -1,7 +1,7 @@ import { join } from 'node:path'; import type { Event } from '@sentry/core'; import { afterAll, describe, expect, test } from 'vitest'; -import { NODE_VERSION } from '../../utils/index'; +import { supports } from '../../utils/index'; import { cleanupChildProcesses, createRunner } from '../../utils/runner'; function EXCEPTION(thread_id = '0', fn = 'longWork') { @@ -240,12 +240,7 @@ describe('Thread Blocked Native', { timeout: 30_000 }, () => { .completed(); }); - test('Capture scope via AsyncLocalStorage', async ctx => { - if (NODE_VERSION < 24) { - ctx.skip(); - return; - } - + test.runIf(supports({ min: 24 }))('Capture scope via AsyncLocalStorage', async () => { const instrument = join(__dirname, 'instrument.mjs'); await createRunner(__dirname, 'isolated.mjs') .withMockSentryServer() diff --git a/dev-packages/node-integration-tests/utils/index.ts b/dev-packages/node-integration-tests/utils/index.ts index b3a1dfd8153d..01f6f4bf039b 100644 --- a/dev-packages/node-integration-tests/utils/index.ts +++ b/dev-packages/node-integration-tests/utils/index.ts @@ -3,13 +3,13 @@ import { parseSemver } from '@sentry/core'; import { execFileSync } from 'child_process'; import type * as http from 'http'; -export const NODE_VERSION = parseSemver(process.versions.node).major || 0; +const NODE_VERSION = parseSemver(process.versions.node).major || 0; /** * The runtime that runs the scenarios (`node`, `bun` or `deno`), from the `RUNTIME` env var. */ export type Runtime = 'cloudflare' | 'node' | 'bun' | 'deno'; -export const RUNTIME = (process.env.RUNTIME || 'node') as Runtime; +const RUNTIME = (process.env.RUNTIME || 'node') as Runtime; // Vitest runs on Node, so it cannot read `Bun.version` of the `bun` binary that runs the scenarios. const BUN_VERSION = RUNTIME === 'bun' ? execFileSync('bun', ['--version'], { encoding: 'utf8' }).trim() : undefined;