Issue 311: the media catalogue's check ran no tests for its TypeScript modules
Its check declared only Go, so ten modules merged with nothing of their own run, and a Go module without tests would have passed too.
This commit is contained in:
@@ -0,0 +1,78 @@
|
||||
---
|
||||
status: located
|
||||
opened: 2026-10-08
|
||||
located-in: [mesh-media-catalog merge-check.sh]
|
||||
fixed-by: mesh-media-catalog#14
|
||||
amended-design:
|
||||
---
|
||||
|
||||
# 311 — The media catalogue's check ran no tests for ten of its twelve modules
|
||||
|
||||
## What was observed
|
||||
|
||||
The media catalogue's `merge-check.sh` is the second layer of its merge check, `mesh/repo-check`. It runs
|
||||
the Go tests of every module a change touches, in the Go toolchain. Two of the catalogue's twelve modules
|
||||
are Go: the media server's and the music manager's. The other ten are TypeScript, and for each of them the
|
||||
script said only that it was TypeScript and that the gate would judge its manifest. A change to any of the
|
||||
ten therefore ran nothing of its own before it merged, and the layer still passed.
|
||||
|
||||
The gap was not a lack of tests. Six of the ten TypeScript modules carry unit tests, and so does the music
|
||||
manager's TypeScript downloads step: the subtitle manager, the book manager, the request portal, the
|
||||
movie and series managers, and the watch-statistics app. Those tests cover how each one writes a binding
|
||||
into its app, refuses a credential the provider rejects, and leaves another person's entries alone.
|
||||
Nothing on the build seat ran them. The other four (the indexer proxy, the metadata manager, and the two
|
||||
download clients) have no tests at all, and nothing reported that either.
|
||||
|
||||
The Go side had a quieter version of the same gap. A Go module with no test files passed `go test` with
|
||||
nothing run, so the layer that exists to run a module's tests could go green on a module that has none.
|
||||
|
||||
## Why it is an issue
|
||||
|
||||
ADR 0237 (as amended) says every repository runs its own check in the toolchain it declares, and that
|
||||
what is not checked is reported, never passed silently. The catalogue's script declared only Go. Issue 302
|
||||
gave a repository more than one toolchain (`# mesh-check-also:`), and the catalogue had not used it. The
|
||||
rule against passing silently was kept in words, because the script printed "not tested here". But no part
|
||||
of the check could close the gap, so the TypeScript tests were proven only on a workstation.
|
||||
|
||||
## Fix
|
||||
|
||||
**The catalogue declares its TypeScript part.** `merge-check-typescript.sh` runs in the TypeScript
|
||||
toolchain after the Go script. For every touched module that has a `package.json`, it runs that module's
|
||||
`test/*.test.ts` under Node's own test runner with types stripped, which is what the module's own test
|
||||
script does. It installs nothing: the tests import only Node and the module's own sources, and the check
|
||||
holds no credential for the forge the SDK comes from. When a test imports the compiled step, that step is
|
||||
transpiled first with the toolchain's compiler and without type-checking. The SDK's types cannot be
|
||||
resolved from the clone, and the build, which can resolve them, does the type-checking. A TypeScript module
|
||||
with no tests is reported as not tested on every run that touches it.
|
||||
|
||||
**A Go module without tests fails.** In the Go script, a touched module that has Go code and no test file
|
||||
fails the layer by name. The rule is phased in: it applies only to modules that have Go code, and both of
|
||||
those already have tests. A TypeScript module, or a module that is only a container, is not held to it.
|
||||
|
||||
## Modules and their tests
|
||||
|
||||
As of the fix:
|
||||
|
||||
1. Go, tested under the race detector, with `go vet`: the media server (34 tests), the music manager (23).
|
||||
2. TypeScript, tested by the new part: the subtitle manager (8), the book manager (16), the music
|
||||
manager's downloads step (16), the request portal (21), the movie manager (16), the series manager (16),
|
||||
the watch-statistics app (11, five of which need python3 and skip where the toolchain has none).
|
||||
3. TypeScript with no tests, reported on every run: the indexer proxy, the metadata manager, and the
|
||||
two download clients. They are the candidates to port to Go, which would bring them under the rule above.
|
||||
They are not ported here.
|
||||
4. No module is only a manifest. Every module carries code of the mesh's own.
|
||||
|
||||
## How it is checked
|
||||
|
||||
**The check:** `mesh/repo-check` on every catalogue pull request that touches a module. Its report shows
|
||||
the TypeScript part (`merge-check-typescript.sh (typescript)`) with a `testing modules/<name> (TypeScript)`
|
||||
line for each touched TypeScript module that has tests, and a `NOT TESTED` line for each one that has none.
|
||||
|
||||
- By hand, `merge-check.sh` runs both parts with every module named in `MESH_CHECK_MODULES`. The TypeScript
|
||||
part was also run inside the TypeScript toolchain's image with no network, and every test passed.
|
||||
- The rule against a Go module without tests was proven with a module that has `go.mod` and a `main.go`
|
||||
and no test: the layer failed and named the module.
|
||||
- This is not a core issue (the media catalogue), so it has no replay.
|
||||
|
||||
The live proof is the first catalogue pull request that touches a module after the fix merges. Its report
|
||||
must show the TypeScript part running.
|
||||
Reference in New Issue
Block a user