Wire up test runner and run unit tests in CI#3368
Open
jaisinha77777 wants to merge 3 commits into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The repo already has several
*.test.tsfiles (using the customtest/assert.tsharness), but nothing actually runs them:testnpm script,make testonly launches the Docker image, andtest.ymlworkflow only runstsc --noEmitplus 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.tsthat are fixed here.Changes
test/run.ts- a small runner that discovers every*.test.tsfile via Node's built-infs.globSyncand runs each one in its ownnode --import tsxchild process. The test files are standalone scripts that callprocess.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 (tsxis already a devDependency).package.json- adds"test": "tsx test/run.ts"..github/workflows/test.yml- adds aRun unit testsstep (npm test) after the existingtscstep in thetestjob, 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.tsWhile wiring up the runner I found the commented-out IPv6 tests would not pass as written:
testCidrToRangeusedassertEqualsObj, whichJSON.stringifys its arguments and throws onBigInt- so the entire block was dead code.endspanning2^64addresses rather than2^96, i.e. a/64instead of the/32under test.findNextAvailableCidrIPv6 case used2001:db8::/32and2001:db8:1::/32, which both mask down to the same network, and expected2001:db8:2::/32from a/16search that actually returns2001::/32.This PR re-enables
testCidrToRange(comparingstart/endasBigInt, and asserting the/32spans2^96) and adds correct IPv6 cases tofindNextAvailableCidr: a gap allocation across two distinct/48blocks (-> 2001:db8:2::/48), a no-space case, and the mixed-version guard. The implementation inip.tswas already correct; only the tests were wrong.Build awareness
server/privateis licensed under the Fossorial Commercial License rather than AGPLv3. The runner is build-aware (matching thebuild === "oss"checks used elsewhere in the codebase): the OSS build skipsserver/privatetests, while the enterprise and saas builds run them. Sonpm testis clean for OSS contributors out of the box.