test: Add build-time tests for node & bundler plugins - #23618
Conversation
size-limit report 📦
|
87c2113 to
66d93fa
Compare
66d93fa to
cf5f1d1
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cf5f1d1. Configure here.
… plugin excludes runtime injection Build the entry twice: plain webpack (runtime channel injection bundled by default) and with `sentryWebpackPlugin` (build-time instrumentation), which defaults `excludeChannelInjection` to true and tree-shakes the runtime injection out. assert.mjs verifies the marker is present in the plain build and absent in the plugin build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> test(e2e): Minify node-webpack builds so runtime-injection tree-shaking runs The dead `if (useChannelInjection)` branch is only pruned by the minifier, so `minimize: false` left the runtime injection in the bundle. Verified: with `minimize: true`, the real `sentryWebpackPlugin` excludes the runtime injection while a plain build keeps it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> test(e2e): Add node-vite and node-rollup runtime-injection-exclusion tests Mirror node-webpack for Vite and Rollup: build the entry plain and with the respective Sentry plugin, and assert the runtime channel-injection marker is present in the plain build but tree-shaken out with the plugin (build-time instrumentation defaults `excludeChannelInjection` to true). Verified locally that both bundlers tree-shake the runtime injection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> test(e2e): Use esnext target for node-vite so top-level await builds Vite defaults to a browser target that rejects the entry's top-level await; set a node target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> test(e2e): Add node-esbuild build-time instrumentation test Build the entry plain and with `sentryEsbuildPlugin`. esbuild's single-pass tree-shaking keeps the (now dead) runtime injection in the bundle unlike webpack/vite/rollup, so this app only asserts the plain build bundles the runtime injection and the plugin build succeeds; the runtime-behavior side is covered separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> fixes...
The plugin-build assertion searched a whitespace-stripped bundle for `g.bundler=g.bundler||[]`, but `ORCHESTRION_BUNDLER_MARKER_BANNER` initializes the set with `new Set()` (stripped: `newSet()`), so the marker never matched and the `plugin` assertion always failed. Rolldown additionally emits a `/* @__PURE__ */` annotation inside the banner, which whitespace-stripping leaves intact. Match the real banner: strip block comments as well as whitespace before searching, and look for `g.bundler=g.bundler||newSet()`. Verified against the plugin bundles of all five bundlers (webpack, esbuild, vite, rollup, rolldown). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3d0dbfe to
99cf169
Compare
| process.exit(1); | ||
| } | ||
| // eslint-disable-next-line no-console | ||
| console.log('All bundle assertions passed.'); |
There was a problem hiding this comment.
l: this logic is duplicated for every test in this PR, should we consolidate this?
| // A distinctive slice of the orchestrion banner that the bundler plugin's build-time code transform | ||
| // prepends to the entry chunk (see `ORCHESTRION_BUNDLER_MARKER_BANNER` in `@sentry/server-utils`). | ||
| // It is emitted only when the plugin's build-time instrumentation runs, so it tells a `plugin` build | ||
| // apart from a `plain` one. Before matching we strip block comments and whitespace, because bundlers | ||
| // format the injected banner differently — Rolldown pretty-prints it and inserts a `/* @__PURE__ */` | ||
| // annotation. The banner initializes the set with `new Set()`, hence the stripped `newSet()` form. | ||
| const BUILD_TIME_TRANSFORM_MARKER = 'g.bundler=g.bundler||newSet()'; |
There was a problem hiding this comment.
L: similar to Nico's comment - this is also duplicated in all the tests.
|
the tests in this PR are generally refactored/enhanced in a follow up PR stacked here, so I think it is fine to merge this for now :) |

This adds test for webpack, vite, rollup, rolldown, esbuild to verify that build-time injection with orchestrion generally works.