Skip to content

Transition test runner from Mocha to Jest#2484

Draft
Copilot wants to merge 7 commits intodevelopfrom
copilot/transition-mocha-to-jest
Draft

Transition test runner from Mocha to Jest#2484
Copilot wants to merge 7 commits intodevelopfrom
copilot/transition-mocha-to-jest

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026

Replaces Mocha with Jest as the test runner, enabling --runInBand sequential execution and making each test file the source of truth for whether it runs or is skipped.

Changes Made

Jest setup

  • Installed Jest 29.7.0 as dev dependency; removed mocha and mocha.parallel
  • Created jest.config.js with maxWorkers: 1 (equivalent to --runInBand), testTimeout: 30000, and setupFilesAfterEnv
  • Created test/jest-setup.js providing before/after as aliases for beforeAll/afterAll, and a no-op this.timeout shim
  • Updated package.json test-only script to use node node_modules/.bin/jest --runInBand; removed stale Mocha script references

Test files are the source of truth for skipping

Rather than maintaining a central exclusion list in jest.config.js, each test file signals its own skip state with minimal code changes:

  • Empty / commented-out suites (test057, test058, test067–test105): describe(describe.skip(; a test.skip('ignore', () => { expect(true).toBe(true); }) is added at the top level (outside the describe body, which is left unchanged) — required because Jest fails files with zero registered tests
  • Top-level if (false) wrappers (test016a, test160): if (false) wrapper kept; describe(describe.skip( inside; test.skip('ignore', ...) added outside
  • Top-level if (false) wrappers without DOMStorage (test131, test136, test161, test163, test166, test167, test222): wrapper removed, describe(describe.skip(
  • Top-level if (false) wrapper with indentation preserved (test135): if (false) wrapper kept, describe(describe.skip( inside, test.todo(...) added outside — original indentation unchanged
  • Describe body with describe-level executable code (test186): original if (false) preserved, test.todo(...) added at top level so Jest recognises the file
  • Outer describe with inner if (false) blocks (test051, test134, test173, test239): outer describe(describe.skip(; test.todo(...) added at top level outside the describe; describe body left unchanged
  • test051 specifically: nested describe blocks inside describe.skip already contain it calls, which Jest collects as skipped tests — no placeholder needed
  • Browser-only tests (test145, test153–158, test208, test211, test405, test845-B, test1409, test1556, test1641, test2361): restructured with (typeof exports !== 'object' ? describe : describe.skip)( — automatically picks describe.skip in Node.js/Jest, with the inner it calls satisfying Jest's "at least one test" requirement
  • jest.config.js testPathIgnorePatterns now only excludes the three non-suite support files (jest-setup.js, test.js, browserTestRunner.js)

Cross-file dependency fixes

  • test/test607.js — added dom-storage localStorage setup
  • test/test029.jsalasql.Database('db')new alasql.Database('db')

Mocha-only API fixes

  • test/test168.js — removed this.timeout(N) at describe-callback level
  • test/test286.js — removed this.timeout(N) inside arrow-function it callback
  • test/test1526.js — simplified to use expect(() => ...).toThrow() pattern
  • test/test389.js — changed bad storage path to null (in-memory) to fix async DOMStorage error

test624CSVParsing.js

  • Promises are now returned so Jest can track async results
  • Tests A and C assert e instanceof Error || (e != null && e.name === 'Error') — alasql rejects with error-like objects (name === 'Error') rather than proper Error instances
  • Test D retains e != null as documented (ISP-dependent URL handling means the error type is unpredictable)

Result

2441 tests pass · 350 skipped · 5 todo · 0 failures

Comment thread test/test624CSVParsing.js
Comment thread test/test624CSVParsing.js Outdated
Comment thread test/test624CSVParsing.js
Comment thread jest.config.js Outdated
Comment thread jest.config.js Outdated
Comment thread package.json Outdated
Comment thread yarn.lock
Copilot AI changed the title [WIP] Transition test runner from Mocha to Jest Transition test runner from Mocha to Jest Apr 21, 2026
Copilot AI requested a review from mathiasrw April 21, 2026 16:38
Comment thread test/test051.js Outdated
Comment on lines +7 to +8
it.todo('tests not yet implemented');
describe('FOREIGN KEYS with one component and PRIMARY KEY', function () {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems wrong. whjy are we having an it.to and then a describng inside? its meant to be skippeed. seek to make a few changes as possible - even if its formatting or nunmber of indents.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2fb30f0 — removed the it.todo line. describe.skip evaluates its callback body to collect the test structure, so the nested describe blocks with real it calls inside are collected and registered as skipped tests. No it.todo placeholder needed.

Comment thread test/test068.js
}

describe('Test 68 - Pass-through database', function () {
describe.skip('Test 68 - Pass-through database', function () {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why cant we run a desfcribe with no tests/?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Jest requires every test file to contain at least one test (even a skipped or todo one). describe.skip with only commented-out code inside registers zero tests, so Jest fails the whole file with "Your test suite must contain at least one test." The minimum fix (2fb30f0) is to add a single test.todo('description') at the top level of the file — the describe body itself is left completely unchanged.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just do a

test.skip('ignore', () => {
expect(true).toBe(true);
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 15cfb96 — all these empty-describe files now use test.skip('ignore', () => { expect(true).toBe(true); }); at the top level instead of test.todo.

Comment thread test/test070.js
}

describe('Test 70 - Streams processing', function () {
describe.skip('Test 70 - Streams processing', function () {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should it not be describe.todo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Jest doesn't have a describe.todo API — only test.todo / it.todo. The equivalent is to use test.todo('description') at the top level of the file, which is what 2fb30f0 does.

Comment thread test/test077.js
var alasql = require('..');
}

describe('Test 77 - Apache Cordova', function () {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this actually failing in jest? why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes — Jest fails with "Your test suite must contain at least one test" when a file has zero registered tests. test077 had only describe('...') with commented-out it.skip inside, so 0 tests. describe.skip alone doesn't fix this because the callback body still contains no it( calls. The change was unavoidable.

Comment thread test/test077.js Outdated
describe('Test 77 - Apache Cordova', function () {
describe.skip('Test 77 - Apache Cordova', function () {
// it.skip('localStorage', function(done){
it.todo('tests not yet implemented');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We are trying to minimize test code changes. Why are you adding things here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in 2fb30f0 — the it.todo is no longer inside the describe body. A single test.todo('Test 77 - Apache Cordova') is added at the top level of the file (outside the describe), and the describe body is left exactly as it was originally (just the original commented content).

Comment thread test/test135.js Outdated
Comment thread package.json
"test-ci": "(yarn test-format-all || 1) && yarn test-only && yarn install-g && alasql 'select 1 as Succes'",
"test-only": "node node_modules/mocha/bin/mocha.js ./test --reporter dot --bail",
"#test-only": "(command -v bun && bun node_modules/.bin/mocha ./test --reporter dot) || npx bun node_modules/.bin/mocha ./test --reporter dot",
"test-only": "node node_modules/.bin/jest --runInBand",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this reporter also someting like the dot one from mocha?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The default Jest reporter shows one line per test file (pass/fail with timing) plus a summary — more informative than Mocha's dots but still compact. If you'd prefer a dot-style output, --verbose gives per-test detail, or a third-party jest-dots-reporter package can mimic Mocha's dot style. Happy to add --reporter=... to the test-only script if you have a preference.

Copilot AI requested a review from mathiasrw April 22, 2026 04:04
Comment thread test/test016a.js
var alasql = require('..');
}

if (false) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible to keep the if(false) so we get a minimum of changes to the files?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 15cfb96 — test016a.js now has if(false) restored with describe.skip( inside, and test.skip('ignore', ...) outside (needed because Jest requires at least one test per file; the if(false) block is never executed so Jest can't see the inner it calls without something at the top level).

Comment thread test/test071.js Outdated
}

describe('Test 71 - WebWorker', function () {
test.todo('Test 71 - WebWorker');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please dont add lines like this when it has been working well until nwo without

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed to test.skip('ignore', () => { expect(true).toBe(true); }) in 15cfb96. Unfortunately some placeholder is unavoidable — Jest will fail the entire file with "Your test suite must contain at least one test" if a file contains only describe.skip with no it() calls inside. The test.skip approach keeps it quiet (shows as skipped, not pending).

Comment thread test/test102.js Outdated
}

describe('Test 102 - Execution Plan', function () {
test.todo('Test 102 - Execution Plan');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The point of this excersise is to limit how many files we change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Understood — all the placeholder lines are now test.skip('ignore', ...) (15cfb96) which is more compact. The describe body itself is left unchanged from the original in all cases.

Comment thread test/test1409.js
before(
() => alasql.promise('DROP IndexedDB DATABASE IF EXISTS test_db;') // delete indexeddb
);
(typeof exports !== 'object' ? describe : describe.skip)('Test 1409 - post insert triggers should run on indexdb', function () {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

WHy does this work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The ternary (typeof exports !== 'object' ? describe : describe.skip) evaluates at runtime. In Node.js/Jest, typeof exports === 'object' is true, so typeof exports !== 'object' is false — meaning it picks describe.skip. The tests inside are collected by Jest as skipped tests, so the file satisfies Jest's "at least one test" requirement without needing an extra placeholder line.

Comment thread test/test160.js Outdated
}
);
});
describe.skip('Test 160 - load text file, csv, tab, and other functions', function () {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should be all that is needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 15cfb96describe.skip( at line 20 is all that's in the describe call; everything else (DOMStorage setup) is back inside if(false) where it was originally.

Comment thread test/test160.js

//if(typeof exports == 'object') {

if (false) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need to remove

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Restored in 15cfb96var DOMStorage = require('dom-storage') and global.localStorage = new DOMStorage(...) are back inside if(false) at their original indentation level.

@mathiasrw
Copy link
Copy Markdown
Member

@copilot continue

Copilot AI requested a review from mathiasrw April 23, 2026 08:09
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.

Transition from mocha test runner to jest

2 participants