Conversation
Contributor
size-limit report 📦
|
JPeer264
added this pull request to stack #24613
September 23, 2026 06:14
JPeer264
force-pushed
the
jp/node-suites-bun
branch
from
September 23, 2026 07:29
f544fac to
d5222d1
Compare
JPeer264
force-pushed
the
jp/node-suites-bun
branch
2 times, most recently
from
September 23, 2026 08:27
9099fef to
34ea3db
Compare
JPeer264
removed this pull request from stack #24613
September 23, 2026 09:51
JPeer264
added this pull request to stack #24633
September 23, 2026 09:51
JPeer264
force-pushed
the
jp/node-suites-bun
branch
from
September 23, 2026 10:02
34ea3db to
0a3ba84
Compare
JPeer264
force-pushed
the
jp/node-suites-bun
branch
from
September 23, 2026 10:04
0a3ba84 to
460db99
Compare
JPeer264
force-pushed
the
jp/node-suites-bun
branch
from
September 23, 2026 12:24
460db99 to
5df282e
Compare
JPeer264
force-pushed
the
jp/node-suites-bun
branch
4 times, most recently
from
September 23, 2026 16:19
1feee30 to
278df2a
Compare
JPeer264
removed this pull request from stack #24633
September 23, 2026 16:19
JPeer264
changed the base branch from
jp/node-suites-runtime-switch
to
jp/node-suites-bun-shared-runner
September 23, 2026 16:19
JPeer264
added this pull request to stack #24670
September 23, 2026 16:19
JPeer264
marked this pull request as ready for review
September 23, 2026 16:42
JPeer264
requested review from
isaacs and
mydea
and removed request for
a team
September 23, 2026 16:42
JPeer264
removed this pull request from stack #24670
September 23, 2026 17:26
JPeer264
added this pull request to stack #24671
September 23, 2026 17:31
isaacs
reviewed
Sep 23, 2026
isaacs
left a comment
isaacs
left a comment
Member
There was a problem hiding this comment.
One minor-ish confusing thing I think probably should be fixed (if I'm understanding accurately what's going on), but otherwise this is fine.
JPeer264
force-pushed
the
jp/node-suites-bun
branch
from
September 24, 2026 09:13
2f6d13a to
da4d406
Compare
JPeer264
force-pushed
the
jp/node-suites-bun
branch
3 times, most recently
from
September 24, 2026 14:19
c4ddeec to
e986b27
Compare
Base automatically changed from
jp/node-suites-bun-shared-runner
to
develop
September 25, 2026 15:41
The Bun package now selects every Node suite and excludes the ones that fail on Bun, grouped by cause: Node-only features, no `http.server` span because Bun does not publish `http.server.request.start`, no `fetch` instrumentation because Bun's `fetch` does not publish undici channels, no outgoing `node:http` instrumentation on Bun 1.3.14 (JS-3507), and no auto-instrumentation under `bun run` (JS-3508). Excluded files do not run, so the job stays fast. The list is based on Bun 1.3.14, the version CI pins. The suites that assert the SDK name use `EXPECTED_SDK_NAME`, so they also pass with `@sentry/bun`, and `bun-runtime-metrics` runs on Bun instead of Node. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JPeer264
force-pushed
the
jp/node-suites-bun
branch
from
September 25, 2026 15:41
e986b27 to
d72d702
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
isaacs
approved these changes
Sep 25, 2026
isaacs
left a comment
isaacs
left a comment
Member
There was a problem hiding this comment.
This is great! Love to see our tests come together like this, it's so validating of the overall architectural work we've been doing the last few months.
Approved because the only really blocking concern (the prisma failure) is being caught by CI anyway, so fix that and we're good 👍
The other relevant concern is that some tests actually are being dropped that were run before. I thought at first it'd be a tiny couple-line patch to pull them back in, but it's a bit more involved. If you wanna do that as a followup, that's fine, but it could also just go in here, I think.
| 'suites/tracing/postgresjs/test.ts', | ||
| 'suites/tracing/prisma-orm-v5/test.ts', | ||
| 'suites/tracing/prisma-orm-v6/test.ts', | ||
| 'suites/tracing/prisma-orm-v7/test.ts', |
Member
There was a problem hiding this comment.
This is why the CI is failing, because a new test was added on develop, but not here.
Obviously, once this lands, it'll be less of an issue. But, in addition to adding it to the set right now, I think we can help prevent the footgun that exposes.
I'd recommend:
- Add a line in dev-packages/node-integration-tests/README.md, like "When adding a test to this suite, also consider whether that test can run properly on Bun. If not, create an exclusion in
dev-packages/bun-integration-tests/node-suites/excludes.ts." - This could also maybe be added to our skills which mention node-integration-tests (maybe just tell them to be sure to check the readme?)
- .claude/skills/add-ai-integration/SKILL.md
- .claude/skills/vendor-otel/SKILL.md
- .claude/skills/write-tests/SKILL.md
Suggested change
| 'suites/tracing/prisma-orm-v7/test.ts', | |
| 'suites/tracing/prisma-orm-v7/test.ts', | |
| 'suites/tracing/prisma-orm-v8/test.ts', |
| max?: number; | ||
| }): typeof describe | typeof describe.skip { | ||
| // Vitest always runs on Node, so its Node version says nothing about Bun or Deno running the | ||
| // scenario. Those runtimes list the suites they cannot run in their own exclude lists. |
Member
There was a problem hiding this comment.
Might also want to add a line here to say that a file with a complementary min/max gate will not work properly on Bun, and must be excluded in dev-packages/bun-integration-tests/node-suites/excludes.ts.
|
|
||
| // `@sentry/node` instruments `fetch` through undici's diagnostics channels, which Bun's `fetch` | ||
| // does not publish. `@sentry/bun` has its own `fetchIntegration` for this. | ||
| const NO_FETCH_INSTRUMENTATION = [ |
Member
There was a problem hiding this comment.
The way this is structured right now, bun's fetchIntegration tests don't actually run.
It's a little complicated to explain in ye olde englishe, so I wrote up a patch that would bring the lost coverage. It's a little complicated, there might be an opportunity to simplify it down a bit, or if you think it's cleaner, we could do as a followup. But I think it's important to not lose test coverage in the end, if possible.
https://gist.github.com/isaacs/06a374f207bca343b51d9ad831485b7b
Member
There was a problem hiding this comment.
(Same for http.server tests. Updated gist.)
|
|
||
| describe('bunRuntimeMetricsIntegration', () => { | ||
| // The integration measures the Bun process, so the suite runs on Bun only. | ||
| describe.skipIf(RUNTIME !== 'bun')('bunRuntimeMetricsIntegration', () => { |
Member
There was a problem hiding this comment.
Can we move this test to dev-packages/bun-integration-tests, and then remove this skipIf condition?
This branch has not been deployed
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
2 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.
Bun now runs every Node suite except an explicit exclude list, grouped by cause, so a new Node test runs on Bun by default and a gap shows up as a failing test rather than as missing coverage. Excluded files do not run, which keeps the job fast even though most exclusions would otherwise wait for a timeout.
The exclude groups document what
@sentry/nodecurrently cannot do on Bun: nohttp.serverspan (Bun does not publishhttp.server.request.start), nofetchspans (Bun'sfetchdoes not use undici), no outgoingnode:httpinstrumentation on Bun 1.3.14 (#23881), and no auto-instrumentation underbun run(#23882). The list is based on Bun 1.3.14, the version CI pins; several suites already pass on newer Bun.The suites that assert the SDK name use
EXPECTED_SDK_NAME, so they also pass in the@sentry/bunproject.FWIW in a later PR more tests will be enabled as Bun will then run
Bun.buildbefore - see #24612Part of #23889.
🤖 Generated with Claude Code