Skip to content

Fix every type error in test/ - #44614

Merged
Jarred-Sumner merged 11 commits into
mainfrom
claude/fix-test-type-errors
Oct 9, 2026
Merged

Jarred-Sumner merged 11 commits into
mainfrom
claude/fix-test-type-errors

Conversation

@Jarred-Sumner

@Jarred-Sumner Jarred-Sumner commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

cd test && bun check (#44361) reported 8,080 type errors in 1,107 files. Now it reports none. tsc 7.0.2 agrees on both numbers.

$ cd test && bun check
✓ No type errors in 2,615 files [418.81ms]

Apart from the few files listed under "The JavaScript that did change", every edit to a test is erased by the transpiler: annotations, type arguments, !, casts, import type, // @ts-expect-error. That is checked mechanically, see below.

test/tsconfig.json

  • baseUrl is gone. TypeScript 7 rejects it (TS5102) and stops before checking anything. paths resolve relative to the tsconfig.json without it, in tsc and in Bun's resolver. Of the 1,697 distinct (directory, bare specifier) pairs imported anywhere below test/, .js files included, one resolved through baseUrl: "harness.js" in pnpm-migration.test.ts. It is "harness" now, which is the same file.
  • lib.dom is gone. The config says "lib": ["ESNext"], but one unused svelte fixture had /// <reference lib="dom" />, which adds lib.dom to the whole program. With lib.dom loaded, bun-types steps aside for ReadableStream, WebSocket, CompressionStream, fetch and others, so type: "direct" streams, new CompressionStream("zstd"), new WebSocket(url, { headers }) and so on were all errors.
  • composite: false, like src/js/tsconfig.json. Tests emit no declarations, and they import files from all over the repository, each of which was a TS6307.
  • bun-types comes in through include, like in src/js/tsconfig.json. A types entry is looked up from the directory of the config that extends this one, so bundler/transpiler/tsconfig.json could not find it, and neither could an editor or bun check <file> for a file in that directory.
  • Excluded: code vendored from Node.js and the React Compiler, esbuild-decorator-tests.ts (standard decorators, this project sets experimentalDecorators), the bun-types fixture (declares globals such as ProcessEnv.FOO, and bun-types.test.ts checks it with its own config), and six fixture apps that have their own package.json and tsconfig.json and whose dependencies the test installs.
  • ../src/runtime/bake/client/**.ts is no longer included. Tests import two of those files, which still brings them in. The rest needs lib.dom and belongs to src/runtime/bake/tsconfig.json.

Two small ambient files are new. test/internal-for-testing.d.ts declares the 9 $ intrinsics that src/js/internal-for-testing.ts calls (src/js/builtins.d.ts and private.d.ts declare them next to changes to Error, Function and other globals that tests should not see). test/asset-imports.d.ts declares *.c, *.cc, *.h, *.pem, *.br and *.gzip, which tests import as a path or as text.

Tests import a few files from outside test/, so those are part of the project: 5 files in packages/bun-error, packages/bun-release and scripts/build got the same kind of erased fix.

bun-types

Correct tests that did not type check because the declarations were wrong. Each has a case in test/integration/bun-types/fixture.

before
test.todo("label"), describe.todo("label"), and the same for skip TS2554, fixes #25959. The runtime allows a missing callback for both. Parameters<typeof test.skip> is unchanged, and describe.skip(SomeClass) is still an error: on its own, a function is the body
test("label", { retry: 1 }, () => {}) TS2769. It is the form in docs/test/index.mdx. Parameters<typeof test> is unchanged
test.each(["a", "b"] as const), describe.each(...) "readonly ... cannot be assigned to the mutable type 'unknown[]'"
expect(await sql`select 1`).toEqual([{ x: 1 }]) "not assignable to parameter of type 'undefined'". For a call that is only generic in its return type, T was inferred as never from expect(actual?: never), so that overload won. Same for webview.evaluate()
Bun.write(path, Bun.TOML.stringify({ ... })) string | undefined for every input. It is undefined only for undefined, a function or a symbol, and the return type says so now. This was about 350 of the errors
Bun.YAML.parse(buffer) string only. The runtime takes what Bun.TOML.parse takes: typed arrays, ArrayBuffer, DataView, Blob
new ReadableStream({ type: "bytes" }) without lib.dom TS2769, and a comment said Mode "bytes" is not supported. The overload is the last one, so a source typed any still gives ReadableStream<any>
new Blob().slice() without lib.dom TS2339
new TextDecoderStream("utf-16le", { fatal: true }) without lib.dom TS2554
webview.goBack(), webview.goForward() declared as back() and forward(), which do not exist at runtime

The JavaScript that did change

For the type check:

  • cli/install/migration/pnpm-migration.test.ts: "harness.js" is "harness", see above.
  • js/sql/wire-frames.ts declared export function pgParameterStatus twice, with the same body (TS2393). The second one is gone.
  • bundler/transpiler/macro-test.test.ts: its three assert { type: "macro" } imports are with { type: "macro" }. In TypeScript 7 assert is a syntax error (TS2880), which // @ts-expect-error cannot suppress and which stops the check of the whole project. The transpiled imports are the same. One test is new: it covers assert for a named, aliased, default and namespace import and for "bun", from a fixture string.
  • js/bun/xml/generate_xml_test_suite.ts emits the expect<unknown>(...) that the generated suite now has.
  • harness.ts passes toMatchNodeModulesAt, toHaveBins, toBeValidBin and toBeWorkspaceLink to its own expect.extend(). It declares them on Matchers<T> now, which applies to the whole project, so a test that forgot to register them would type check and then fail with "toHaveBins is not a function".
  • 6 files in integration/bun-types/fixture have new cases. They are only type checked.

Bugs in tests that a type error pointed at, where the alternative was a cast that hides it. Each was run on main and on this branch:

on main now
js/web/fetch/headers.undici.test.ts 14 lines are expect(actual, expected): no matcher, the second argument is the failure message expect(actual).toBe(expected), all pass
js/node/crypto/crypto.key-objects.test.ts an expect() without a matcher, publicKey.y compared with itself (2x), 8 assertions after publicKeyFromJwk / privateKeyFromJwk that check the other key, a callback for generateKeyPairSync() that uses undeclared resolve / reject they assert what they mean to, the callback is gone, same pass count
js/bun/shell/test_builder.ts import { ShellError } from "bun" is undefined, so .error(aShellError) throws Right hand side of instanceof is not an object. doChecks() is not awaited in the catch path $.ShellError, awaited. The shell suite gives the same results
js/bun/http/bun-serve-ssl.test.ts keepAlive: false, which fetch does not read keepalive
js/bun/websocket/websocket-server-backpressure-buffer.test.ts maxBackpressure, which Bun.serve does not read, so both tests ran with the default limit backpressureLimit
bundler/bundler_splitting.test.ts 10 tests pass env: "inline", which is the environment of the subprocess (0=i, 1=n, ...) dotenv: "inline"
bundler/esbuild/default.test.ts automaticRuntime: false, which the helper does not read, so the test was a todo runtime: "classic", passes, not a todo
js/web/fetch/body.test.ts const { readable } = file(url) is undefined, so the case was skipped as "fails, text is empty" file(url).stream(), passes, not skipped
js/web/fetch/fetch.test.ts first[0] on an ArrayBuffer: compares undefined with undefined Uint8Array views, so it checks that arrayBuffer() returns a copy
js/bun/util/password.test.ts the async hash helper calls hashSync() password.hash()
js/node/http/node-http.test.ts socket => expect(socket).toBe(socket) shadows the socket it means to compare with compares with the outer one
cli/install/bun-install.test.ts with --todo, two todo tests fail in their setup: ctx is not defined, and join({ root }, ...) they reach their assertions. Still todo
js/bun/test/fake-timers/sinonjs/fake-timers.test.ts let timersModule, timersPromisesModule shadows the two imports (inside describe.todo) the let is gone
harness.ts toHaveBins() passes for an empty .bin directory, [].every(...) is true compares the number of bins. The 56 call sites pass, also on Windows
harness.ts, cli/install/bun-install-registry.test.ts new Error("Failed to create user:", response.statusText) drops the status text it is in the message
docker/index.ts waitTcp() calls Bun.connect() without socket, which throws, so it times out on an open port resolves in 2 ms, still times out on a closed port
js/bun/archive-extract-leak-repro.ts Bun.Archive.from() does not exist new Bun.Archive(), the script runs

With TypeScript 6.0.2

cd test && bun run typecheck goes from 8,065 errors to 2, both TS2786 in jsx-production.tsx and jsx-dev.tsx. TypeScript 6 adds an implicit react/jsx-runtime import to every file of a react-jsx project, also to ../src/js/internal-for-testing.ts, where it resolves to the root's @types/react 18.3 instead of test/'s 18.0. TypeScript 7 only adds it to files that have JSX.

Not in this PR

  • Nothing in CI type checks test/, so this can regress. .github/workflows/lint.yml checks scripts, src/js, src and src/runtime/bake.
  • bun check at the repository root follows the references in the root tsconfig.json, and src/tsconfig.json, src/js/tsconfig.json and src/runtime/bake/tsconfig.json still have baseUrl.

How did you verify your code works?

  • bun check, tsc 7.0.2: 0 errors. tsc 6.0.2: the 2 above.
  • Same JavaScript. A script transpiles each changed file at main and on this branch (bun build --no-bundle, so with this tsconfig.json's emitDecoratorMetadata) and compares the output: of 662 files, 634 are byte-identical, 1 differs in the line breaks of an object literal that prettier re-wrapped (identical with --minify-whitespace), and 27 are the ones listed above.
  • Same test results. Line numbers, columns and file sizes do change, and some tests look at their own (response.test.ts snapshots Response (8.0 KB) for its own source file). So I ran the 1,322 test files that changed or that sit next to a changed helper or fixture, on main and on this branch, with the same release build (linux x64, no docker): 49,644 tests pass on main. 1,302 files have the same exit code, the same pass / fail / skip / todo counts and the same failing tests. The other 20 differ in both directions (11 better, 9 worse). Run twice more on each side, each of them gets the same result on both sides at least once, and the one that looked worst (serve-http2-lifecycle.test.ts) fails 3 of 5 runs on main and 1 of 5 here. The files touched after that run were run again (191, then 161 files), with the same outcome. 70 of the 1,322 import bun:internal-for-testing, which a release build only allows with BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING and BUN_GARBAGE_COLLECTOR_LEVEL set, so they failed to load on both sides in that run. Run again on both sides with the two variables (3,682 tests pass on main): 67 are the same, and the other 3 are the same when run on their own. I did not run anything with a debug build: there is no native change.
  • No hunk is unnecessary. For each hunk: revert it, run bun check, keep the revert if there is still no error. 504 hunks were reverted that way, most of them workarounds that the bun-types fixes above made unnecessary. 2,908 hunks are left.
  • bun test test/integration/bun-types/bun-types.test.ts: 22 pass, with and without lib.dom.
Other bugs in tests that the type errors pointed at. Not fixed here

Each was confirmed by reading what the test calls or by running it. They need a service, another OS, the network, or a decision about what the test should do. Paths are relative to test/.

  • js/bun/http/bun-serve-ssl.test.ts:128: The loop over serverNames only sets the Host request header, which never selects the TLS SNI name in Bun's fetch; SNI is always the URL hostname (localhost). The localhost2.com entry of the valid 2 fixture is never exercised, even if keepAlive is corrected to keepalive; tls: { serverName } would be needed.
  • js/bun/perf_hooks/histogram.test.ts:537: The "toJSON method" test wraps all its assertions in if (typeof h.toJSON === "function"). Bun's histogram has no toJSON, so the test only logs a message and passes with zero assertions.
  • js/node/http/node-http.test.ts:1340: "should noop keepSocketAlive" never calls agent.keepSocketAlive(): the call sits in the request handler of a server that never listens, so the only assertion that runs is agent.keepAlive === true. The dead handler also passes a ClientRequest instead of a socket and calls server.end(), which does not exist, and keepSocketAlive is no longer a no-op in Bun.
  • js/sql/sql.test.ts:1224: "Transaction throws on uncaught named savepoint" calls the misspelled sql.savepoit("watpoint", ...). The call throws TypeError: sql.savepoit is not a function, .catch(() => "fail") maps any error to "fail", and the test passes without ever creating a savepoint.
  • js/valkey/reliability/connection-failures.test.ts:307: "should handle rapid connection/disconnection" pushes client.close() results into promises and awaits Promise.all, but close() returns undefined, so nothing is awaited. The clients never connect either, because new RedisClient() is lazy. The test has no expect and opens no socket.
  • js/web/abort/abort.test.ts:9: "AbortSignal > spawn test" never runs the 23 tests in abort.ts. It copies them to <tmpdir>/bun.test.XXXX/abort.test.ts, then runs bun test /abort.test.ts with cwd: os.tmpdir(); that filter matches nothing and the child exits 1, but the test only asserts that stderr lacks "✗" and never checks the exit code. abort.ts is referenced nowhere else.
  • regression/issue/25589-frame-size-connect.test.ts:147: The three tests in HTTP/2 FRAME_SIZE_ERROR with @connectrpc/connect-node never send a request: each one only does assert.ok(transport) on the object returned by createGrpcTransport(). makes basic gRPC request without FRAME_SIZE_ERROR makes no gRPC request, the 50-header interceptor in transport with large headers in interceptor never runs, and the EchoService definition is unused.
  • bundler/cli.test.ts:271: describe.concurrent("bun build", () => {...}, 10_000) passes a timeout as third argument to describe. describe parses it but never applies it, so the tests in the block run with the default timeout instead of 10 s.
  • js/bun/http/async-iterator-stream.test.ts:245: "async generator aborted doesn't crash" has its only assertions (toBeInstanceOf(DOMException), name === "AbortError") inside catch; if fetch resolved instead of rejecting, the test would pass with zero assertions. Today fetch does reject, so the assertions run.
  • js/bun/http/bun-serve-ssl.test.ts:144: The last fetch of the valid/valid 2 tests (Host: "badhost.com") assigns const res and the test ends without any assertion on it, so the only thing checked is that the fetch does not reject.
  • js/bun/image/image.test.ts:1425: "BMP 32-bit BI_RGB is XRGB" passes { backend: "bun" } to the Bun.Image constructor, which has no such option, so it is silently ignored. The backend is the process-global Bun.Image.backend. On macOS and Windows that global is "system" at this point, so the test decodes through ImageIO/WIC and not through Bun's own BMP decoder, which it means to cover.
  • js/bun/perf_hooks/histogram.test.ts:474: "percentiles map properties" asserts that the values of h.percentiles are bigint and equal to those of h.percentilesBigInt. This bakes in Bun's divergence from Node, where percentiles holds numbers. In Node the assertion only holds because both getters return the same Map object, which the percentilesBigInt getter has just refilled.
  • js/bun/test/parallel/test-http-get-can-use-Agent.ts:7: test-http-get-can-use-Agent.ts and test-https-get-can-use-Agent.ts make real requests to google.com, which the repo's test rules forbid ("Never contact the public internet"). The tests depend on external network access.
  • js/bun/util/filesink.test.ts:74: Bun.file(path).stream(64), commented "test it on a small chunk size", has no effect. The chunk size is used only for in-memory blobs and is dropped for file-backed ones, so the FIFO reader runs with the default chunking.
  • js/first_party/ws/ws.test.ts:172: In the send() tests the typeof data === "string" branch is dead code: the ws client always emits a Buffer, and the echo server re-sends the Buffer it received, so string messages come back as binary frames and take the else branch (isBinary true). expect(isBinary).toBeFalse() never runs, so no test in the file checks isBinary === false on a client socket (only server sockets are checked, in isBinary and handles data passed along with the upgrade request).
  • js/node/tls/test-system-ca-https.test.ts:40: test.skip(skipReason) is called from inside the running test to skip it. bun:test throws Cannot call test.skip() inside a test. Call it inside describe() instead., so on a Linux machine with no system CA bundle the test fails instead of being skipped (reached only with BUN_TEST_ALLOW_NET=1; otherwise the whole test is test.skip).
  • js/web/fetch/response.test.ts:52: The print size test snapshots Bun.inspect(new Response(Bun.file(import.meta.filename))), which prints the size of the test file itself. The snapshot Response (8.0 KB) holds only while the file is 8000 to 8049 bytes, so any edit that moves the size out of that window breaks the test, including adding a test to the file.
Gaps in bun-types that are left. The tests use a cast there, never @ts-expect-error, so fixing the types breaks nothing

Each was checked against the runtime (release build) and the current declarations.

Web globals without lib.dom:

  • Request#clone(), Response#clone(), formData() and bytes() are inherited from undici-types and return undici's Request / Response / FormData / Uint8Array<ArrayBufferLike>. Bun.serve({ fetch: () => res.clone() }), new Response(await req.formData()) and const r: Request = req.clone() are errors.
  • The global FormData has no [Symbol.iterator], and values() / entries() are typed as yielding string only.
  • ReadableStream#getReader() returns the reader of node:stream/web, which is not assignable to the global ReadableStreamDefaultReader. So stream.getReader().readMany() does not type check.
  • new Request(url: URL, init?) has no overload, and there is no RequestInfo.
  • ResponseInit.headers is undici's HeadersInit, not Bun.HeadersInit (RequestInit overrides it). Bun.HeadersInit has no iterable member, so new Headers(map) is an error.
  • port.onmessage = (e: MessageEvent) => {} on a MessagePort or BroadcastChannel is an error: the handler gets undici's MessageEvent, and Bun.MessageEventSource is undefined. MessageEventInit has no ports.
  • Blob#stream(chunkSize): declared without parameters, two docs guides show blob.stream(1024).
  • URLSearchParams#toJSON() returns string[] for a repeated key, typed Record<string, string>.
  • WebAssembly.Exception and WebAssembly.Tag are missing.
  • Client WebSocket: binaryType rejects "blob", send() / ping() / pong() reject a Blob, WebSocketEventMap has no ping / pong.
  • Bun.WorkerOptions has no execArgv.

bun:test:

  • jest.mock() and jest.now() are missing from namespace jest. jest.now() is in docs/test/dates-times.mdx.
  • toHaveReturnedWith(), toHaveLastReturnedWith(), toHaveNthReturnedWith(n): expected is optional at runtime and in Jest's types.
  • expect.extend({ toBeEven(received: number) {} }) is an error: the first parameter can only be unknown or any. Jest's types use any.
  • expect({ value: 1, name: undefined }).toEqual({ value: 1 }), the example in the JSDoc of toStrictEqual, is an error.

Bun.*:

  • Bun.JSON5.parse takes string only. Like Bun.TOML.parse, the runtime takes typed arrays, ArrayBuffer, DataView and Blob.
  • Bun.YAML.stringify returns undefined for undefined, a function or a symbol. It is typed string, where TOML.stringify and JSON5.stringify are string | undefined.
  • SyncSubprocess.exitCode is null when a signal killed the child, typed number.
  • Bun.spawn: cmd and stdio reject readonly arrays, stdin rejects ArrayBuffer / SharedArrayBuffer. The error lands on cmd ("does not exist in type 'string[]'").
  • Bun.udpSocket({ socket: { error(socket, error) {} } }): the runtime calls error with one argument, the error, and this is the socket. The sockets have [Symbol.dispose], undeclared.
  • Bun.serve({ unix, http2 }) and { unix, idleTimeout } are errors: those options are only on HostnamePortServeOptions. server.fetch(url, init) takes one parameter. WebSocketHandler.message is required, the runtime wants any one handler.
  • Bun.connect({ fd, socket }) has no overload, although Bun.FdSocketOptions is declared.
  • Bun.TLSOptions lacks crl, sigalgs, ecdhCurve, allowPartialTrustChain, sessionTimeout. The Error of handshake() and getAuthorizationError() always has a code, undeclared.
  • BuildConfig.metafile is boolean. string and { json?, markdown? } are documented and work.
  • Bun.Transpiler: no minify option, scan() / scanImports() take no loader, transform(code, loader) rejects json, toml, yaml, text and others.
  • BuildMessage#notes, line, column and ResolveMessage#line, column are missing.
  • Bun.file("s3://...", s3Options) is an S3File at runtime, typed BunFile. Bun.write(dest, data, options) does not take S3Options, and Bun.write(S3File, Response | Request) has no overload.
  • BunFile#write() rejects a Blob. Its options are typed { highWaterMark }, the runtime reads createPath and type.
  • Bun.hash(data) always returns a bigint, typed number | bigint.
  • Bun.peek(rejected) returns the rejection reason, typed Promise<T> | T. The example in docs/runtime/utils.mdx does not type check.
  • new Bun.CryptoHasher(algorithm, hmacKey) rejects an ArrayBuffer key.
  • RedisClient: psubscribe(), punsubscribe(), pubsub() are missing, KeyLike excludes ArrayBuffer.
  • sql.array() rejects typed arrays, and Bun returns int[] columns as Int32Array.
  • $.ShellPromise has no bytes().
  • Bun.WebView overloads addEventListener for CDP events, not removeEventListener.
  • JSCallback (bun:ffi) has [Symbol.dispose], undeclared.
Runtime behavior noticed on the way

Each was reproduced with the release build, and compared with Node 26.3 where that applies.

  • src/js/internal/sql/shared.ts:1342: sql.close({ timeout: 0 }), documented as closing all connections immediately, behaves like close() with no timeout and waits for pending queries, so it never resolves while a connection is stuck mid-handshake. hasTimeout = !!timeout is false for the number 0, so the timeout === 0 branch is only reached by the string "0". sql-close-pending-connection.test.ts passes "0" to get the forced close.
  • src/ast/scope.rs:494: Bun accepts an ES module with two top-level function f / export function f declarations and silently keeps the last one. Node rejects the same file with SyntaxError: Identifier 'f' has already been declared, because top-level function declarations are lexical in a module. Bun also does not report the duplicate export name.
  • src/jsc/bindings/JSNodePerformanceHooksHistogram.cpp:232: Histogram.percentiles returns a Map with bigint values in Bun (Map { 0: 5n, 100: 5n }), where Node returns numbers (Map { 0 => 5, 100 => 5 }). getPercentiles has the same body as getPercentilesBigInt.
  • src/jsc/bindings/JSNodePerformanceHooksHistogramPrototype.cpp:39: perf_hooks.createHistogram() objects have no toJSON() in Bun. JSON.stringify(histogram) gives {}, where Node gives {"count":1,"min":5,"max":5,"mean":5,"exceeds":0,"stddev":0,"percentiles":{...}}.
  • src/jsc/bindings/webcore/JSDOMFormData.cpp:314: A plain Blob appended to a FormData without a filename comes back from get() and from Response.formData() as instanceof File with name === undefined and constructor.name === "Blob", and is serialized with filename="". The XHR spec and Node give such an entry the name "blob" (filename="blob"). test/js/web/html/FormData-multipart-serialization.test.ts asserts the current behaviour (name: undefined, filename="").
  • src/runtime/ffi/FFI.h:222: Calling a cc()-compiled function whose argument is "cstring" with a JS string or a CString object segfaults, while the same call through dlopen works and the shared types allow string. This is the "FIXME: bus error" behind the skipped block in test/js/bun/ffi/cc.test.ts.
  • src/js/node/inspector.ts:526: inspector.Session#post(method) without a callback returns the result object in Bun and throws synchronously on an error such as an unknown method. Node returns undefined and does not throw. test/js/node/inspector/inspector-profiler.test.ts asserts the Bun-only return value, e.g. expect(session.post("Profiler.enable")).toEqual({}).
  • src/runtime/crypto/PBKDF2.rs:164: crypto.pbkdf2(password, salt, iterations, keylen, "md55") with no callback throws ERR_CRYPTO_INVALID_DIGEST in Bun and ERR_INVALID_ARG_TYPE (The "callback" argument must be of type function) in Node, because Bun validates the digest name before the callback. The digest test in test/js/node/crypto/pbkdf2.test.ts omits the callback and so pins the Bun-only order.

`cd test && bun check` reported 8,080 type errors in 1,107 files, and
reports none now. tsc 7.0.2 agrees on both numbers.

The edits to tests are erased by the transpiler (annotations, type
arguments, `!`, casts, `import type`, `// @ts-expect-error`), so no test
changes what it does.

test/tsconfig.json drops `baseUrl` (removed in TypeScript 7), no longer
gets lib.dom from a fixture's triple-slash reference, is not `composite`,
takes bun-types through `include`, and excludes vendored code and fixture
apps that have their own tsconfig.json.

bun-types: `test.todo(label)`, `test(label, options, fn)`, `each()` with
readonly tables, `expect(await genericCall())`, `TOML.stringify()` of an
object, `YAML.parse()` of bytes, `new ReadableStream({ type: "bytes" })`,
`Blob#slice()` and `new TextDecoderStream(label, options)` without
lib.dom, `WebView#goBack()` / `goForward()`.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the new conditional return type of Bun.TOML.stringify against the runtime (src/runtime/api/TOMLObject.rs:65-84): arrays, Dates, Temporal objects and primitives at the top level throw rather than return undefined, so string for object inputs and undefined only for undefined/function/symbol matches what the runtime actually returns.

Extended reasoning...

The change is a 670-file sweep of type-only edits across test/ plus declaration changes in packages/bun-types (test.d.ts, bun.d.ts, globals.d.ts) and a rewritten test/tsconfig.json; it touches no security-sensitive surface. The bug hunt stopped at its finding cap rather than running dry, and the inline findings on the bun-types declaration changes are the part a human should weigh, so this is not an approval.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟡 packages/bun-types/test.d.ts — nit: Users calling test.skip("reason") with only a label still get a type error after this PR, even though the runtime accepts it exactly like test.todo("label"), which this PR fixes. The diff adds the label-only overload to todo at packages/bun-types/test.d.ts:532 but leaves skip: Test<T> at test.d.ts:524 (and Describe.skip at test.d.ts:242) without it, and instead silences the two in-tree callers with (test.skip as any)(...). Fix: declare skip: Test<T> & ((label: string) => void) (and the Describe twin) so the types mirror the runtime, then drop the as any wrappers at test/regression/issue/09469.test.ts:5 and test/js/node/tls/test-system-ca-https.test.ts:40. [also at: packages/bun-types/test.d.ts:532 - nit: users of bun-types still get a type error for test.skip("label") and describe.skip("label"), which the runtime accepts exactly like .todo, and this PR hides the two in-tree callers behind as any instead.]

    Why this was flagged

    The runtime treats skip and todo the same for a missing callback: src/runtime/test_runner/ScopeFunctions.rs:152 maps SelfMode::Skip | SelfMode::Todo => CallbackMode::Allow, so test.skip("label") is accepted. This PR adds todo: Test<T> & ((label: string) => void) at packages/bun-types/test.d.ts:532, but skip at test.d.ts:524 stays Test<T>, which requires a function. Rather than add the overload, the diff wraps the two callers that use label-only skip in (test.skip as any)(...) at test/regression/issue/09469.test.ts:5 and test/js/node/tls/test-system-ca-https.test.ts:40. On the base branch those sites were plain test.skip(...) calls that typechecked against an incomplete declaration; after merging, any user writing test.skip("label") still gets TS2554 while test.todo("label") now typechecks, and the repo's own .d.ts rule (declare only what is implemented, never as any to silence a call site) is not met for skip.

    Verification: src/runtime/test_runner/ScopeFunctions.rs:152 maps SelfMode::Skip | SelfMode::Todo => CallbackMode::Allow, so a label-only skip call is accepted. packages/bun-types/test.d.ts:532 adds todo: Test<T> & ((label: string) => void); while line 524 remains skip: Test<T>;, so test.skip("label") is still TS2554. test/regression/issue/09469.test.ts:5 is cast with as any instead.

Comment thread packages/bun-types/bun.d.ts
Comment thread packages/bun-types/globals.d.ts Outdated
Comment thread packages/bun-types/test.d.ts
Comment thread test/js/bun/css/color.test.ts
Comment thread test/integration/next-pages/test/dev-server-puppeteer.ts
Comment thread test/tsconfig.json Outdated
Comment thread test/docker/index.ts Outdated
Comment thread test/js/web/fetch/headers.undici.test.ts Outdated
Comment thread test/js/node/tls/test-system-ca-https.test.ts Outdated
…t.ts

- `test.skip(label)` and `describe.skip(label)` type check, like `todo`.
  The runtime allows a missing callback for both.
- The `type: "bytes"` overload of `ReadableStream` comes after the generic
  ones, so a source typed `any` still gives `ReadableStream<any>`, and it
  takes a type argument.
- `YAML.parse` and `TOML.parse` say that a file-backed `Blob` throws.
- test/tsconfig.json excludes the directories of the Expo app and not
  expo.test.ts next to them.
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator Author

Review, the finding outside the diff: test.skip("label") and describe.skip("label") type check as of 0581fb7, like todo (ScopeFunctions.rs maps SelfMode::Skip | SelfMode::Todo to CallbackMode::Allow, and a file with both reports 1 skip). The two (test.skip as any)(...) are gone, so 09469.test.ts and test-system-ca-https.test.ts are the same as on main again.

CI on c59a61c, build 123496: the one hard failure is test/js/bun/spawn/spawn.test.ts on debian 13 x64-asan, stdout reader of an unref'd child and process lifetime > an idle reader stopped at the highwater mark does not keep the process alive. This PR changes 4 lines of that file, all erased casts, none in that test. The same test fails on builds of unrelated branches from the same hours: 123480 (hard failure), 123485 and 123490 (passed on retry). claude-find-issues fails on every PR at the moment, before it reads anything.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Still open from earlier reviews (5):

  • Unresolved: 5 minor or pre-existing.

Comment thread packages/bun-types/test.d.ts Outdated
`Parameters<>` reads the last call signature of an intersection, so the
label-only signature of `skip` and `todo` goes first.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-reviewed after the latest push and found no bugs; the follow-ups to the earlier inline comments look right, but with 668 files and public bun-types changes a human look is still worthwhile.

What was reviewed:

  • 21aa092a: ((label) => void) & Test<T> / & Describe<T> ordering — the one-arg signature is tried first (so test.skip("x", fn) falls through on arity) and Parameters<> still reads the last Test<T> signature; the new fixture lines in test/integration/bun-types/fixture/test.ts lock this in.
  • Test<T> gaining (label, options, fn) ahead of (label, fn, options?): Parameters<typeof test> is unchanged because it takes the last signature.
  • each(table: readonly T[]) widening and expect(): Matchers<undefined> — no new overload-resolution regressions spotted beyond what was already raised inline.
Extended reasoning...

The PR is a type-error sweep across test/ (668 files) plus changes to the public declarations in packages/bun-types (test.d.ts, bun.d.ts, globals.d.ts), test/tsconfig.json restructuring, and small typed edits in packages/bun-error, packages/bun-release and scripts/build. It touches no security-sensitive surface. The latest commit only reorders the skip/todo intersection types and adds fixture assertions, which addresses the previously raised Parameters<> regression. The sheer size, the public .d.ts surface (including the WebView back/forward rename and overload reordering that affects downstream inference), and my inability to run the bun-types fixture test in this session are what rule out approving.

Still open from earlier reviews (2):

  • Unresolved: 2 minor or pre-existing.

- headers.undici.test.ts: `expect(actual, expected)` has no matcher, the
  second argument is the failure message. They are `.toBe(expected)` now,
  and all 14 pass.
- docker/index.ts: waitTcp() called Bun.connect() without `socket`, which
  always throws, so it timed out on an open port too.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread packages/bun-types/test.d.ts Outdated
On its own, a function is the body of the group and not its label, so
`describe.skip(SomeClass)` throws "Cannot call a class constructor", and
`describe.skip({ name })` is rejected too.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread test/harness.ts Outdated
`toMatchNodeModulesAt`, `toHaveBins`, `toBeValidBin` and
`toBeWorkspaceLink` are declared on `Matchers<T>` for the whole project,
so a test that forgot `expect.extend()` type checked and then failed with
"toHaveBins is not a function".
@Jarred-Sumner
Jarred-Sumner force-pushed the claude/fix-test-type-errors branch from 9127821 to be54155 Compare October 6, 2026 05:40
@robobun

robobun commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator
Updated 12:43 AM PT - Oct 6th, 2026

❌ @Jarred-Sumner, your commit 37c653b has 2 failures in Build #123529 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 44614

That installs a local version of the PR into your bun-44614 executable, so you can run:

bun-44614 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

* @param end byte offset to stop at. Defaults to `size`.
* @param contentType the `type` of the new `Blob`.
*/
slice(start?: number, end?: number, contentType?: string): Blob;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 nit (optional): Users without lib.dom who call new Blob([...]).slice("text/plain") or .slice(1, "text/plain") get a type error after merging, although the runtime accepts both. The new Blob.slice declaration at packages/bun-types/globals.d.ts:1517 lists only (start?, end?, contentType?), while the shared runtime implementation also takes a string first or second argument. Fix: give the new Blob.slice the same three overloads BunFile.slice already declares at packages/bun-types/bun.d.ts:2696-2715, so a plain Blob no longer needs a BunFile cast to type-check. The PR hides the gap at test/js/web/fetch/blob.test.ts:66 with (blob as Bun.BunFile) on a plain new Blob(["Bun", "Foo"]).

Why this was flagged

A bun-types consumer without lib.dom writes new Blob(["a"]).slice("text/plain") or .slice(1, "text/plain"). packages/bun-types/globals.d.ts:1517 is the only slice on interface Blob, typed slice(start?: number, end?: number, contentType?: string): Blob, so a string in the first or second position is TS2345. At runtime get_slice in src/runtime/webcore/Blob.rs:1844-1851 moves a string found in args[0] or args[1] into the contentType slot for every Blob, and test/js/web/fetch/blob.test.ts:26-66 asserts blob.slice("text/plain;charset=utf-8").type for a plain new Blob(["Bun", "Foo"]). The PR makes that line type-check by casting the plain Blob to Bun.BunFile (blob.test.ts:66), whose declaration at packages/bun-types/bun.d.ts:2696-2715 has the three overloads the runtime implements. On the base branch slice was not declared on Blob at all without lib.dom, so the declaration this PR adds is new surface that is narrower than the runtime it documents.

Verification: nit — triggers when a bun-types consumer without lib.dom calls Blob#slice with a string in the first or second position. The diff adds exactly one slice to interface Blob at packages/bun-types/globals.d.ts:1517. The runtime in src/runtime/webcore/Blob.rs:1844-1851 explicitly shifts a string in position 0 or 1 into the contentType slot for every Blob. Nothing that compiled on the base breaks.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it, and it breaks more than it fixes. With slice(start?: number, contentType?: string) and slice(contentType?: string) added to the global Blob, 9 of the 22 cases in bun-types.test.ts fail, starting with the fixture for #24154:

24154.ts:5:3  TS2322: Type 'import("node:buffer").Blob' is not assignable to type 'Blob'.
  Types of property 'slice' are incompatible.
    Types of parameters 'end' and 'contentType' are incompatible.
      Type 'string' is not assignable to type 'number'.

That is async function example(): Promise<Blob> { return await (await fetch("foo")).blob(); }: response.blob() is the Blob of node:buffer, whose slice has the one standard signature, so it stops being assignable to a global Blob that has more. #41809 came to the same conclusion. The one call in blob.test.ts that uses the string-first form keeps its cast.

Comment thread test/harness.ts
`[].every(...)` is true, so an empty `.bin` directory, or one with only
the first of the expected bins, passed.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

Still open from earlier reviews (1):

  • Unresolved: 1 minor or pre-existing.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator Author

CI on 457443d, build 123517: the only hard failure is test/js/bun/s3/s3.test.ts on ubuntu 25.04 x64 and alpine 3.23 aarch64, in the tests that talk to the real R2 bucket:

S3Error: Please look at https://www.cloudflarestatus.com for issues or contact customer support.
 code: "ServiceUnavailable"

A different test fails on each lane, the same file passes on retry on other lanes of that build, and cloudflarestatus.com reports "Partial System Outage" at the moment. The empty commit runs CI again.

All 16 Windows test shards pass (2019 x64 and 11 aarch64), which is what covers the Windows branch of the toHaveBins() change.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

Still open from earlier reviews (1):

  • Unresolved: 1 minor or pre-existing.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator Author

CI on af27923, build 123524: R2 is fine this time. The hard failure is spawn.test.ts on debian 13 x64-asan again, an idle reader stopped at the highwater mark does not keep the process alive, the same test as on the first build of this PR (when every test file still emitted the same JavaScript as on main).

How often that test fails elsewhere, from the annotations of builds 123420 to 123525: of the 47 finished builds of other branches, it is a hard failure in 7 (15%) and passes on retry in 19 more (40%).

Hard failures: 123449, 123452, 123469, 123479, 123480, 123489, 123520, on 6 unrelated branches.

Running CI again.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

Still open from earlier reviews (1):

  • Unresolved: 1 minor or pre-existing.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator Author

CI on f9c9530, build 123525: spawn.test.ts passes this time. The hard failure is test/js/web/timers/timer-gc-roots.test.ts on debian 13 x64-asan, an RSS threshold:

error: expect(received).toBeLessThan(expected)
Expected: < 8
Received: 8.07421875
✗ AbortSignal.timeout is released when its wrapper is collected > AbortSignal.any([timeout, controller.signal]).abort releases the timeout

This PR does not touch that file, there is no native change, and the measurement runs in a bun -e child that does not import harness. In the annotations of builds 123380 to 123526, the file fails and passes on retry in 17 of the 58 finished builds of other branches (29%), for example 123519, 123507, 123505.

Running CI again.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Still open from earlier reviews (1):

  • Unresolved: 1 minor or pre-existing.

Comment thread test/js/bun/archive-extract-leak-repro.ts Outdated
Comment thread test/js/bun/shell/test_builder.ts Outdated
Comment thread test/harness.ts Outdated
Each of these had a cast or a directive that silenced the error.

- shell/test_builder.ts: `ShellError` is `$.ShellError`, the import from
  "bun" is undefined, so `.error(aShellError)` threw. The checks in the
  catch path are awaited, like in the try path.
- bun-serve-ssl.test.ts: `keepAlive` is `keepalive`.
- websocket-server-backpressure-buffer.test.ts: `maxBackpressure` is
  `backpressureLimit`.
- bundler_splitting.test.ts: `env: "inline"` is `dotenv: "inline"`. `env`
  is the environment of the subprocess.
- esbuild/default.test.ts: `automaticRuntime: false` is
  `runtime: "classic"`, and the test passes, so it is no longer a todo.
- crypto.key-objects.test.ts: an `expect()` without a matcher, two
  `publicKey.y` compared with itself, eight assertions on the wrong key, and
  a callback for generateKeyPairSync() that used undeclared names.
- password.test.ts: the async `hash` helper called hashSync().
- node-http.test.ts: a parameter shadowed the socket it was compared with.
- body.test.ts: `BunFile` has no `readable`. With `.stream()` the case
  passes, so it is no longer skipped.
- fetch.test.ts: an ArrayBuffer was indexed directly.
- bun-install.test.ts: two todo tests failed in their setup (`ctx` not in
  scope, `{ root }` used as a string).
- fake-timers.test.ts: a `let` shadowed the two `timers` imports.
- harness.ts, bun-install-registry.test.ts: the status text was passed as
  the options of `new Error()`.
- archive-extract-leak-repro.ts: `Bun.Archive.from()` does not exist.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

Still open from earlier reviews (1):

  • Unresolved: 1 minor or pre-existing.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator Author

CI on 37c653b, build 123529: passed, on every platform. This is the first run of the test fixes of that commit on macOS and Windows.

The 2 failures in the status comment are on darwin 27 aarch64, which is a soft-fail lane: node-net.test.ts (a socket destroyed while connecting ... can connect again at once, connectionAttemptFailed ECONNRESET) and sql-close-pending-connection.test.ts (the process exits after the connection timeout of a dial that never completes). The edits to those two files here are erased by the transpiler. The same two files fail together on that lane in 14 of the 49 finished builds of other branches between 123440 and 123529, for example 123519, 123510, 123507, 123503.

@Jarred-Sumner
Jarred-Sumner merged commit e655c58 into main Oct 9, 2026
12 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the claude/fix-test-type-errors branch October 9, 2026 05:10
robobun added a commit that referenced this pull request Oct 10, 2026
Main changed bun-types and test/tsconfig.json (#44614). With them, four
lines of the package had a type error:

- `request.bytes()` has the type of undici. The body is read with
  `arrayBuffer()`, which `Bun.gunzipSync` takes.
- `request.clone()` has the type of undici too. It gets the cast that
  the tests of main have.
- `BodyInit` is `Bun.BodyInit`.

tsc reports the same 7 errors for test/ on main and on this branch. None
is in a file of this change.
Jarred-Sumner added a commit that referenced this pull request Oct 10, 2026
main's test/ has had none since #44614. 235 came back with this branch: the
tests it adds were never checked, and where a hunk of #44614 met a change of
this branch the rebase kept this branch's side.

Annotations, type arguments, casts, `!` and `import type` only: each of the 31
files transpiles to the same JavaScript as before.
dylan-conway added a commit that referenced this pull request Oct 13, 2026
### Problem
- The install tests fork verdaccio 6 and assert its answers where npm
differs. `/-/whoami` with an unknown token: verdaccio `200 {}`, npm `401
{}`.
- All 31 files share one `htpasswd` and one storage directory. When
verdaccio cannot listen on its random port, `start()` never settles: 6
of 155 local file runs, and main build 124221.

### Fix
- `test/packages/registry` is an npm registry on `Bun.serve`, without
dependencies. Its state is in memory, per instance.
- The 31 files import `TestRegistry` from `"registry"`.
`VerdaccioRegistry` and the verdaccio dependencies are removed, and
three packages that only verdaccio installed leave the bytecode snapshot
of `libraries.js`.
- `yarn-berry.test.ts` came to main with #40057, after the rebase. It is
migrated too (7025402), so 32 files use `TestRegistry`. Each number
for 31 files is from before that.
- Verified: `test/packages/registry/` (183 tests) and the 31 files (2518
pass, 0 fail).

### Test timings
| 31 files | main, verdaccio | this PR |
|---|---|---|
| local, wall-clock | 158 s | 83 s |
| local, CPU | 214 s | 113 s |
| CI, six lanes | 985 to 1090 s | 549 s |

Same base commit. Details: Notes.

### Background
- #33115 has the same goal. A maintainer has to choose.
- Reads follow registry.npmjs.org: 49 of 63 compared requests agree.
Writes follow the npm docs.
- Considered a class in `harness.ts`: every test file would load it.

### Downsides
- The registry is faster, so packuments arrive in another order. One
test depended on that (54 of 100 local runs failed) and orders them now.
- The registry runs on the test thread: a `spawnSync` of bun against it
never returns.

<details><summary>Notes</summary>

#### What I ran

After the rebase. The head is 9e9e394 on main c6da4a4. Linux x64,
12 vCPU. bun is the canary build e555359, which is 8 commits behind
that main commit.

| what | result |
|---|---|
| the 31 migrated files, 5 rounds, head 2d236bf | 154 of 155 file
runs passed: 2518 pass and 3 skip in a round. One run had one failing
test, which the commit after it repairs |
| the 31 files, 5 more passes and 5 runs in a row of each file, head
9e9e394 | 310 file runs, 0 fail |
| the same 31 files on main with verdaccio, 5 rounds | 2516 pass, 0
fail, 3 skip. 6 of 155 file runs did not start verdaccio |
| `test/packages/registry/` | 183 pass |
| `bundler_bytecode_portable.test.ts` | 22 pass |
| `tsc --noEmit` in `test/` | 7 errors on main, the same 7 on this
branch, none in a file of this PR |
| CI, build https://buildkite.com/bun/bun/builds/124229, head 2d236bf
| 183 of 183 jobs passed |

- The two tests that only this PR has are the difference between 2516
and 2518: the `whoami` test and the test of the publish header.
- The runs are after a clean install of `test/node_modules` from the
lockfile of each tree.
- The canary build passes the bytecode test of main on the tree of main.
So its bytecode is the bytecode of main.
- I did not run Windows or macOS after the rebase. The Windows machine
was not available. CI ran both.
- Build 124229 has one test of this PR that passed on a retry: see the
flaky tests below.

After the merge of main 110efa0 (d947d38) and the migration of
`yarn-berry.test.ts` (7025402). Linux x64. bun is `bun-linux-x64` of
main build 124948, which is 110efa0:

| what | result |
|---|---|
| `yarn-berry.test.ts` on d947d38 | does not load: `Export named
'VerdaccioRegistry' not found` |
| `yarn-berry.test.ts` on 7025402 | 58 pass, 0 fail |
| the same file, 40 runs in a row, then 5 rounds of 8 runs at the same
time | 80 runs, 0 fail |
| the same file with a registry that has no packages | 21 of 58 fail, so
the tests do read the registry |
| `bundler_bytecode_portable.test.ts` | 22 pass |
| `test/packages/registry/` | 183 pass |

- None of the inputs of the bytecode snapshot changed between c6da4a4
and 110efa0, so its values stay.
- The other 31 files were not run again after the merge. CI runs them.

Before the rebase, on the head a4367e5 (then 3b5d5ee):

- CI build https://buildkite.com/bun/bun/builds/121299 passed 181 of 181
jobs.
- Windows x64, canary a4f1429: `test/packages/registry/` had 183 pass.
`bun-publish`, `config-precedence`, `pnpm-lock-v9` and
`bun-update-lockfile-sync` had 258 pass, 0 fail, 1 skip, three commits
earlier.
- A local debug build with ASAN: `test/packages/registry/`, `hoist`,
`npmrc`, `config-precedence`, `bun-publish` and `bun-lockb` had 319
pass, 0 fail, when the package had 165 tests.
- With the leftovers of verdaccio in the fixture directory, the 31 files
had 2499 pass and 1 fail. The failure was `npm manifest cache entries
are only reused for the package name they were saved for`, which is
flaky on main too.

#### The rebase

- The base moved from a4f1429 to c6da4a4, 88 commits. The merge
commit is gone. The 12 commits were replayed, so each has a new id.
- 7 files had a conflict. In 5 of them #44614 changed a line that this
PR deletes or renames. `test/package.json` is the file of main without
the two verdaccio lines.
- `test/bun.lock` is the lockfile of main after `bun install
--lockfile-only` without verdaccio. Its diff has the same 578 lines as
before the rebase.
- After the replay, 42 of the 59 files had the same bytes as before the
rebase. For the other 17, the patch of this PR had the same lines, apart
from lines of main that it deletes and the snapshot.
- The snapshot of `libraries.js` has new values, because main has a new
WebKit: see the bytecode snapshot below.
- Two commits are new. 2d236bf: main changed bun-types and
`test/tsconfig.json` (#44614). With them, four lines of the package had
a type error. `request.bytes()` and `request.clone()` have the types of
undici there, and `BodyInit` is `Bun.BodyInit`.
- 9e9e394: see "A test that depended on the order of two packuments".

#### This PR and #33115

#33115 replaces verdaccio too. I did not find it before I opened this
PR: my search for open pull requests stopped at 20 results. Both PRs
change the same test files, so only one can land as it is. The author of
#33115 compared them in
#33115 (comment).

| | #33115 | this PR |
|---|---|---|
| Files | 1083 | 59 |
| State | conflicts with main, four review passes | applies to main |
| Fixtures | converted to source directories | as they are on main |
| The two mock registries (`dummy.registry.ts`,
`simple-dummy-registry.ts`) | replaced | not touched |

A maintainer has to choose. If this PR lands, #33115 can be closed, or
reduced to the parts that only it has.

**Rulings of the review of #33115 that this PR follows**

- A publish is refused with 415 unless the Content-Type is
`application/json`, character for character. `bun-publish.test.ts`
asserts the header that bun sends. The two comments in
`src/runtime/cli/publish_command.rs` stay as they are.
- An error that a handler throws fails the test: `stop()` throws it.
- The bulk advisory endpoint reads a gzip body, and a test sends one.
- `createTestDir()` does not remove users or tokens.
- `hasInstallScript` is true for a script that is not empty.
- `time.modified` moves on each write.
- A field of an advisory that is passed as undefined keeps its default.
The test compares with literal values.
- Nothing here says that bun reads `Cache-Control`. The registry sends
the header because npm does.

**What only #33115 has**

- The replacement of `dummy.registry.ts` and `simple-dummy-registry.ts`.
18 test files import them.
- `registry.define()`: a package that a test declares in code, with a
tarball writer and its golden files.
- The conversion of the fixtures to source directories.
- `registry-resolver-matrix.test.ts`.

Each of these can go on top of this registry as a PR of its own. So can
the three servers in `bun-audit.test.ts`, `config-precedence.test.ts`
and `bun-install-lifecycle-scripts.test.ts` that forward to the registry
to see or to change a request: `intercept` and `recordRequests` do that
now.

#### How each behaviour was established

**Compared with registry.npmjs.org, GET and HEAD only**

I sent 63 requests to registry.npmjs.org and to this registry with
copies of `is-number` and `@types/is-number`, and compared the status,
nine headers and the body. 49 agree. The requests cover:

- The packument in both forms, `HEAD`, `Range`, `?write=true`, and a
scoped name as `@s%2fn`, `@s/n` and `%40s%2fn`.
- A version by number and by tag, and one that is not there: 404 with
the JSON string `"version not found: 9.9.9"`.
- A tarball: `GET`, `HEAD`, `Range` with 206 and 416, `If-None-Match`,
and the 404 for the scope in the file name.
- `dist-tags`, `collaborators`, `visibility`, `ping`, `whoami`, the
account endpoints, `/-/v1/done`.
- The same endpoints with a token that npm does not know.

Separate probes gave:

- The abbreviated version is an allow list without `libc`,
`description`, `scripts` and `time`. `hasInstallScript` is there only
when true. The census covered 5763 versions of 9 packages.
- `Accept`: the registry looks for the media type anywhere in the
header, in exact case. It does not weigh q.
- The entity tag is the hex md5 of the body. It is weak on an encoded
answer. `If-Modified-Since` has no effect.
- Encoding: brotli when accepted, else gzip. Bodies of 21 and 26 bytes
came back as they are, a body of 63 bytes came back encoded. The
registry here starts at 48 bytes.
- `Last-Modified` is `modified` rounded up to the second. I checked one
packument.
- `You must be logged in to publish packages.` is the text of npm for a
token that it does not know, on `collaborators`.

**The 14 requests that differ**

| request | npm | this registry |
|---|---|---|
| a version, and search (4 requests) | the headers change between a
first and a later request: `public`, `last-modified` and `accept-ranges`
come and go | always `cache-control: max-age=300` |
| a packument or tarball that is not there (3) | `cache-control: public,
max-age=300` for an unscoped name, nothing for a scoped one | no
`cache-control` |
| `collaborators` | names in another order | the order of `maintainers`
|
| `/-/npm/v1/keys` | the keys of npm | `{"keys":[]}`, it does not sign |
| `/-/nonexistent-endpoint`, `/-/v1/login/cli/<id>` | 405 with `allow` |
404 `ResourceNotFound` |
| `/<name>/-rev/<rev>` with GET | `allow: PUT` | `allow: PUT, DELETE` |
| `?write=true` with the abbreviated `Accept` | `content-type:
text/plain` | `application/json` |
| `/login/cli/<id>` | 404, the page is on the website | stands in for
that page |

**From the npm/registry docs and the npm CLI source, not sent to the
live registry**

- Login with `PUT /-/user/org.couchdb.user:<name>`: 201 and `{ok, id,
rev, token}`, or 401 and `{"ok":false}`.
- Tokens and profile under `/-/npm/v1/`.
- The publish document, `www-authenticate: OTP`, the web flow with
`authUrl` and `doneUrl`, 202 with `retry-after`.
- The one-time password message. bun compares against this exact text.
- The request sequences of deprecate, unpublish, owner and dist-tag.
- The bulk advisory request and answer.

**My wording, because I do not know the text of the registry**

- Each `400 Bad Request: ...` for a publish body that fails a check, the
415, and the 403 for a read-only token.
- `409 Document update conflict.` is the text of CouchDB.
- Publish answers `200 {"ok":true,"id","rev"}` and a dist-tag write
answers `{"ok":"dist-tags updated"}`. I recall both from npm output. I
did not verify them.
- A restricted package answers 404 to a client that cannot read it. I
recall this from npm. I could not probe it, because I know no private
package.
- `You cannot publish over the previously published versions`, `You do
not have permission to publish` and `cannot be republished until 24
hours have passed` are the texts of npm as I recall them. I did not
probe them.
- A publish with a token that the registry does not know answers 401
without `www-authenticate`. npm sends `www-authenticate: Basic, Bearer`
on `collaborators`. I do not know what it sends on a publish.

#### Where the registry is more permissive than npm

- Every user can publish into every scope. npm needs a user or an org
with the name of the scope.
- A restricted package needs no paid plan. npm answers 402.
- A package of the storage directory has no maintainers, so every user
can add a version to it.

Not implemented: signatures, provenance, OIDC, orgs and teams.

#### What the tests saw change

Two tests of the 31 files depended on verdaccio. `whoami > invalid
token` asserted its answer, `200 {}`. It asserts `401 Unauthorized` now,
and a new test keeps the `200 {}` case with a mock server. `ranged peer
dependency resolution is stable across installs from bun.lock` depended
on its speed: see the section on the order of two packuments. Every
other test passes unchanged.

`bun-publish.test.ts`:

- Each `rm` of a package directory is a `registry.packages.delete()` of
that name. The next section has the reason.
- `--ignore-scripts` published `publish-pkg-4` and removed
`publish-pkg-5`. It passed only because the test before it removed
`publish-pkg-4`. It removes the right package now.
- `republishing normally fails` matched `/403|409|already exists|already
present|cannot publish/`. It asserts the status and the message.
- A new test asserts that the publish request has `content-type:
application/json`.

`harness.ts` does not export `VerdaccioRegistry`. `REVIEW.md` says to
delete code in the PR that makes it dead. The eight files that had a
variable `verdaccio` keep that name, so their diff stays small.

A bug of the registry that the review found: `bun publish` of a version
with build metadata got a 400. bun sends the key `1.0.0` and a manifest
with `1.0.0+build.5`. The registry accepts that now, and `cli.test.ts`
publishes one with bun.

#### A checkout that ran the tests on verdaccio

Verdaccio wrote each package that a test published into
`test/cli/install/registry/packages`. That is the fixture directory too,
and ignore rules hide what verdaccio wrote. One run of the 31 files on
main left 18 packages and `.verdaccio-db.json` there.

The new registry reads that directory. So it served those packages, and
a publish of the same version got `403 You cannot publish over the
previously published versions`. With the leftovers of that run in place,
15 of the 47 tests of `bun-publish.test.ts` failed.

- `bun-publish.test.ts` calls `registry.packages.delete()` in the 26
places where main removes a package directory. With the same leftovers
in place the file passes.
- The ignore rules stay as they are on main, so the leftovers stay out
of `git status`.
- The builds of this PR did not show this: a checkout of CI has no
leftovers.

#### Layout

The package has the layout of `test/packages/s3-server`, which came to
main while this PR was open: the sources in `src/`, the tests in
`test/`, and the helpers of the tests in `test/helpers.ts`. `index.ts`
does not import the harness. `test-registry.ts` has the class for the
tests of bun, and the alias `"registry"` points to it.

#### One port, one registry

`Bun.serve` sets `SO_REUSEPORT` when it gets `development: false`
(`src/runtime/server/ServerConfig.rs:716`). The registry passed that
option, so a second registry on the same explicit port started with no
error. It passes `reusePort: false` now, and the second one fails with
`EADDRINUSE`. The new test fails without the option.

The tests listen on port 0. On Linux, 20000 listeners on port 0 with
`SO_REUSEPORT` got 20000 distinct ports, so I have no sign that a test
shared a port. I did not check that on macOS or Windows.

#### The bytecode snapshot

`bundler_bytecode_portable.test.ts` pins the bytecode of a bundle of 64
libraries from `test/node_modules`. The bundle holds each package that a
library can load, also one that it loads in a `try` block and does not
declare.

`test/` uses the isolated linker. A package that is not declared is
still found through `node_modules/.bun/node_modules`, which has one
version of each installed package. Two libraries of the bundle load a
package that way:

| library | loads | with verdaccio | without |
|---|---|---|---|
| `mongodb` (`lib/deps.js`) | `aws4` | `aws4@1.13.0`, installed only for
`@cypress/request`, a dependency of verdaccio | not found, so not in the
bundle |
| `follow-redirects` (`debug.js`) | `debug`, an optional peer |
`debug@4.3.4` with `ms@2.1.2`. Only `@verdaccio/config` and
`@verdaccio/local-storage-legacy` resolve to that version |
`debug@4.4.0`, which `superagent` and others already bring |

So 7 modules of 3 packages leave the bundle: `aws4@1.13.0`,
`debug@4.3.4` and `ms@2.1.2`. Nothing is added. To check this, the
lockfile of c6da4a4 and the lockfile of this PR were installed in two
directories, and `libraries.js` was built in each with the command of
the test. The sha256 of the two `.js` outputs are the old and the new
value of the snapshot. A metafile names the two importers above.

The bytecode is 32632 bytes smaller: 21179744, where main has 21212376.
Before the rebase the two numbers were 21174272 and 21206912, with the
WebKit of that time. The new values are the same on every platform,
which is the case where the comment at the top of that file says to
update the snapshot.

#### Test timings

"Before" is main at c6da4a4, where the 31 files fork verdaccio.
"After" is this PR on the same commit.

**Local**

- Linux x64 VM, 12 vCPU (Intel Xeon, 2.90 GHz), 16 GB. bun is the canary
build e555359 for both sides.
- Each file ran alone: `bun test --timeout=90000 <file>`. The two trees
took turns on each file. The order of the two changed with each file and
each round. 5 rounds, 310 file runs.
- Wall-clock is from the start of `bun test` to its exit. CPU is the
user and system time of all processes of the VM in that time, from the
cgroup. It has the verdaccio process, which the test does not wait for.
- The table has the median of the runs that exited with 0, and the
lowest and the highest in brackets. Seconds.
- The after side is the commit 2d236bf. The commit after it changes
one test of `isolated-install.test.ts` and adds no wait for time.
- The VM lost CPU time to other VMs in each round (steal time), so the
wall-clock has noise. The rounds agree: before is 1.7 to 2.1 times after
in each one.

| file | tests | before, wall | after, wall | ratio | before, CPU |
after, CPU |
|---|---|---|---|---|---|---|
| `bun-add-catalog.test.ts` | 149 | 4.81 (4.34 to 6.17) | 2.15 (1.88 to
3.47) | 2.2 | 6.94 | 3.16 |
| `bun-add-filter.test.ts` | 123 | 4.67 (3.81 to 7.01) | 1.91 (1.84 to
2.19) | 2.4 | 6.75 | 3.10 |
| `bun-audit.test.ts` | 182 | 6.79 (6.43 to 9.03) | 5.21 (4.62 to 5.39)
| 1.3 | 10.43 | 6.83 |
| `bun-dedupe.test.ts` | 82 | 4.31 (4.10 to 4.74) | 2.19 (1.99 to 2.37)
| 2.0 | 6.96 | 3.61 |
| `bun-install-lifecycle-scripts.test.ts` | 128 | 10.40 (8.70 to 14.14)
| 8.51 (7.50 to 9.92) | 1.2 | 21.07 | 16.04 |
| `bun-install-native-binlink.test.ts` | 16 | 3.93 (3.71 to 6.41) | 1.79
(1.40 to 5.45) | 2.2 | 5.40 | 3.04 |
| `bun-install-patch.test.ts` | 31 | 3.93 (3.77 to 4.66) | 2.52 (2.04 to
2.88) | 1.6 | 4.15 | 2.09 |
| `bun-install-registry.test.ts` | 282 | 18.79 (16.54 to 26.41) | 15.38
(10.74 to 16.10) | 1.2 | 18.89 | 12.45 |
| `bun-lock.test.ts` | 40 | 3.98 (3.40 to 4.51) | 1.33 (1.27 to 1.59) |
3.0 | 4.48 | 1.61 |
| `bun-lockb.test.ts` | 8 | 2.29 (1.90 to 2.39) | 0.53 (0.43 to 0.64) |
4.3 | 2.45 | 0.52 |
| `bun-patch.test.ts` | 37 | 4.16 (4.05 to 6.92) | 3.10 (2.44 to 3.78) |
1.3 | 5.02 | 3.18 |
| `bun-pm-licenses.test.ts` | 80 | 5.16 (3.53 to 5.78) | 2.19 (1.53 to
2.36) | 2.4 | 6.61 | 2.91 |
| `bun-prune.test.ts` | 121 | 5.40 (5.14 to 5.63) | 2.80 (2.62 to 3.46)
| 1.9 | 8.18 | 4.38 |
| `bun-publish.test.ts` | 47 | 5.40 (5.03 to 6.02) | 2.30 (2.00 to 3.30)
| 2.3 | 5.42 | 2.15 |
| `bun-update-lockfile-sync.test.ts` | 77 | 4.22 (3.39 to 4.68) | 1.78
(1.52 to 2.15) | 2.4 | 5.95 | 2.78 |
| `bun-update-transitive.test.ts` | 224 | 8.46 (7.73 to 10.16) | 5.06
(4.44 to 8.29) | 1.7 | 13.50 | 8.56 |
| `bun-update.test.ts` | 159 | 8.28 (7.66 to 8.69) | 4.59 (4.27 to 5.02)
| 1.8 | 11.96 | 6.73 |
| `bun-workspaces.test.ts` | 82 | 3.56 (3.22 to 3.75) | 1.59 (1.48 to
1.77) | 2.2 | 4.85 | 2.42 |
| `catalogs.test.ts` | 89 | 5.90 (4.93 to 6.02) | 2.04 (2.00 to 2.20) |
2.9 | 7.00 | 3.28 |
| `config-precedence.test.ts` | 51 | 3.65 (2.81 to 4.11) | 0.90 (0.74 to
0.97) | 4.1 | 4.46 | 1.12 |
| `frozen-lockfile-missing-workspace.test.ts` | 1 | 1.79 (1.64 to 2.12)
| 0.15 (0.14 to 0.18) | 11.9 | 2.02 | 0.18 |
| `frozen-lockfile-pruned.test.ts` | 101 | 4.76 (4.05 to 5.96) | 1.83
(1.71 to 1.95) | 2.6 | 7.05 | 3.23 |
| `hoist.test.ts` | 1 | 1.75 (1.71 to 1.91) | 0.17 (0.12 to 0.21) | 10.3
| 2.09 | 0.17 |
| `isolated-install.test.ts` | 85 | 6.24 (5.94 to 7.74) | 3.59 (3.15 to
3.95) | 1.7 | 7.67 | 4.32 |
| `isolated-relink.test.ts` | 6 | 2.29 (1.89 to 2.96) | 0.35 (0.30 to
0.47) | 6.5 | 2.72 | 0.48 |
| `migration/pnpm-lock-v9.test.ts` | 84 | 3.86 (3.64 to 4.85) | 1.54
(1.34 to 1.83) | 2.5 | 4.57 | 1.89 |
| `migration/pnpm-migration.test.ts` | 3 | 1.89 (1.72 to 2.16) | 0.18
(0.16 to 0.22) | 10.5 | 2.29 | 0.23 |
| `nested-overrides.test.ts` | 144 | 8.95 (8.20 to 10.21) | 5.92 (4.50
to 8.86) | 1.5 | 15.03 | 9.69 |
| `npmrc.test.ts` | 47 | 2.52 (2.32 to 2.68) | 0.58 (0.55 to 0.61) | 4.4
| 2.91 | 0.65 |
| `public-hoist-pattern.test.ts` | 14 | 2.53 (2.44 to 2.62) | 0.46 (0.44
to 0.48) | 5.5 | 3.01 | 0.60 |
| `update_interactive_formatting.test.ts` | 24 | 2.99 (2.79 to 3.28) |
0.70 (0.55 to 0.99) | 4.3 | 3.93 | 1.11 |
| **sum** | **2518** | **157.7** | **83.3** | **1.9** | **213.8** |
**112.5** |

Each round, the sum over the files where both sides exited with 0:

| round | files | before, wall | after, wall | before, CPU | after, CPU
|
|---|---|---|---|---|---|
| 1 | 29 | 146.4 | 71.2 | 184.7 | 95.8 |
| 2 | 30 | 140.9 | 68.0 | 188.4 | 98.6 |
| 3 | 31 | 165.4 | 84.1 | 217.1 | 115.2 |
| 4 | 29 | 158.3 | 80.0 | 202.1 | 106.5 |
| 5 | 29 | 150.3 | 88.1 | 209.3 | 114.6 |

Runs that are not in the tables:

- 6 of the 155 runs with verdaccio did not start it. The log has
`Verdaccio exited with code 2 and signal null`. Code 2 is the exit of
verdaccio when it cannot listen (`build/lib/bootstrap.js`). `start()`
waits for a message that does not come. Five runs ended at the hook
timeout of 90 s. `bun-install-registry.test.ts` waits at module scope,
where no timeout applies: I stopped it after 678 s.
- I started verdaccio on a port that was in use, to check: the same
line, and `start()` did not settle.
- 1 of the 155 runs of this PR had one failing test: see the next
section.

**CI**

Build 124221 is main at c6da4a4. Build 124229 is this PR on the same
commit. Builds 124209 and 124201 are main, one and four commits earlier,
with the same 31 files. Each number is the sum over the 31 files of the
duration that the runner logs for a file. I read the logs with the
parser of `scripts/update-test-durations.mjs`. Seconds.

| lane | main 124221 | main 124209 | main 124201 | this PR 124229 |
|---|---|---|---|---|
| debian 13 x64 | 222.6 | 131.4 | 126.7 | 51.5 |
| ubuntu 25.04 x64 | 126.1 | 126.0 | 126.4 | 39.1 |
| alpine 3.23 x64 | 203.8 | 172.8 | 156.9 | 119.8 |
| debian 13 aarch64 | 141.7 | 145.6 | 141.2 | 45.6 |
| Windows 2019 x64 | 149.6 | 166.2 | 171.6 | 109.9 |
| Windows 11 aarch64 | 245.8 | 243.4 | 264.1 | 182.8 |
| **sum of the six lanes above** | **1089.6** | **985.4** | **986.9** |
**548.7** |
| debian 13 x64 ASAN | no tests | no tests | no tests | 340.2 |
| macOS aarch64 | no tests | no tests | no tests | 53.1 |
| macOS x64 | no tests | no tests | no tests | 67.1 |

- The 222.6 s of build 124221 have one run of 90 s:
`isolated-install.test.ts` on shard 8. The log has `Verdaccio exited
with code 2 and signal null` and the hook timeout. The second attempt
passed.
- The main builds have no test jobs for ASAN and macOS.
- A build is one sample. Apart from the 90 s run, the three main builds
differ by up to 47 s on a lane (alpine).
- Not each file is faster in CI. On each lane, 1 to 6 of the 31 files
took longer than in each of the three main builds. The largest
differences are on alpine: `bun-install-registry.test.ts` took 37.3 s
against 27.9 to 29.2 s, and `bun-update.test.ts` took 7.2 s against 2.5
to 2.7 s. That is one sample, and I do not know the cause. Locally each
file is faster.
- The four test files of `test/packages/registry/` take 1.5 to 4.8 s on
a lane.

Import cost, CPU, the lowest of the runs. Measured before the rebase, on
a build of 367d939:

| | release | debug |
|---|---|---|
| `harness.ts` | 29.5 ms | 1378 ms |
| the registry | 21.5 ms | 1162 ms |
| `node:zlib` alone | 11.5 ms | 615 ms |

#### A test that depended on the order of two packuments

`isolated-install.test.ts`, `ranged peer dependency resolution is stable
across installs from bun.lock`. The root depends on
`normal-dep-and-dev-dep` (which has `no-deps@1.0.1`), on
`two-range-deps` (`no-deps@^1.0.0`) and on `peer-deps-fixed` (peer
`no-deps@^1.0.0`). The test needs 1.0.1 and 1.1.0 in the graph.

- bun resolves `^1.0.0` to 1.1.0 when the packument of `two-range-deps`
is there first. When the packument of `normal-dep-and-dev-dep` is there
first, 1.0.1 is in the graph and `^1.0.0` resolves to it.
- I delayed one packument by 40 ms, with 12 installs for each case. A
slow `two-range-deps` gave one version 12 times. A slow
`normal-dep-and-dev-dep` gave both versions 12 times. No delay gave both
7 times and one 5 times.
- On main with verdaccio the test passed 50 of 50 runs. With the new
registry it failed 54 of 100 runs alone, and 10 of 20 runs of the file.
It fails with the older bun build 367d939 too (15 of 40), so the
rebase did not bring it.
- CI did not show it, before and after the rebase. CI sets
`BUN_INSTALL_CACHE_DIR`, so the tests of a file share one manifest
cache. With that variable the file passed 10 of 10 local runs. Without
it each test has its own cache.
- My local runs before the rebase did not show it. They were on a host
with a load average of 400 to 900.
- The test has its own registry now. Its `intercept` answers for
`normal-dep-and-dev-dep` after bun asked for `no-deps`, which bun does
when it has the packument of `two-range-deps`. There is no wait for
time. 100 of 100 runs alone and 20 of 20 runs of the file pass.
- After that change: 5 passes over the 31 files, and 5 runs in a row of
each file. 310 file runs, 0 failures.

Other tests can depend on the order in the same way. `hoisting > peers >
it should hoist 1.0.1 when peer *` does: see the flaky tests below. The
cause is in `bun install`, not in the registry: one `package.json` gives
two different trees.

#### Other things I found

- Before the rebase, `test/bun.lock` on main was stale: `bun install
--lockfile-only` on an unchanged main dropped five nested `react`
entries, and I kept them by hand. The lockfile of main is current now.
The lockfile diff here is what bun writes: verdaccio and its
dependencies go, and `debug` hoists to 4.4.0.
- `manifest conditional requests > a changed etag returns 200 and the
new etag is cached` failed once on unchanged main and passed on the next
run. It uses its own mock server, not the registry. It is in
`test/flaky-tests.txt`.
- On my local debug build, `Bun.serve` adds `content-type:
application/octet-stream` to an answer that has no body: a 304, and a
401. Two release builds, one of 367d939 and one of 36cd151, do not.
I did not find the cause and I saw it on one local build only. The tests
here assert the exact headers on the `Response` of the registry, and on
the wire only the headers that the registry sets.
- `Bun.serve` sets `SO_REUSEPORT` when it gets `development: false`. The
type of `reusePort` says that the default is false.
- `bun install` gives another tree for the same `package.json` when the
packuments arrive in another order: see the section on the order of two
packuments.
- `bun run` and `bun test` use a whole core, and do not end, while a
top-level await waits for a promise that nothing settles. A file with
`await new Promise(() => {})` used 9.9 s of CPU in 10 s. node prints
`Warning: Detected unsettled top-level await` and exits with 13. The
loop is `wait_for_promise` in `src/jsc/event_loop.rs`. This is how
`bun-install-registry.test.ts` on main hangs when verdaccio does not
start.

#### Flaky tests in files that this PR edits

Two tests of migrated files passed on a retry in the earlier builds of
this PR. Main has both without this change. In build 121299, the last
one before the rebase, no file of this PR was in the flaky annotations.
Files outside this PR were.

- `bun-install-registry.test.ts`, `hoisting > peers > it should hoist
1.0.1 when peer *`, on Windows 11 aarch64, with `a-dep@1.0.9`. The same
test fails the same way on the same lane in the main builds
https://buildkite.com/bun/bun/builds/120426 and
https://buildkite.com/bun/bun/builds/120659. I could read 11 main
builds. After the rebase it passed on a retry on ubuntu 25.04 aarch64 in
build 124229, again with `a-dep@1.0.9`. The version that it gets depends
on the order of the packuments.
- `bun-lock.test.ts`, `peer no published version satisfies > declared by
a registry package`. It runs against its own mock server.
`test/flaky-tests.txt` lists it at 34 of 80 builds.

I cannot say that the rate of the first one is unchanged. On my machine
it failed 1 time in 150 runs with this registry, and 0 times in 29 runs
with verdaccio.

A third test failed in my runs, and not in the builds of this PR:

- `bun-install-registry.test.ts`, `npm manifest cache entries are only
reused for the package name they were saved for`: `failed to open
manifest file ... ENOENT`. It failed in 1 of 8 runs of the file. Alone,
it failed in 2 of 40 runs with this registry, and in 5 of 40 runs on
main with verdaccio. `bun install` removes a manifest file that is not
valid and saves the new one in a task of the thread pool
(`Serializer::save_async` in `src/install/npm.rs`). I found nothing that
waits for that task before the exit. The test reads the file after the
exit. `test/flaky-tests.txt` has `failed to open ...` for this file.

#### Reviews of the diff

My review of the diff raised 9 concerns. 8 were addressed in full: the
header of a publish, an error in a handler, the export of the old name,
`pack()`, the claims about npm, #33115, the flaky tests, and the time of
the tests in `cli.test.ts`. One is addressed in part: the review named
six follow-ups, and these Notes list the three that I can describe.

The reviews on GitHub raised 12 more. All are addressed:

- The URL of a registry names the host that it listens on. It was
`localhost` for each host.
- A checkout with the leftovers of verdaccio failed the publish tests:
see the section above.
- The ignore rules for the leftovers stay.
- `intercept` gets a copy of the request, so it can read the body of a
request that the registry answers.
- The web login test fails with the output of bun when bun exits before
it asks for the session. Before, it waited for the timeout.
- Each test of the package disposes its registry when an assertion
fails.
- `VerdaccioRegistry` is removed. My review asked to keep the export for
open PRs. The review on GitHub asked to remove it, and `REVIEW.md` says
the same.
- A request that the registry refused left a part of its change in the
package: a `PUT` to `/<name>/-rev/<rev>` with a tag that is not valid,
and a deprecation with a message that is not a string. The checks come
before the first change now.
- In a search, a qualifier that matched took away the miss of a word
before it. A package is found when each word matches.
- A storage that is not a directory gave a registry that answered 404
for every package. The constructor throws now, and `cli.ts` prints the
message. A read that failed does not stay as the state of the package.
- `cli.ts` ended with a stack trace for a `--user` that the registry
refuses, for an option that it does not know, and for a port that is in
use. One handler prints the message and the usage for each refusal now.
- `cli.ts` took an empty `--port` as port 0, and it took `0x50` and
`1e3`. The value must be digits now.

One more finding got no change: a scope directory that another process
replaces with a file while the registry reads it. Nothing writes into
the storage while a registry runs, and a check before the read has the
same window. The reviewer withdrew the finding.

#### Related open PRs

These change `VerdaccioRegistry` in `test/harness.ts`. This PR removes
that class, so git reports a conflict, and the problem each one
addresses is not there with the new registry:

- #36352: `stop()` does not end the process. The new `stop()` closes the
server.
- #38891: the random port and a verdaccio that exits early. The new
registry listens on port 0 in the process of the test.
- #40219: `createTestDir()` removes the users. The new registry keeps
them.
- #40766: run verdaccio under node in CI.

These edit the line of `hoist.test.ts` that imports `VerdaccioRegistry`.
Git reports a conflict on that line. The new line imports `TestRegistry`
from `"registry"`:

- #43878
- #38976

I looked at the 500 open PRs that changed last. 72 of them change a file
under `test/cli` or `test/regression`, the harness, or a test with
`install`, `publish` or `registry` in its path. I read the diff of each.
No PR other than the ones above adds or changes a line with
`VerdaccioRegistry`.

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 0 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/cli/update_interactive_formatting.test.ts,
test/cli/install/migration/migrate.test.ts,
test/cli/install/isolated-relink.test.ts,
test/cli/install/isolated-install.test.ts,
test/cli/install/frozen-lockfile-pruned.test.ts,
test/cli/install/bun-workspaces.test.ts,
test/cli/install/bun-publish.test.ts,
test/cli/install/bun-prune.test.ts, test/cli/install/bun-patch.test.ts,
test/cli/install/bun-lockb.test.ts, test/cli/install/bun-lock.test.ts,
test/cli/install/bun-install-registry.test.ts,
test/cli/install/bun-install-patch.test.ts,
test/cli/install/bun-install-native-binlink.test.ts,
test/cli/install/bun-install-lifecycle-scripts.test.ts,
test/cli/install/bun-add-filter.test.ts,
test/bundler/bundler_bytecode_portable.test.ts

<!-- robobun:evidence:end -->

---------

Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
robobun added a commit that referenced this pull request Oct 14, 2026
On main `cd test && bun check` reports no type error (#44614). The tests
of this branch had 9, in quic-sni.test.ts and quic-stream.test.ts.

The edits are casts, `!` and one annotation. The transpiled output of
both files is byte-equal before and after.
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.

test.todo documentation snippet causes TypeScript error with @types/bun

2 participants