Skip to content

Improves test scripts, cross-platform compatibility and async test patterns#629

Open
uguraslan wants to merge 2 commits intolightning-js:devfrom
uguraslan:fix/test-scripts
Open

Improves test scripts, cross-platform compatibility and async test patterns#629
uguraslan wants to merge 2 commits intolightning-js:devfrom
uguraslan:fix/test-scripts

Conversation

@uguraslan
Copy link
Contributor

@uguraslan uguraslan commented Mar 6, 2026

While investigating some test related problems I found a few issues that needed fixing.

  • Removal of scripts/runTests.js
  • Updated test scripts, removed test:ci
  • Updated CI workflows
  • Fix scripts/prepublishOnly.js
  • Fix assert.end() in async test callbacks
  • Improved tests in scripts/prepublishOnly.test.js

Removal of scripts/runTests.js

This script was originally added to work around a bash extglob pattern that doesn't work on Windows. The problem is it spawns tape as a child process which causes different event loop timing. This was silently masking some test failures depending on how tests were run.

Replaced it with a direct tape CLI call using explicit glob patterns that tape expands internally:

node -r global-jsdom/register node_modules/tape/bin/tape "scripts/**/*.test.js" "src/**/*.test.js"

No custom runner needed, no buffering, no child process overhead.
Tested on Windows 11 Pro and MacOS.

Updated test scripts, removed test:ci

test:ci had c8 on top of test:run but the GitHub workflows were using their own hardcoded commands with c8 anyway, so test:ci was never actually called from CI. It wasn't adding any value, just sitting there as an unused script.

Updated CI workflows

With runTests.js the workflow was parsing its custom output format (passed: N failed: N) to detect failures. Switched to:

  • Process exit code (steps.run-tests.outcome) as the failure signal. With this change we can detect silently failing tests due any problem so it is more reliable
  • Native TAP output (# tests, # pass, # fail) for the PR comment summary

Also updated master-coverage.yml to use npm run test:run instead of its own hardcoded tape command so it stays in sync.

Fix scripts/prepublishOnly.js

When the precompiler processes a non-Blits file it returns undefined. Without a guard this meant ESLint was being called on every non-Blits file unconditionally. Added newSource !== undefined check before comparing with source.

Fix assert.end() in async test callbacks

tape docs are clear on this:

"Do not call t.end() if your test callback returns a Promise."
https://github.com/tape-testing/tape/blob/master/readme.markdown#tenderr

"Explicitly calling t.end() while also returning a Promise that fulfills is an error."
https://github.com/tape-testing/tape/blob/master/readme.markdown#testname-opts-cb

An async function always returns a Promise so assert.end() should never be called inside one. Went through all 21 async callbacks across three files and removed them:

  • src/application.mouse.test.js: 9 async tests, removed assert.end() from each
  • src/router/router.test.js: 10 async tests, removed assert.end(). 1 test had no await at all so converted it to a sync callback
  • scripts/prepublishOnly.test.js: 1 async test (Setup), removed assert.end()

Improved tests in scripts/prepublishOnly.test.js

While going through the async test cleanup, I noticed that the processDirectory tests were missing the execStub. The stub for childProcess.exec was only set up in the Setup test but never passed down or recreated in the individual processDirectory tests. So real exec calls were running. Added execStub = sinon.stub(childProcess, 'exec') and execStub.restore() to each of those tests.

Also rewrote the formatFileWithESLint tests. They were just checking that the function doesn't throw. Replaced them with proper behavior tests that verify the exact ESLint command being called, the error handling (stderr output, error messages), and the silent success case.

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Test Results: ✅ PASSED

Run at: 2026-03-06T22:59:20.677Z

Summary:
passed: 1117 failed: 0 of 1117 tests

@github-actions
Copy link

github-actions bot commented Mar 7, 2026

Test Results: ✅ PASSED

Run at: 2026-03-07T07:58:52.630Z

Summary:
passed: 1117 failed: 0 of 1117 tests

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.

1 participant