Skip to content

test(node-integration): Skip flaky tedious tests#15798

Merged
Lms24 merged 1 commit into
developfrom
lms/test-skip-tedious-tests
Mar 24, 2025
Merged

test(node-integration): Skip flaky tedious tests#15798
Lms24 merged 1 commit into
developfrom
lms/test-skip-tedious-tests

Conversation

@Lms24
Copy link
Copy Markdown
Member

@Lms24 Lms24 commented Mar 24, 2025

This test now failed twice in a row for me and I've seen it fail in countless PRs. Skipping it for now but we should prioritize #15579 to eventually unflake and unskip them again.

@Lms24 Lms24 marked this pull request as ready for review March 24, 2025 13:03
@Lms24 Lms24 requested review from lforst and mydea March 24, 2025 13:04
@Lms24 Lms24 self-assigned this Mar 24, 2025
@Lms24 Lms24 enabled auto-merge (squash) March 24, 2025 13:07
@lforst
Copy link
Copy Markdown
Contributor

lforst commented Mar 24, 2025

you could say the tests were ... tedious 👉😎👉

@Lms24 Lms24 merged commit c622c8a into develop Mar 24, 2025
@Lms24 Lms24 deleted the lms/test-skip-tedious-tests branch March 24, 2025 13:11
mydea added a commit that referenced this pull request May 18, 2026
…s` (#20961)

## Summary

Migrates the remaining integration-specific tracing tests under
`dev-packages/node-integration-tests/suites/tracing/` to use the
`createEsmAndCjsTests` helper, which auto-runs each scenario in both ESM
and CJS mode from a single `.mjs` source.

For each folder:
- `scenario*.js` → `scenario*.mjs` (ESM imports)
- Sentry.init extracted into a separate `instrument.mjs`
- `test.ts` switched to `createEsmAndCjsTests(__dirname, scenarioPath,
instrumentPath, callback, options?)`
- `failsOnEsm: true` added only where CJS passed and ESM legitimately
fails (i.e. instrumentation only patches the CJS module shape)

## Migrated folders

| Folder | Notes |
|---|---|
| `lru-memoizer` | `failsOnEsm: true` — OTel
`instrumentation-lru-memoizer` only patches the CJS function-shaped
export. |
| `mongodb` | mongodb v3 is CJS-only and has no named ESM exports — uses
`import mongodb from 'mongodb'` + destructure. |
| `mongoose` | Straight migration. |
| `mysql2` | Straight migration (docker). |
| `mysql` | `failsOnEsm: true` on all 3 scenarios — mysql v2 is
CJS-only. Three scenarios kept (`withConnect`, `withoutCallback`,
`withoutConnect`). |
| `postgres` | 3 scenarios. `ignoreConnectSpans` reuses the default
scenario but with a separate `instrument-ignoreConnect.mjs`. `pg-native`
scenario migrated as-is (still relies on local `setupCommand: 'yarn'` to
build the native bindings). |
| `postgresjs` | Consolidated previously-parallel `.cjs`/`.mjs`
scenarios into a single `.mjs` each. The `wait-for-postgres.js` helper
is still required from the scenarios via
`createRequire(import.meta.url)` (revert of the .mjs-helper conversion
approach — see notes below). The error stacktrace assertion uses
`expect.stringMatching(/postgres(\.cjs)?\.src:connection/)` so the same
expectation matches both modes. |
| `redis` | Straight migration (docker). |
| `redis-cache` | 2 scenarios (`ioredis`, `redis-4`). Each scenario has
its own `instrument-*.mjs` because they pass different `cachePrefixes`
to `redisIntegration`. |
| `redis-dc` | Uses `await import('redis-5')` inside `run()` to preserve
the comment-documented ordering where the DC subscriber must be
registered (via `Promise.resolve().then`) before node-redis eagerly
creates its native TracingChannels on require/import. |
| `tedious` | Migrated but kept `describe.skip` (the test was previously
skipped as flaky in #15798). Manually verified both modes pass when
un-skipped. |
| `apollo-graphql` | 3 scenarios (query, mutation, error). Shared
`apollo-server.mjs` helper copied into the tmp dir via `copyPaths:
['apollo-server.mjs']` and loaded with `await
import('./apollo-server.mjs')` from each scenario. |
| `apollo-graphql/useOperationNameForRootSpan` | 6 scenarios. Reuses the
parent `apollo-server.mjs` via `../../apollo-server.mjs` (the scenarios
run from `useOperationNameForRootSpan/tmp_xxx/`, so this resolves back
to the apollo-graphql folder). Scenarios use `Sentry.getClient().tracer`
instead of capturing the client returned by `Sentry.init` (since init
now lives in `instrument.mjs`). |

## Out of scope

The following folders under `tracing/` are still using `createRunner`
directly — they are not integrations for a specific package but general
tracing-behavior tests, so they're intentionally left alone:

- `meta-tags*`, `meta-tags-twp*`
- `sample-rate-propagation/*`, `sample-rand-propagation`
- `tracePropagationTargets/*`
- `traceid-recycling*`
- `dsc-txn-name-update`
- `http-client-spans/*`
- `maxSpans`, `envelope-header/*`, `linking`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants