docs: add a root CLAUDE.md with the build, test and lint setup - #642
docs: add a root CLAUDE.md with the build, test and lint setup#642dobby-coder[bot] wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
VERDICT: approve
Rules Dobby 2 — consolidated review (rule sweep + handed-forward review findings).
One blocking finding, fixed on the branch
The postgres-init claim was factually wrong. It said the container "creates the irma schema; without it the keyshare and myirma database tests fail". Neither half of the consequence holds:
- The tests create the schema themselves.
keyshareserver/postgresdb_test.go:171,myirmaserver/postgresdb_test.go:239andtasks/tasks_test.go:308each defineSetupDatabase(t)asRunScriptOnDB(t, "../cleanup.sql", true)thenRunScriptOnDB(t, "../schema.sql", false), and every Postgres test calls it first (keyshareserver/postgresdb_test.go:20,61,141 and the equivalents in the other two packages).schema.sqlopens withCREATE SCHEMA irma;. - CI never runs
postgres-init. Thetestservice'sdepends_on(docker-compose.yml:89-108) lists postgres, mysql, sqlserver, mailhog, eudi_openid4vp, eudi_openid4vp_jwt, veramo_openid4vp, tls_proxy and statuslist_agent, and notpostgres-init.postgres-initappears only under theyiviservice (docker-compose.yml:127-130).docker compose run teststarts a service's own dependencies, so the exact CI command never brings it up, and the suite is green.
This is the same class of thing the PR already got right for Redis: a convenience for the run-a-server-by-hand path, not a test prerequisite. Since it is a two-sentence reword on a self-authored draft with no human review to dismiss, I pushed the correction rather than looping: fb7ee8f.
Rule sweep
Per-rule check over the rules that apply to a docs-only Go-repo diff. All clean:
writing-rules— no banned vocabulary, openers or phrases; em-dash and bold counts both under the 1-per-1,000-words cap.irmago-lint-job-gates-on-misspell—misspell -error -i "adres,instelling,relatie"run against the file, clean. The CIlintjob is green.staticcheck-needs-a-toolchain-as-new-as-go-mod— the file documents theGOTOOLCHAINpin correctly, which is the point of the rule.patch-in-comment-gate-is-not-live-yet— no CI job or gate is described as live that is not. The PR adds no workflow files.no-justification-paragraphs-for-simple-changes,cross-repo-link-format,conventional-commit-pr-titles(nopr-title.ymlin this repo, so it does not apply),pr-close-issue-keywords(Closes #641, own line, correct form),pr-review-assignments(@kamphuisem requested) — all clean.irmamobile-claude-md-gitignoreddoes not extend here. That rule is scoped to privacybydesign/irmamobile, which gitignores its CLAUDE.md; irmago does not.
Other claims, verified against the tree
Checked the rest of the file rather than trusting it, since a wrong line in CLAUDE.md is auto-loaded context for every later run. All accurate: the lint step order and exact commands against .github/workflows/status-checks.yml; staticcheck.conf keeping SA1019 on while CI adds -SA1019; changelog.yml's base-to-head diff plus skip-changelog escape; the testdata/configurations/ file list; ports 5432/3306/1433/1025 and the postgres.localhost / mailhog.localhost aliases; the 8443/8444/8445 tls_proxy mapping; go.mod's go 1.26.0 / toolchain go1.26.4; the root being package irmago with only Version; CGO_ENABLED=0 in .github/actions/build/action.yml:35 and the Dockerfile; go list -deps ./client pulling in both sqlcipher packages while ./yivi pulls in neither; internal/sessiontest/irma_issuance_test.go:67 calling startRevocationServer, defined at revocation_test.go:909 behind //go:build !local_tests; and no Redis service in docker-compose.yml with miniredis at exactly the two named sites.
One thing worth a maintainer's eye, not blocking: the PR body flags that the README's go test -p 1 --tags=local_tests ./... does not build. That is a real pre-existing breakage this PR only documents, and worth its own issue.
| `internal/sessiontest` and the `!local_tests` files need the services in | ||
| `docker-compose.yml`. The tests reach them over `localhost`: | ||
|
|
||
| - `postgres` on 5432. The tests create the `irma` schema themselves: the |
There was a problem hiding this comment.
Corrected in fb7ee8f. The previous wording said this step "creates the irma schema; without it the keyshare and myirma database tests fail" — the tests create the schema themselves via each package's SetupDatabase(t) (cleanup.sql then schema.sql), and postgres-init is in the yivi service's depends_on, not the test service's, so docker compose run test never starts it. Left here as the record of what changed and why.
Records the repo-specific knowledge that is currently spread over README.md, docker-compose.yml, Dockerfile.test, staticcheck.conf and the CI workflows: the package layout, the SQLCipher/cgo prerequisite and which three packages need it, how to run the tests with and without the compose services, the misspell and staticcheck gates, and the CHANGELOG.md requirement. Every command in the file was run in a checkout of this repo, except the docker compose ones; the file says so where that applies. Refs #641
The tests create and drop the `irma` schema themselves via each package's `SetupDatabase(t)` (cleanup.sql then schema.sql), and `postgres-init` is in the `yivi` service's depends_on, not the `test` service's, so `docker compose run test` never starts it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fb7ee8f to
1ade735
Compare
Closes #641.
Adds a root
CLAUDE.mdcovering the layout, build, tests, lint gates and the changelog requirement. Documentation only; no code or CI changes.Every command in the file was run in a checkout of this repo, with one exception that the file marks explicitly: this container has the Docker client but no Docker daemon, so
docker compose run test -v ./...anddocker compose upcould not be executed. What the file says about those services is read offdocker-compose.yml,Dockerfile.testand.github/workflows/status-checks.yml.Things found while verifying, worth a look
Three of these contradict what the issue or the README says, so flagging them rather than burying them in the file.
--tags=local_tests ./...from the README does not build.internal/sessiontest/irma_issuance_test.go(untagged) callsstartRevocationServer, which lives inrevocation_test.gobehind//go:build !local_tests:Everything else passes with no services running.
CLAUDE.mddocuments the workaround (go test -p 1 --tags=local_tests $(go list ./... | grep -v /internal/sessiontest)) and names the cause, but the fix is behavioral and out of scope for a docs PR. Happy to send it separately if you want the README command to work again.Redis is not a test dependency. The issue asked for the "Postgres/Redis services from
docker-compose.yml", but there is no Redis service indocker-compose.yml, and both Redis test sites (internal/sessiontest/redis_test.go,irma/server/keyshare/myirmaserver/session_test.go) use in-processminiredis. Thedocker run redissnippet in the README is for running a server by hand. The file says this so a later reader does not go hunting for a container.staticcheck needs a toolchain at least as new as
go.mod.go install honnef.co/go/tools/cmd/staticcheck@2025.1.1compiles staticcheck with whatevergois on PATH. If that is older than 1.26, staticcheck reports nothing useful:CI is unaffected (
actions/setup-gousesgo-version-file: go.mod), but it bites locally.GOTOOLCHAIN=go1.26.4 go install ...fixes it.Verified locally
The SQLCipher claim was checked by rebuilding with an empty
PKG_CONFIG_LIBDIRon a coldGOCACHE:go build ./clientfails oneudi/storage/db/sqlcipher,go build ./yivisucceeds.Notes
CLAUDE.mdas a side change. Whichever merges second will conflict on the file; the content here is a superset.skip-changelog: a contributor-facing notes file does not belong in the user-facing changelog. Remove the label and I will add an entry if you would rather have one.