ci: run the web unit tests - #334
Merged
Merged
Conversation
The web package's 219 vitest tests ran nowhere. ci.yml only built web (tsc --noEmit plus vite build) and mobile.yml was the sole workflow calling npm test, so a failing web unit test could not turn a check red. The gap was wider than one missing suite. web/src/shared/crypto/interop-fixtures.test.ts is a generator rather than a test: it writes testdata/crypto/*_fixtures.json from the current TypeScript crypto and only asserts a count. The Go interop tests in internal/domain/crypto and internal/application/clientcrypto then read those files back. With the generator never running in CI, that pair only ever proved Go could still decrypt fixtures committed from a laptop months earlier, not that the two implementations agree today. A change to the TypeScript crypto that broke compatibility would have sailed through green. Running npm test ahead of make test closes both halves at once: fresh fixtures from the current TypeScript, read back by the current Go. The step ordering is load-bearing, so it carries a comment saying so. Verified locally end to end - 219 tests pass, both fixture files regenerate, and all four Go interop tests pass against the regenerated values. The regenerated fixtures are random per run and dirty the working tree, which is harmless here: the only tree check in this job is git diff --exit-code scoped to go.mod and go.sum.
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.
Follow-up to #333, which covered the extension. This closes the same class of gap on the web side.
The gap
The web package's 219 vitest tests ran nowhere.
ci.ymlonly built web (npm run buildistsc --noEmit && vite build), andmobile.ymlwas the only workflow in the repo callingnpm test. A failing web unit test could not turn a check red.It goes further than one missing suite.
web/src/shared/crypto/interop-fixtures.test.tsis a generator, not a test: it writestestdata/crypto/*_fixtures.jsonfrom the current TypeScript crypto and only asserts a count. The Go interop tests then read those files back:internal/domain/crypto/interop_test.go(AES-GCM, HKDF, padding)internal/application/clientcrypto/interop_test.go(Argon2id)With the generator never running in CI, that pair only proved Go could still decrypt fixtures committed from a laptop months ago - not that the two implementations agree today. A change to the TypeScript crypto that broke cross-language compatibility would have gone through green.
The fix
One step,
npm testinweb, placed ahead ofmake test. That closes both halves at once: fresh fixtures generated from the current TypeScript, read back by the current Go.The ordering is load-bearing, so it carries a comment explaining why. Reorder it after
make test, or drop it, and the interop tests quietly go back to validating stale committed data.Verification
Run locally end to end:
actionlintclean.Note
The regenerated fixtures are random per run and dirty the working tree. Harmless here: the only tree check in this job is
git diff --exit-codescoped togo.mod/go.sum. It does mean the files show as modified after any localnpm testin web, which is pre-existing behaviour and not changed by this PR.