| index df7ee79..98aa6fd 100644 |
| --- a/theme/theme_test.go |
| +++ b/theme/theme_test.go |
| @@ -1,6 +1,8 @@ |
| package theme |
| |
| import ( |
| + "bytes" |
| + "fmt" |
| "regexp" |
| "sort" |
| "strings" |
| @@ -10,92 +12,147 @@ import ( |
| ) |
| |
| // tokenDecl matches a custom-property declaration at the start of a |
| -// line, e.g. " --rst-accent: light-dark(...);" -- capturing the name, |
| -// not the value. It deliberately does not match var(--rst-accent) |
| -// *uses*, only declarations, so a theme that merely references another |
| -// token doesn't get counted as declaring it. |
| -var tokenDecl = regexp.MustCompile(`(?m)^\s*(--rst-[A-Za-z0-9-]+)\s*:`) |
| +// line, e.g. " --rst-accent: light-dark(...);" -- capturing the name |
| +// and the value separately. It deliberately does not match |
| +// var(--rst-accent) *uses*, only declarations, so a theme that merely |
| +// references another token doesn't get counted as declaring it. |
| +var tokenDecl = regexp.MustCompile(`(?m)^\s*(--rst-[A-Za-z0-9-]+)\s*:\s*(.+?);\s*$`) |
| |
| -// tokenSet returns the set of custom-property names declared in css. |
| -func tokenSet(css []byte) map[string]bool { |
| - set := map[string]bool{} |
| +// tokenValues returns the custom-property declarations in css as a |
| +// name->value map (the value is the raw declaration text, e.g. |
| +// "light-dark(#2464e0, #7ba7ff)", not further parsed). |
| +func tokenValues(css []byte) map[string]string { |
| + set := map[string]string{} |
| for _, m := range tokenDecl.FindAllSubmatch(css, -1) { |
| - set[string(m[1])] = true |
| + set[string(m[1])] = string(m[2]) |
| } |
| return set |
| } |
| |
| -// TestTokenSetMatchesDay is estilo's half of the two-pin drift check |
| -// AGENTS.md calls for under "Dependency": rastrillo pins nothing above |
| -// it, estilo pins rastrillo, and the apps will pin estilo. day.css' |
| -// own comment says a theme's whole contract is its token set -- |
| -// declare every name day declares -- so this test reads day's token |
| -// set straight from the rastrillo release estilo currently imports |
| -// (via ui.ThemeCSS("day"), not a copy) and fails if it and estilo.css |
| -// ever disagree. |
| +// stripHeaderComment removes the single leading "/* ... */" comment a |
| +// theme file opens with. estilo.css's and day.css's each explain their |
| +// own provenance there and legitimately differ -- estilo's names |
| +// AGENTS.md and this test, day's doesn't know estilo exists. Every |
| +// byte after that comment is the theme itself, and "estilo is day" |
| +// (AGENTS.md, "Theme") is a claim about exactly that span. |
| // |
| -// What this catches: rastrillo shipping a release where day adds a |
| -// token estilo hasn't picked up, drops one estilo still declares, or |
| -// renames one -- exactly the "adds or renames a token" case from |
| -// AGENTS.md, caught here on `go test` after a `go get -u` of |
| -// rastrillo, rather than surfacing as an undefined var(--rst-foo) or a |
| -// dead custom property in Docs, Sheets or a third consumer. |
| +// It only strips a comment that starts at byte 0; a file that doesn't |
| +// open with one is returned unchanged, so a missing or rewritten |
| +// header shows up as an ordinary byte diff below rather than silently |
| +// comparing the wrong span. |
| +func stripHeaderComment(css []byte) []byte { |
| + if !bytes.HasPrefix(css, []byte("/*")) { |
| + return css |
| + } |
| + end := bytes.Index(css, []byte("*/")) |
| + if end == -1 { |
| + return css |
| + } |
| + return css[end+len("*/"):] |
| +} |
| + |
| +// TestMatchesDayByteForByte is estilo's enforcement of the ruling |
| +// recorded in AGENTS.md under "Theme": estilo's theme *is* day, not |
| +// derived from it or based on it -- identical, in every declaration |
| +// and every value. Paul, 2026-08-31, verbatim: "Let's have estilo just |
| +// be day for now, but we can add oficina specific changes and |
| +// components to it as we need to." Until a named need is recorded |
| +// there, there is nothing to diverge on, so this test asserts |
| +// byte-for-byte identity between theme/estilo.css and the pinned |
| +// rastrillo module's ui/themes/day.css, past each file's own header |
| +// comment (see stripHeaderComment) -- not just a matching token set, |
| +// which would miss day changing a token's value while keeping its |
| +// name. |
| +// |
| +// A rastrillo upgrade that changes day -- adds, drops or renames a |
| +// token, or simply reweights one of its values -- fails this test on |
| +// `go test` (after a `go get -u` of rastrillo), rather than surfacing |
| +// later as an undefined var(--rst-foo), a dead custom property, or a |
| +// silently stale colour in Docs, Sheets or a third consumer. |
| +// |
| +// A deliberate divergence -- estilo choosing on purpose to differ from |
| +// day -- ALSO fails this test, and that is the point: the rule is not |
| +// "this never fires," it is "a divergence needs a named need." Making |
| +// this pass again means updating theme/estilo.css to the new value AND |
| +// recording the need in AGENTS.md's decisions table in the same |
| +// change. Do not loosen this back to a token-set-only comparison to |
| +// make it pass without doing both. |
| // |
| // What this does NOT catch: |
| -// - day changing a token's *value* while keeping its name. The two |
| -// sets still match; estilo.css silently keeps the old colour until |
| -// someone re-derives it from the new release. (Values are a design |
| -// decision, not something this test could resolve on its own even |
| -// if it noticed.) |
| -// - Drift in tokens.css itself -- the structural tokens (spacing, |
| -// type scale, component rules) are not part of a theme file and |
| -// are out of scope here. |
| -// - rastrillo's other shipped themes (plain, signal): estilo derives |
| -// from day only, per Paul's ruling, so this test never looks at |
| -// them. |
| -// - A consumer app pinning an *older* estilo than the one that ran |
| +// - A rastrillo upgrade landing without anyone running `go test` |
| +// afterward. This fires on the next test run, not the moment |
| +// upstream changes; nothing here watches rastrillo continuously. |
| +// - Drift in tokens.css's structural tokens (spacing, type scale, |
| +// component rules) -- those aren't part of a theme file and are |
| +// out of scope for this test. |
| +// - rastrillo's other shipped themes (plain, signal): estilo is day, |
| +// per Paul's ruling, so this test never looks at them. |
| +// - A consumer app pinning an older estilo than the one that ran |
| // this test. This test runs at estilo's own module boundary; it |
| // says nothing about which estilo version an app has pinned. |
| -func TestTokenSetMatchesDay(t *testing.T) { |
| +func TestMatchesDayByteForByte(t *testing.T) { |
| dayCSS, ok := ui.ThemeCSS("day") |
| if !ok { |
| t.Fatal(`ui.ThemeCSS("day") returned false -- rastrillo no longer ships a theme named "day"`) |
| } |
| - day := tokenSet(dayCSS) |
| - if len(day) == 0 { |
| - t.Fatal("parsed zero --rst-* tokens out of rastrillo's day.css; the parser or the upstream file shape has changed") |
| - } |
| |
| - estilo := tokenSet(css) |
| - if len(estilo) == 0 { |
| - t.Fatal("parsed zero --rst-* tokens out of estilo.css; the parser or this file's shape has changed") |
| + day := stripHeaderComment(dayCSS) |
| + estilo := stripHeaderComment(css) |
| + |
| + if bytes.Equal(day, estilo) { |
| + return |
| } |
| |
| - var missing, extra []string |
| - for name := range day { |
| - if !estilo[name] { |
| + // Diagnose at the token level first: that is almost always what |
| + // changed, and the failure should name it directly rather than |
| + // dumping a raw byte diff on whoever reads the test output. |
| + dayTokens := tokenValues(day) |
| + estiloTokens := tokenValues(estilo) |
| + |
| + var changed, missing, extra []string |
| + for name, dv := range dayTokens { |
| + ev, ok := estiloTokens[name] |
| + if !ok { |
| missing = append(missing, name) |
| + continue |
| + } |
| + if ev != dv { |
| + changed = append(changed, name) |
| } |
| } |
| - for name := range estilo { |
| - if !day[name] { |
| + for name := range estiloTokens { |
| + if _, ok := dayTokens[name]; !ok { |
| extra = append(extra, name) |
| } |
| } |
| + sort.Strings(changed) |
| sort.Strings(missing) |
| sort.Strings(extra) |
| |
| - if len(missing) > 0 || len(extra) > 0 { |
| - var b strings.Builder |
| - b.WriteString("estilo.css's token set has drifted from rastrillo day's:\n") |
| - if len(missing) > 0 { |
| - b.WriteString(" day declares, estilo is missing: " + strings.Join(missing, ", ") + "\n") |
| - } |
| - if len(extra) > 0 { |
| - b.WriteString(" estilo declares, day no longer has: " + strings.Join(extra, ", ") + "\n") |
| - } |
| - b.WriteString("A theme's only contract is its token set (AGENTS.md, \"Vocabulary\") -- " + |
| - "update theme/estilo.css to match, recomputing any contrast rows the change touches.") |
| - t.Fatal(b.String()) |
| + var b strings.Builder |
| + b.WriteString("theme/estilo.css has drifted from rastrillo's day.css (v0.22.0, as pinned in go.mod), past the header comment:\n") |
| + for _, name := range changed { |
| + fmt.Fprintf(&b, " %s: day has %q, estilo has %q\n", name, dayTokens[name], estiloTokens[name]) |
| + } |
| + for _, name := range missing { |
| + b.WriteString(" day declares, estilo is missing: " + name + "\n") |
| + } |
| + for _, name := range extra { |
| + b.WriteString(" estilo declares, day no longer has: " + name + "\n") |
| + } |
| + if len(changed) == 0 && len(missing) == 0 && len(extra) == 0 { |
| + // The bytes differ but no single --rst-* declaration line |
| + // does -- the divergence is in something the token regex |
| + // doesn't parse (color-scheme, the two [data-theme] toggle |
| + // rules, or plain formatting). Say so rather than reporting |
| + // three empty lists and leaving the reader to guess. |
| + b.WriteString(" no single --rst-* declaration differs; the divergence is elsewhere in the file " + |
| + "(the toggle rules, color-scheme, or formatting) -- diff theme/estilo.css against rastrillo's " + |
| + "ui/themes/day.css by hand.\n") |
| } |
| + b.WriteString("If this is an unintended rastrillo upgrade, update theme/estilo.css to match day. " + |
| + "If this is a deliberate divergence, that is allowed but not free: update theme/estilo.css to the " + |
| + "new value AND record the need in AGENTS.md's decisions table in the same change -- \"it would look " + |
| + "better\" is not a need, \"<app> requires <token> which day does not declare\" is.") |
| + t.Fatal(b.String()) |
| } |
amadan