Repository navigation
Conversation
…are-one-realm.test.ts The first two tests each built Bun's own library as the lowest layer of a run. That layer is the same program for both, so their test files are now one project, which runs once under plain Bun and once in layers. The third test keeps its own project. Each run now states how it ends: the summary, the exit code, no dead worker, and how many test files go to a program. The tests state which statements come through, where a value differs between a test file and a file of the project, and every line of the third project.
|
No actionable comments were generated in the recent review. WalkthroughThe test suite compares plain Bun and AOT layered runs for generated statements and values. It also checks shared-realm observations against expected answers and verifies run summaries. ChangesShared-realm test coverage
Possibly related issues
Suggested reviewers: Merge Risk:
|
|
Status: ready for review. This is a test-only change to The file passes on its three lanes at the head commit, in build #137284. That build is red for 28 other test files. All of them also fail in the builds of recently merged PRs. This PR changes none of them. How this was checked:
|
There was a problem hiding this comment.
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 checked the hardcoded summary at test/aot/layers-of-a-test-run-share-one-realm.test.ts:95 against the fixture: made-by-the-library.txt has 118 expressions, one of which throws, so 118 statement files + 117 value files = 235 files and 352 tests, 351 pass / 1 fail — the arithmetic matches. The 600000 ms per-test timeouts predate this change rather than being introduced by it.
Extended reasoning...
The change rewrites one AOT test file so two tests share a single pair of plain/--aot runs and pins exact summaries, exit codes, SEEN line sets and a new 57-line answers.txt fixture; it touches no production or security-sensitive code. Three inline findings about environment-dependent failures and a non-discriminating probe are already attached, so this body only records the fixture-count check that was ruled out.
…of-a-test-run-share-one-realm.test.ts A run that ended otherwise than stated failed on its summary alone, before its lines were compared. Each test now compares the lines and the end of a run in one object, so a failure names the expression. Two probes of the third project could not fail. The probe of Error.prepareStackTrace printed "text" for every string, prepared or not: it now prints what the stack begins with. The stack limit was 3, over the two frames that a stack has there: it is now 1. The first two tests start a program named `true`. Where the system has none they are skipped, and the file says why on stderr.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
The follow-up commit addresses the three points from the earlier review (lines compared together with the run's end, the prepared probe now pins prepared n, and the true-less skip). Beyond the inline note, I also checked the hardcoded summary against the fixture: 118 expressions give 118 statement files plus 117 value files (235) and 118 + 234 tests (352), with only index 057 failing, so " 351 pass / 1 fail / 235 files" is consistent; and the memoized seenOnce result holds only strings, so disposing the temp dir after the first awaiter is fine.
Extended reasoning...
Test-only change to one AOT test file and its fixtures; no runtime source, no security-sensitive surface. The second push reworked assertions to compare SEEN lines and run summary in one object, fixed two probes in the third project's fixture, and gated the first two tests on Bun.which("true"). The restructuring changes what the first test's lowest layers contain (its files now sit above the second test's project file, as the PR notes), which is a trade-off an AOT maintainer may want to weigh, so a human look remains worthwhile.
…-realm.test.ts with either line ending A checkout that converts text files ends the lines of answers.txt with \r\n. The lines were split at \n alone, so each stated answer kept a \r and the third test failed there.
Problem
test/aot/layers-of-a-test-run-share-one-realm.test.tstakes 307 s on debian 13 x64-asan (build #137158). Each of its three tests builds Bun's own library again: 70 to 90% of a run's CPU.Fix
Error.prepareStackTrace,Error.stackTraceLimit). Each now can.bun bd testtakes 273 s before, 208 s after. Release build, 4 CPUs: 23 s before, 18 s after.Background
bun test --aotin layers compiles the packages with Bun's library, then the project's files, then the test files, a few to a program.main.Bun.jest, so the third test's lowest layer would be built for mocks. Considered concurrent tests: AOT: hangs, wrong answers, failed builds;bun test --aotfollows #44833's mocks #47605 keeps this file one by one.Downsides
main).true, the first two tests are skipped and say why.mainruns them there.Notes
Where the time goes. One run in layers of each project of
main, release build, 12 CPUs, withBUN_JSC_logAOTPhaseTimes=1. "CPU" is user plus system time.src/all.jssrc/shared.jsWith the debug build the lowest layer takes 75 s of the 125 s of the shared run, and 85 s of the 108 s of the third project's run. The plain runs take 0.1 to 0.4 s (release) and 2 to 7 s (debug).
What the shared project builds. The build of a layer prints its size with
BUN_JSC_logAOTPhaseTimes=1. Release build.mainsrc/all.jshave the same bytes as onmain. Only the names of the test files differ.000-statement-000.test.js,000-value-000.test.js), so a program holds files of one test.mainand compile the same number of library files themselves (0, or 10 to 13). The last program holds029-statement-116,029-statement-117and029-value-117. Onmainthose three files are two programs.main.--isolategives each test file a new global object, a newprocess.stdoutand newperformanceentries, so a file sees nothing of the file before it.Why the third test keeps its project. The build scans the test files of a run before it builds a layer. One expression of the first two tests is
typeof Bun.jest. The scan then saysmocks any module trueandspies on objects from somewhere truefor the whole run. The third project saysfalsefor both.mainBun.jestbeside its twoThe first project without that expression says
falsefor both too, and its lowest layer has 90,379 call sites, not 90,374. So one project for all three tests changes what each test builds. This PR changes no expression.Why the tests stay one after another. With
--concurrentthe new file takes 10.6 to 12.4 s where it takes 14.5 to 17.4 s one by one (release, 12 idle CPUs, 3 rounds in turn). A lane of CI has no idle CPUs: the file runs in a batch, seven files at a time on 8 CPUs. #47605 importstestOneByOnefor this file, with the reason that a case beside others passes its limit under a sanitizer. The hunk of #47605 applies on top of this PR, and the tests of this PR need nothing else from it.What runs at once. The plain run and the run in layers of a project start together. Debug build, in turn: 247 s and 251 s one after the other, 228 s and 239 s together.
Local timing. Linux x64, 12 CPUs, 16 GB. The host takes a changing part of the machine, so wall time moves between rounds. User time is steady.
Debug build with ASAN. "The binary" is
build/debug/bun-debug test, whichbun bd teststarts. The rows are in the order in which they ran. The last five ran one after another, hours after the others.bun bd testbun bd test, a draft with the same runsbun bd test, the first commitbun bd test, the second commitbun bd testbun bd test, the second commitbun bd test, the third commitbun bd testruns the file as it is onmain: 3 pass. In the last pair of "Before" and "After" the tests take 91 s, 98 s and 82 s before, and 115 s, 0.05 s and 91 s after.Release build (
build/release/bun-profile test), rounds in turn:taskset -c 0-3)Six more runs of the new file with the release build passed: 12.5 to 15.3 s.
Assertions, before and after.
worker crashedfails the run in layers351 pass,1 fail,Ran 352 tests across 235 files.and exit code 1. For the third project:12 pass,0 fail,Ran 12 tests across 2 files.and exit code 0. No lineworker crashedin either. A test compares this in one object with the lines of the run, so a failure shows bothlayers 2, byrun()of the harnesstest files 235 | to a program 4, andtest files 2 | to a program 1ownand onetheirsline for each of 117 expressions, none saysthrew, and the three that differ are stated with both valuesanswers.txt, each twice, for both runsThe one statement that throws makes a
console.Consoleover two objects that can only write. Node v26.3.0 throws there too:TypeError: stream.removeListener is not a function.One more statement starts a program named
true. WhereBun.which("true")finds nothing, the first two tests are skipped, and the file says why on stderr. With an emptyPATHthe file ends with1 pass,2 skip,0 fail. With the skip taken out there, the first test fails and shows- "SEEN 027 came through"beside350 passand2 fail.The three values that differ under plain Bun show one realm. The file of the project answers second. It counts the listener that the test file added to
process.stdout(1, then 2), and theperformancemark of the test file (1, then 2).import.meta.mainistruein the test file andfalsein the file of the project.Two faults, each put into a copy of both versions of the file and run with the release build:
main's fileExpected: "test files 235 | to a program 4",Received: "test files 235 | to a program 16"Two more, in this PR's file alone. With one answer taken out of
answers.txt, the third test fails and shows the line twice. With the test file of the second test kept for the expression that throws, and the counts raised for it, the second test fails on the two lines that the run in layers words otherwise.One run for two tests. If the shared run fails, the first two tests fail together, with the same message.
Two probes of the third project. Both are in the fixtures of
main, and neither could fail.mainError.prepareStackTraceset by the test file, a stack made in each placeshared.txtprintedtextfor every stack that is a string, prepared or notprepared nin all three placesError.stackTraceLimitset by the test file, a stack made in each place3 31 2Plain Bun and the run in layers print the new lines, with the release build and with the debug build.
Removed. The word
TestRunPartsGoOnAfterCrashinBUN_DEBUG_TEST_AOT_WORDS. No code insrc/reads it, and the release binary does not hold the text.Not changed. The three titles, the expressions, the flags of the inner runs (
--isolate --parallel=4 --timeout=60000), the limit of 600 s a test, and the files undertest/aot/harness/.asks.txtandshared.txtchange in the two probes above and nowhere else.Line endings.
listedsplits the fixtures that are lists at/\r?\n/. With a copy of the four fixtures that ends every line with\r\n, the three tests pass. The second commit alone fails the third test there, because each stated answer keeps a\r.Not run. Windows and macOS: the file runs on the three Debian lanes only. A release build with ASAN: a cold build of it did not fit the 16 GB of this machine, so the debug build stands in for the lane with a sanitizer.
CI. The file passed on its three lanes in build #137272 (the first commit) and in build #137284 (the third commit).
main, build #137158The column of
mainand the columns of this PR are not like for like. Onmainthe file runs in a batch, seven files at a time on x64-asan. CI runs a file that a PR changes in a process of its own, beside one or two other files.[auto-merge] gate passed · iteration 0 · 4 files touched
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 0
evidence per changed file