Fix broken tests on main #165
Conversation
| async function t(test) { | ||
| const testName = test.name; | ||
| const match = (process.argv[2] || '').toLowerCase(); | ||
| if (test.name.toLowerCase().indexOf(match) > -1) { | ||
|
|
||
| if (testName.toLowerCase().indexOf(match) > -1) { | ||
| if (match) console.log(`Testing ${test.name}`); | ||
| try { | ||
| await test(); | ||
| console.log(`✅ ${testName}`); | ||
| } catch (e) { | ||
| console.error(`\n\nTest "${test.name}" failed with error:`); | ||
| console.log(`❌ ${testName}`); | ||
| throw e; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This just provides some visual differentiation in the logs
| const testsExpectingExit /*: Map<string, number> */ = new Map(); | ||
| // $FlowFixMe | ||
| process.exit = (code = 0) => { | ||
| const callingTest = getCallingTestFn(); | ||
| if (callingTest && testsExpectingExit.has(callingTest)) { | ||
| testsExpectingExit.set(callingTest, code); | ||
| return; | ||
| } | ||
|
|
||
| // $FlowFixMe: bad libdef | ||
| assert.fail(`'process.exit' was unexpectedly called with code '${code}'`); | ||
| }; |
There was a problem hiding this comment.
There are a number of tests that use expectProcessExit, but only on commands they're expecting to fail. When commands unexpectedly fail, a lot of them will directly call process.exit via spawnOrExit:
Lines 198 to 204 in 172581c
This causes the tests process to immediately exit without reporting which test failed or showing any stack traces.
The mechanism I added here allows expectProcessExit to behave exactly as it did before, but also detects unexpected calls of process.exit and instead throws an assertion error, scoping it to the call stack of the test that caused it, and allowing the test runner to properly report the failure. This makes debugging tests much easier.
The new getCallingTestFn (a little hacky, but works reliably) is used because a lot of tests run in parallel, so we need to be able to identify which test caused process.exit to be called, and only suppress the ones that are expected via expectProcessExit.
You can see how helpful this new mechanism is when the testBazelBuild test fails. Previously the test process exited without any helpful reporting, but with the new mechanism it's very clear what failed and where:
before
after
There are some tests that are currently broken on main. Adding
.bazelrcand.bazelversionfiles to the relevant fixtures fixes them.I also included some small improvements to the devxp of the test runner, as it was previously quite difficult to debug tests or tell which test failed in some cases (see inline comments for context).