test(config): pin resolved config with golden tests#73
Merged
Conversation
Add a golden-test net over GetAssignmentConfig so the upcoming rewrite of
config loading (viper -> typed source schema) can be proven behaviour-
preserving rather than merely compiling.
Fixtures in config/testdata/courses/:
- mpd, vss, algdati, fundc, fun: structural copies of the real course
files, student identities pseudonymized (no real addresses in the repo)
- casecourse: synthetic. The real files are lowercase throughout and
therefore cannot detect a change in case handling. This one pins the
key/value asymmetry that a naive yaml.v3 decoder would break: viper
lowercases map keys (Grp01 -> grp01) but leaves list values alone, and
gitlabProjectPath keeps already-valid paths verbatim (Upper.Case) while
parameterizing invalid ones (plus+tag -> plus-tag).
- legacy: synthetic. Pins the legacy-key shims and their activation rules,
most of which have no coverage in the real files.
The view records the *derived* repo names, not just the raw config: a change
in case handling would not show up in Email/Raw, but it silently renames the
GitLab project. Verified by injecting a regression: the goldens catch it.
viper is reloaded per assignment because GetAssignmentConfig writes the
merged `extends` result back into global state, so results would otherwise
depend on resolution order.
Behaviour is frozen as-is, bugs included. Two are documented in the fixtures
and must be fixed separately, after the rewrite, so migration diffs stay
readable:
- release.dockerImages is read from the wrong key, silently ignoring the
images configured in vss/blatt2
- the `approvalsRequired` alias is dead code (viper lowercases it before
the alias table sees it), silently yielding RequiredApprovals 0 instead
of the configured value
Refresh with: go test ./config/ -run TestGolden -update
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Coverage |
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
1 participant
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.
Schritt 0 des glabs-web-Plans: das Sicherheitsnetz, bevor irgendetwas am Config-Laden angefasst wird.
Warum
config/bautAssignmentConfigheute aus ~63 einzelnenviper.GetString("kurs.assignment.feld")-Aufrufen zusammen — ohne yaml-Tags, mit Vererbung, die beim Lesen in globalen State zurückschreibt. Der geplante Umbau auf ein typisiertes Quellschema muss verhaltenserhaltend sein, und das lässt sich nur beweisen, wenn das IST-Verhalten vorher festgenagelt ist.Dieser PR ändert keinen Produktivcode — nur Tests und Fixtures.
Was drin ist
Fixtures (
config/testdata/courses/):mpd,vss,algdati,fundc,fun— strukturgetreue Kopien der echten Kursdateien, Studi-Identitäten pseudonymisiert. Keine echten Adressen im öffentlichen Repo; verifiziert per Diff, dass sonst nichts an der Struktur abweicht.casecourse— synthetisch, und der eigentliche Grund für diesen PR. Die echten YAMLs sind durchgängig kleingeschrieben und können eine Änderung am Case-Handling nicht entdecken. Dieses Fixture nagelt die Asymmetrie fest, an der ein naiver yaml.v3-Decoder zerbräche:Grp01→grp01), lässt Listen-Werte aber in RuhegitlabProjectPathbehält bereits gültige Pfade verbatim (Upper.Case), parameterisiert ungültige (plus+tag→plus-tag)legacy— synthetisch. Nagelt die Legacy-Key-Shims und ihre Aktivierungsregeln fest (die startercode-Branch-Shims greifen nur, wennbranches:fehlt; die Issue-Shims nur, wennissues:ganz fehlt). Die meisten davon haben in den echten Dateien null Abdeckung.Der Golden-View zeichnet die abgeleiteten Repo-Namen auf, nicht nur die Rohconfig. Das ist der Punkt: eine Änderung am Case-Handling taucht in
Email/Rawgar nicht auf — sie benennt still das GitLab-Projekt um. Gegenprobe gemacht: eine künstlich eingebaute Regression wird gefangen und zeigt exakt den geändertenrepoName.viper wird pro Assignment neu geladen, weil
GetAssignmentConfigdas gemergteextends-Ergebnis in den globalen State zurückschreibt — sonst hinge das Ergebnis von der Auflösungsreihenfolge ab.Zwei Bugs gefunden — bewusst eingefroren, nicht gefixt
Die Goldens frieren das Verhalten ein, inklusive Bugs. Sonst weiß man beim späteren Umbau bei jedem Diff nicht, ob er gewollt ist. Beide sind in den Fixtures dokumentiert und gehören in eigene Commits nach dem Schema-Umbau:
release.dockerImageswird vom falschen Key gelesen.vss.yamllegt die Images unterrelease.mergeRequest.dockerImages,config/release.go:47liestrelease.dockerImages→ die sechs Images invss/blatt2werden stillschweigend ignoriert.approvalsRequired-Alias ist toter Code. viper schreibt den Key aufapprovalsrequiredklein, bevor die Alias-Tabelle inconfig/assignment.go:443ihn gegen die CamelCase-Schreibweise vergleicht — der Fall trifft nie zu. Ergebnis:RequiredApprovals: 0statt des konfigurierten Werts, also keine Approval-Pflicht. Der Goldenlegacy.approvalslist.jsonstellt die drei Schreibweisen nebeneinander:required_approvals→2 ✓,approvalsRequired→0 ✗,requiredApprovals→3 ✓.Beide sind latent, nicht aktiv: keine echte Config nutzt den kaputten Alias, und
vss/blatt2setztcontainerRegistryohnehin explizit. Genau die Klasse Fund, die das geplanteglabs config lintkünftig automatisch meldet.Verifikation
go test ./...grün;gofmt,go vet,golangci-lintsauber-updateerzeugt keine Diffs@hm.eduoder lokale Pfade im Diff, 3589 PseudonymeRefresh:
go test ./config/ -run TestGolden -update🤖 Generated with Claude Code