Skip to content

Wire up test runner and run unit tests in CI#3368

Open
jaisinha77777 wants to merge 3 commits into
fosrl:devfrom
jaisinha77777:chore/wire-up-test-runner
Open

Wire up test runner and run unit tests in CI#3368
jaisinha77777 wants to merge 3 commits into
fosrl:devfrom
jaisinha77777:chore/wire-up-test-runner

Conversation

@jaisinha77777

@jaisinha77777 jaisinha77777 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What

The repo already has several *.test.ts files (using the custom test/assert.ts harness), but nothing actually runs them:

  • there is no test npm script,
  • make test only launches the Docker image, and
  • the test.yml workflow only runs tsc --noEmit plus a curl smoke check.

So these tests can silently break without anyone noticing. This PR wires them up, and turning them on immediately surfaced a set of dead IPv6 tests in ip.test.ts that are fixed here.

Changes

  • test/run.ts - a small runner that discovers every *.test.ts file via Node's built-in fs.globSync and runs each one in its own node --import tsx child process. The test files are standalone scripts that call process.exit() / throw on failure, so per-file process isolation keeps one failure from aborting the rest of the suite. The runner aggregates results and exits non-zero if any file fails. No new dependencies (tsx is already a devDependency).
  • package.json - adds "test": "tsx test/run.ts".
  • .github/workflows/test.yml - adds a Run unit tests step (npm test) after the existing tsc step in the test job, reusing its existing DB/build setup.
  • server/lib/ip.test.ts - adds real IPv6 coverage. The IPv6 cases here were previously commented out and could not have run as written (see below).

The dead IPv6 tests in ip.test.ts

While wiring up the runner I found the commented-out IPv6 tests would not pass as written:

  • testCidrToRange used assertEqualsObj, which JSON.stringifys its arguments and throws on BigInt - so the entire block was dead code.
  • The commented IPv6 conversion expected an end spanning 2^64 addresses rather than 2^96, i.e. a /64 instead of the /32 under test.
  • The commented findNextAvailableCidr IPv6 case used 2001:db8::/32 and 2001:db8:1::/32, which both mask down to the same network, and expected 2001:db8:2::/32 from a /16 search that actually returns 2001::/32.

This PR re-enables testCidrToRange (comparing start/end as BigInt, and asserting the /32 spans 2^96) and adds correct IPv6 cases to findNextAvailableCidr: a gap allocation across two distinct /48 blocks (-> 2001:db8:2::/48), a no-space case, and the mixed-version guard. The implementation in ip.ts was already correct; only the tests were wrong.

Build awareness

server/private is licensed under the Fossorial Commercial License rather than AGPLv3. The runner is build-aware (matching the build === "oss" checks used elsewhere in the codebase): the OSS build skips server/private tests, while the enterprise and saas builds run them. So npm test is clean for OSS contributors out of the box.

The existing *.test.ts files were never executed: there was no test
script, 'make test' only launches Docker, and CI did tsc plus a smoke
check. Add a tsx-based runner that discovers every *.test.ts file and
runs each in its own process (the test files call process.exit, so
isolation keeps one failure from aborting the suite), expose it as
'npm test', and run it in the test workflow.
server/private is licensed under the Fossorial Commercial License, not
AGPLv3. Make the runner build-aware (matching the build === "oss" checks
used elsewhere) so the OSS build skips server/private tests while the
enterprise and saas builds still run them.
The IPv6 cases here were commented out and could not have run as written:

- testCidrToRange used assertEqualsObj, which JSON.stringifies its
  arguments and throws on BigInt, so the whole block was dead.
- The commented IPv6 conversion expected an end address spanning 2^64
  rather than 2^96, i.e. a /64 instead of the /32 under test.
- The commented findNextAvailableCidr IPv6 case used two inputs
  (2001:db8::/32 and 2001:db8:1::/32) that both mask down to the same
  network, and expected 2001:db8:2::/32 from a /16 search that actually
  returns 2001::/32.

Re-enable testCidrToRange comparing start/end as BigInt (avoiding the
assertEqualsObj BigInt issue) and assert the IPv6 /32 spans 2^96. Add
IPv6 cases to findNextAvailableCidr: a gap allocation across two
distinct /48 blocks, a no-space case, and the mixed-version guard.
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