From 1357ffda5bdfc52dddd11e07145f522879394708 Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Mon, 17 Aug 2026 01:27:17 +0200 Subject: [PATCH] Refuse a client budget limit typed into anything the build reads [#89] The design system page states five numbers a native client is held to, and they are the pinned token copy's. Nothing stopped a second copy of one of them from being typed into a template or into the prose beside it. That is the failure the row about a typed colour exists for with a different unit, and it is the harder half to see: a wrong colour looks wrong on the page and a wrong millisecond looks like every other millisecond, so the copy goes on being rendered perfectly after the published value moves. A row cannot be told anything except the bytes of one file, so the table is now handed the numbers it compares against. Every other row ignores what it is given, and the table holds the same rows however it is handed, which is what lets the count the gate prints come from a call that supplies nothing. A run reads the copy before it builds the table and refuses a tree whose copy carries no such number, because a row that compared nothing would report ok and read as a tree holding no second copy. How a limit is spelled is now decided in one place, in the token package, and both the page and the row ask there. A page and a row each carrying their own spelling would part company the day either moved, and the row would go on reporting ok over the words the page had started using. Removing the row's decision reds three cases in the suite, and the two spellings, the numbers it leaves alone and its following the copy rather than carrying the numbers each have a case of their own. Signed-off-by: Nils Lehnen Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- CHANGELOG.md | 7 + internal/gate/gate.go | 4 +- internal/invariant/invariant.go | 101 ++++++++++- internal/invariant/invariant_test.go | 258 ++++++++++++++++++++++++++- internal/site/designsystem.go | 44 ++--- internal/site/designsystem_test.go | 40 +++++ internal/tokens/tokens.go | 90 ++++++++++ 7 files changed, 497 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6d657e..69b14a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,3 +79,10 @@ there is no earlier bundle to compare it against. writing about itself is [decisions/0014-what-the-design-system-page-renders.md](decisions/0014-what-the-design-system-page-renders.md), and the page prints the counts of both so the split can be read off the page. +- The gate refuses a limit a native client is held to, written into anything the + build reads to render a page. The five numbers are the pinned token copy's, + and the row is handed them rather than carrying them, so it follows the copy + the day one moves. It reads both spellings the same value arrives in, the + words the page states it in and the number with its unit alone, and a limit + written in any other unit walks through, which is the same bound the row about + a typed colour declares for itself. diff --git a/internal/gate/gate.go b/internal/gate/gate.go index 6db1266..771b4ff 100644 --- a/internal/gate/gate.go +++ b/internal/gate/gate.go @@ -241,7 +241,9 @@ func invariantsLeg(root string) (string, error) { lines := strings.Split(strings.TrimRight(log.String(), "\n"), "\n") return "", fmt.Errorf("%v:\n%s", err, indent(lines)) } - rules, owed := len(invariant.Rules()), len(invariant.Owing()) + // The count of rows rather than their decisions, which the run above has + // already taken, so the table is handed nothing. + rules, owed := len(invariant.Rules(nil)), len(invariant.Owing()) return fmt.Sprintf("ok, %d rule(s) decided, %d owed and not decided", rules, owed), nil } diff --git a/internal/invariant/invariant.go b/internal/invariant/invariant.go index ac96895..6ac2c87 100644 --- a/internal/invariant/invariant.go +++ b/internal/invariant/invariant.go @@ -257,7 +257,18 @@ const affiliationNotice = "Flowfin is not affiliated with the Jellyfin project." const contentElement = "main" // Rules is the table. The order is the order a run reports them in. -func Rules() []Rule { +// +// It is handed the numbers a client is held to, because one row compares against +// values rather than against a shape, and there is no other way for a value to +// reach a row: a row is given the bytes of one file and nothing else. Carrying +// the numbers in this file instead would make it the second declaration of the +// file the row exists to keep as the only one, which is the failure with the +// name of the rule on it. +// +// What the table holds does not depend on what it is handed. Every row is here +// on every call, and only what one of them compares against moves, so a caller +// that wants the count rather than the decisions may hand it nothing. +func Rules(numbers []tokens.Number) []Rule { return []Rule{ { ID: "page-declares-its-language", @@ -469,6 +480,13 @@ func Rules() []Rule { Refuses: "a colour, a length, a font family or a font weight written into what the build reads, outside a fragment reference", decide: decideTypedTokenValue, }, + { + ID: "client-budget-numbers-live-in-exactly-one-file", + Subject: BuildInputs, + Reason: "a number a client is held to is the same class of fact as a spacing step, and it is the harder one to see when it goes stale: a wrong colour looks wrong on the page and a wrong millisecond looks like every other millisecond, so a second copy of one is a conformance target somebody meets while the published one says something else", + Refuses: "a limit a client is held to, written into what the build reads, in either the words the page states it in or the number and its unit alone", + decide: decideTypedBudgetNumber(numbers), + }, { ID: "version-lives-in-exactly-one-file", Subject: TrackedTextOutsideTheVersionRegister, @@ -860,6 +878,49 @@ func decideTypedTokenValue(body []byte) []string { return details } +// decideTypedBudgetNumber refuses one of the numbers a client is held to, +// written into something the build reads. +// +// It is the same rule as the one above with a different unit, and the unit is +// what makes it a separate row rather than a fifth shape in that one. A colour, +// a length, a font stack and a weight are recognisable from their own spelling, +// so that row can be told what to look for once and never revisited. A latency +// ceiling is digits and a word, indistinguishable from a transition duration or +// from a figure in a sentence, so the only thing that separates a copy of the +// budget from an unrelated number is the budget itself. That is why this one is +// handed the values and the others are not. +// +// Two spellings per number, and the longer one first so that a line stating the +// limit in the words the page uses is reported as that rather than as the bare +// number inside it. What it compares is exact: a limit written in another unit +// walks through, which is the same bound the row above declares for a colour +// spelled in a form CSS does not read. The file stays the authority for the set, +// and this row is what stops a second copy of one member of it. +// +// A run handed no numbers refuses nothing, which is why Run reads the copy +// before it builds the table and refuses a tree whose copy carries none. A green +// mark from this row over an empty set would say the tree holds no second copy +// of a number, when what it means is that nobody told it what the numbers are. +func decideTypedBudgetNumber(numbers []tokens.Number) func([]byte) []string { + return func(body []byte) []string { + var details []string + for i, line := range strings.Split(string(body), "\n") { + for _, n := range numbers { + for _, spelling := range []string{n.Stated(), n.Bare()} { + if !strings.Contains(line, spelling) { + continue + } + details = append(details, fmt.Sprintf( + "line %d writes %q, which is what %s says %s is, and %s is the one file it is read from", + i+1, spelling, tokens.File, n.Name, tokens.File)) + break + } + } + } + return details + } +} + // decideImports refuses an import that is neither standard library nor inside // this module. A standard library path has no dot in its first element, which is // the rule the toolchain itself uses to tell the two apart. @@ -893,12 +954,42 @@ func Owing() []Owed { } } +// budgetNumbers reads the numbers a client is held to out of the pinned copy, so +// that the row about a second copy of one has something to compare against. +// +// It fails closed in both directions the row cannot survive, and each is refused +// in its own words rather than collapsed into one. A copy that could not be read +// is a tree the row was never decided against. A copy that carries no such +// number is a row that would report ok having compared nothing, which reads as a +// tree holding no second copy and means that nobody said what the numbers are. +// That is the same position gather takes about the copy existing at all, one +// step further in. +func budgetNumbers(root string) ([]tokens.Number, error) { + values, err := tokens.Load(root) + if err != nil { + return nil, fmt.Errorf("the row about where a client budget number is read from cannot be decided: %w", err) + } + numbers, reasons := tokens.Numbers(values) + if len(reasons) > 0 { + return nil, fmt.Errorf("%s, %d reason(s):\n %s", tokens.File, len(reasons), strings.Join(reasons, "\n ")) + } + if len(numbers) == 0 { + return nil, fmt.Errorf("%s carries no client budget number, and the row about where one is read from is a row about there being exactly one file that carries it", tokens.File) + } + return numbers, nil +} + // Run decides every rule against the tree at root and writes what it examined // to log. It builds the site into a directory it throws away, so what the page // rules read is what a build produces rather than whatever is sitting in the // output directory from an earlier one. func Run(root string, log io.Writer) error { - rules := Rules() + numbers, err := budgetNumbers(root) + if err != nil { + return err + } + + rules := Rules(numbers) ids := make([]string, len(rules)) for i, r := range rules { ids[i] = r.ID @@ -2010,8 +2101,12 @@ func decideCitedChecks(body []byte) []string { // decided answers whether a name is a row this gate decides. It reads the table // rather than a list written beside it, so a row added, renamed or removed // changes this answer without anybody remembering to. +// +// The table is asked for its names rather than for its decisions, so it is +// handed nothing. What a row compares against does not change whether it is in +// the table, which is the sentence Rules carries and the suite checks. func decided(name string) bool { - for _, r := range Rules() { + for _, r := range Rules(nil) { if r.ID == name { return true } diff --git a/internal/invariant/invariant_test.go b/internal/invariant/invariant_test.go index 72ee5a1..093acd6 100644 --- a/internal/invariant/invariant_test.go +++ b/internal/invariant/invariant_test.go @@ -14,6 +14,7 @@ package invariant import ( "bytes" "encoding/base64" + "fmt" "io" "os" "os/exec" @@ -40,6 +41,18 @@ func b64(t *testing.T, s string) []byte { return b } +// What the table is handed where a case is about the rows rather than about a +// tree. Two numbers rather than the five the real copy carries, and neither of +// them one of those five: a case asserting against the published limits would +// pass by reading the same file the row reads, which is the one thing this row +// cannot be allowed to prove about itself. One is a ceiling and one is an exact +// value, because the two are spelled differently and a fixture carrying only a +// ceiling would leave the other spelling untried. +var fixtureNumbers = []tokens.Number{ + {Name: "first-frame", Limit: "37", Unit: "ms", Comparison: "below"}, + {Name: "torn-frames", Limit: "4", Unit: "frames", Comparison: "equal"}, +} + // The opening tag of the content, written once because most fixtures below put // their markup next to it and a second spelling of it here would drift against // the page silently: the replacement would find nothing, the fixture would be @@ -237,6 +250,14 @@ func TestEveryRowRefusesItsOwnViolationAndPassesTheNeighbour(t *testing.T) { // published one moves. "design-tokens-live-in-exactly-one-file": []byte(strings.Replace(cleanPage, ``, ``, 1)), + // The limit quoted in a sentence, which is how a number a client + // is held to gets into prose: somebody explains what the software + // promises and writes the figure down beside it. It is the same + // second definition as the colour above and it looks like an + // ordinary sentence afterwards, which is why nobody finds it. + "client-budget-numbers-live-in-exactly-one-file": []byte(strings.Replace(cleanPage, + `

A title

`, + `

A title

A key press answers `+fixtureNumbers[0].Stated()+`.

`, 1)), // The sentence a document gains the day somebody wants a reader to // know which release they are looking at. It is assembled from the // constant rather than written out, because a test source carrying @@ -261,7 +282,7 @@ func TestEveryRowRefusesItsOwnViolationAndPassesTheNeighbour(t *testing.T) { // wrong reason. cleanTest := b64(t, "cGFja2FnZSBzYW1wbGUKCmltcG9ydCAoCgkib3MiCgkidGVzdGluZyIKKQoKZnVuYyBUZXN0U29tZXRoaW5nKHQgKnRlc3RpbmcuVCkgewoJaWYgb3MuR2V0ZW52KCJIT01FIikgPT0gIiIgewoJCXQuU2tpcCgibm8gaG9tZSIpCgl9Cn0K") - rules := Rules() + rules := Rules(fixtureNumbers) if len(rules) != len(violations) { t.Fatalf("the table holds %d row(s) and this test carries %d violation(s); a row without one proves nothing", len(rules), len(violations)) @@ -566,7 +587,7 @@ func TestTheContentLinkRowLeavesWhatIsOutOfTheOrderAlone(t *testing.T) { // meaning anything. The name is taken out of the table rather than typed here, // so this test cannot go on passing against a row that was renamed. func TestTheCitationRowLeavesARealCheckAlone(t *testing.T) { - real := Rules()[0].ID + real := Rules(fixtureNumbers)[0].ID page := []byte(strings.Replace(cleanPage, contentOpen, contentOpen+``, 1)) if got := decideCitedChecks(page); len(got) != 0 { @@ -612,7 +633,7 @@ func TestAViolationRedsExactlyOneRow(t *testing.T) { noLang := []byte(strings.Replace(cleanPage, ``, ``, 1)) var refused []string - for _, r := range Rules() { + for _, r := range Rules(fixtureNumbers) { if len(r.decide(noLang)) > 0 { refused = append(refused, r.ID) } @@ -722,13 +743,19 @@ func tree(t *testing.T, template string) string { // whole suite the day a row is renamed. wr(filepath.FromSlash(site.PrivacyFile), "A second title\n\ndescription: What the second fixture page is.\n\nOne paragraph.\n\n"+ - "checked: One statement. ["+Rules()[0].ID+"]\n\n"+ + "checked: One statement. ["+Rules(fixtureNumbers)[0].ID+"]\n\n"+ "residual: What a host sees is true whatever this site does.\n") // The copy the build reads. It carries a colour, because the row about // where a colour is read from is about there being one file that may // carry one, and a fixture whose copy held none would prove nothing - // about which file that is. - wr(filepath.FromSlash(tokens.File), `{"surface":{"ground":{"dark":{"srgb":"#121216","alpha":1}}}}`) + // about which file that is. It carries the client budget for the same + // reason one row further on, and the numbers are the ones the cases + // above are written against rather than a second set: what the run + // refuses and what a row refuses have to be the same values, or a case + // that passed here would say nothing about the run. + wr(filepath.FromSlash(tokens.File), + `{"surface":{"ground":{"dark":{"srgb":"#121216","alpha":1}}},"budget":{"numbers":{`+ + strings.Join(asJSON(fixtureNumbers), ",")+`}}}`) // The source of the produced reporting route. The day is far enough // ahead that the fixture does not expire while nobody is looking at it, // which is the one thing in this tree that would go red on a date rather @@ -746,6 +773,18 @@ func tree(t *testing.T, template string) string { return root } +// asJSON writes the fixture numbers the way the token file carries them, so a +// tree and the cases about a single row are held to one set of values rather +// than to two that agree today. +func asJSON(numbers []tokens.Number) []string { + var out []string + for _, n := range numbers { + out = append(out, fmt.Sprintf(`%q:{"limit":%s,"unit":%q,"comparison":%q}`, + n.Name, n.Limit, n.Unit, n.Comparison)) + } + return out +} + func git(t *testing.T, dir string, args ...string) { t.Helper() cmd := exec.Command("git", args...) @@ -869,6 +908,91 @@ func TestRunRefusesAFrameThatDroppedTheAffiliationNoticeOnEveryPageItProduced(t } } +// The whole run over a tree whose frame quotes a limit the copy is the +// authority for. It is the shape somebody writes when they are explaining what +// the software promises with the number in front of them, and the run names the +// input it was typed into rather than the pages it came out on, because the +// input is where the repair is. +func TestRunRefusesALimitTypedIntoWhatTheBuildReads(t *testing.T) { + root := tree(t, strings.Replace(goodTemplate, `

{{ .Title }}

`, + "

{{ .Title }}

\n

A key press answers "+fixtureNumbers[0].Stated()+".

", 1)) + + var log bytes.Buffer + if err := Run(root, &log); err == nil { + t.Fatalf("Run accepted a build input quoting a client budget limit:\n%s", log.String()) + } + for _, want := range []string{ + "client-budget-numbers-live-in-exactly-one-file: REFUSED, 1 violation(s)", + "templates/page.html.tmpl: line 13", + `"under 37 ms"`, + tokens.File, + } { + if !strings.Contains(log.String(), want) { + t.Errorf("the run does not say %q; it said:\n%s", want, log.String()) + } + } +} + +// The same tree with the sentence taken back out. Without this the case above +// would pass over a run that refuses every tree, which proves the opposite of +// what it is for. +func TestRunAcceptsAFrameThatQuotesNoLimit(t *testing.T) { + var log bytes.Buffer + if err := Run(tree(t, goodTemplate), &log); err != nil { + t.Fatalf("Run refused a tree quoting no limit: %v\n%s", err, log.String()) + } + if !strings.Contains(log.String(), "client-budget-numbers-live-in-exactly-one-file: ok") { + t.Errorf("the run did not report the row as examined; it said:\n%s", log.String()) + } +} + +// A copy carrying no client budget number is refused rather than decided +// against. The row would report ok having compared nothing, which reads as a +// tree holding no second copy of a limit and means that nobody said what the +// limits are. +func TestRunRefusesACopyThatCarriesNoClientBudget(t *testing.T) { + root := tree(t, goodTemplate) + if err := os.WriteFile(filepath.Join(root, filepath.FromSlash(tokens.File)), + []byte(`{"surface":{"ground":{"dark":{"srgb":"#121216","alpha":1}}}}`), 0o644); err != nil { + t.Fatalf("rewriting the copy: %v", err) + } + git(t, root, "add", "-A") + + var log bytes.Buffer + err := Run(root, &log) + if err == nil { + t.Fatalf("Run decided the table against a copy carrying no client budget:\n%s", log.String()) + } + if !strings.Contains(err.Error(), "carries no client budget number") { + t.Errorf("the refusal reads %q, which does not say what was missing", err) + } + if strings.Contains(log.String(), "client-budget-numbers-live-in-exactly-one-file: ok") { + t.Errorf("the run reported the row as examined against a copy with nothing in it:\n%s", log.String()) + } +} + +// A number the copy carries with no unit beside it is refused before any row is +// decided, and the refusal names the number rather than the file alone. A limit +// with no unit states nothing, so a row comparing against it would be comparing +// against a bare figure and would refuse every line that happened to carry it. +func TestRunRefusesACopyWhoseLimitCarriesNoUnit(t *testing.T) { + root := tree(t, goodTemplate) + if err := os.WriteFile(filepath.Join(root, filepath.FromSlash(tokens.File)), + []byte(`{"budget":{"numbers":{"first-frame":{"limit":37,"comparison":"below"}}}}`), 0o644); err != nil { + t.Fatalf("rewriting the copy: %v", err) + } + git(t, root, "add", "-A") + + var log bytes.Buffer + err := Run(root, &log) + if err == nil { + t.Fatalf("Run decided the table against a limit that states nothing:\n%s", log.String()) + } + if !strings.Contains(err.Error(), "budget.numbers.first-frame") { + t.Errorf("the refusal reads %q, which does not name the number", err) + } +} + // The link to the content is the first thing in the frame, so the one line // deleted takes it off every page at once. The row over a page says that page // is wrong; this says the frame is, and the two are told apart by both produced @@ -1078,7 +1202,7 @@ func TestAnImageWithoutItsDimensionsRedsExactlyOneRow(t *testing.T) { contentOpen+`The mark`, 1)) var refused []string - for _, r := range Rules() { + for _, r := range Rules(fixtureNumbers) { if len(r.decide(body)) > 0 { refused = append(refused, r.ID) } @@ -1144,7 +1268,7 @@ func TestAHeadWithNoSchemeRedsExactlyOneRow(t *testing.T) { ` `+"\n", "", 1)) var refused []string - for _, r := range Rules() { + for _, r := range Rules(fixtureNumbers) { if len(r.decide(body)) > 0 { refused = append(refused, r.ID) } @@ -1246,12 +1370,128 @@ func TestTheMotionRowNamesTheLineAndTheDeclaration(t *testing.T) { } } +// The two spellings one limit arrives in, and the longer one reported as itself +// rather than as the bare number inside it. A failure naming "37 ms" where the +// line says "under 37 ms" sends the next person looking for a string that is not +// there. +func TestTheBudgetRowRefusesBothSpellingsAndReportsTheLongerOne(t *testing.T) { + decide := decideTypedBudgetNumber(fixtureNumbers) + + for _, c := range []struct{ line, want string }{ + {"A key press answers " + fixtureNumbers[0].Stated() + ".", `"under 37 ms"`}, + {"The ceiling is 37 ms and nothing may exceed it.", `"37 ms"`}, + {"It drops exactly 4 frames on the worst row.", `"exactly 4 frames"`}, + } { + got := decide([]byte("A first line.\n" + c.line + "\n")) + if len(got) != 1 { + t.Errorf("%q produced %d detail(s), want 1: %v", c.line, len(got), got) + continue + } + if !strings.Contains(got[0], c.want) { + t.Errorf("the failure for %q reads %q, which does not carry %s", c.line, got[0], c.want) + } + if !strings.Contains(got[0], "line 2") { + t.Errorf("the failure for %q reads %q, which does not name the line", c.line, got[0]) + } + if !strings.Contains(got[0], tokens.File) { + t.Errorf("the failure for %q reads %q, which does not name the file it is read from", c.line, got[0]) + } + } +} + +// What the row leaves alone. It compares against the values it was handed rather +// than against the shape of a number, so a figure that is not one of them walks +// through, and a limit in another unit walks through as well. Both bounds are +// deliberate: a row that refused every number with a time unit on it would +// refuse a transition duration, and the file rather than this row is what the +// set of budget numbers is decided by. +func TestTheBudgetRowLeavesANumberThatIsNotABudgetNumberAlone(t *testing.T) { + decide := decideTypedBudgetNumber(fixtureNumbers) + + for name, line := range map[string]string{ + "a duration that is not a limit": "a { transition: color 120ms }", + "a neighbouring figure": "It answers under 38 ms on the machine it was measured on.", + "the same limit in another unit": "It answers under 0.037 s.", + "the digits with no unit": "Thirty seven is 37 and nothing follows it.", + } { + if got := decide([]byte(line + "\n")); len(got) != 0 { + t.Errorf("the row refused %s: %v", name, got) + } + } +} + +// The row follows the copy rather than carrying the numbers. A row holding the +// five values in its own source would be the second definition of the file it +// exists to keep as the only one, and the way that shows is here: the same line +// is refused or left alone depending on what the copy says, and nothing in this +// package decides which. +func TestTheBudgetRowFollowsTheCopyRatherThanCarryingTheNumbers(t *testing.T) { + line := []byte("A key press answers under 37 ms.\n") + + if got := decideTypedBudgetNumber(fixtureNumbers)(line); len(got) != 1 { + t.Fatalf("the row produced %d detail(s) against the copy that carries that limit, want 1: %v", len(got), got) + } + + moved := []tokens.Number{{Name: "first-frame", Limit: "24", Unit: "ms", Comparison: "below"}} + if got := decideTypedBudgetNumber(moved)(line); len(got) != 0 { + t.Errorf("the limit moved in the copy and the row went on refusing the old one: %v", got) + } + if got := decideTypedBudgetNumber(moved)([]byte("A key press answers under 24 ms.\n")); len(got) != 1 { + t.Errorf("the limit moved in the copy and the row did not refuse the new one: %v", got) + } +} + +// A row handed nothing refuses nothing, which is why the run reads the copy +// before it builds the table. This is that half stated where somebody changing +// the table will meet it, and the run's own refusal is the case below. +func TestTheBudgetRowHandedNothingRefusesNothing(t *testing.T) { + if got := decideTypedBudgetNumber(nil)([]byte("A key press answers under 37 ms.\n")); len(got) != 0 { + t.Errorf("a row that was told no numbers refused a line anyway: %v", got) + } +} + +// The table is the same table however it is handed, which is what lets the count +// printed by the gate be taken from a call that supplies nothing. A row that +// appeared only when values arrived would make that count a different number +// from the one the run decided. +func TestTheTableHoldsTheSameRowsHoweverItIsHanded(t *testing.T) { + var with, without []string + for _, r := range Rules(fixtureNumbers) { + with = append(with, r.ID) + } + for _, r := range Rules(nil) { + without = append(without, r.ID) + } + if strings.Join(with, ",") != strings.Join(without, ",") { + t.Errorf("the table handed values holds %v and the table handed nothing holds %v", with, without) + } +} + +// The limit typed into a page reds this row and no other, so a red run says +// which repair it wants rather than which area to look in. The colour row is the +// near neighbour: both are about a value that belongs in the copy, and a fixture +// tripping the two of them would leave neither proved. +func TestABudgetNumberTypedIntoAPageRedsExactlyOneRow(t *testing.T) { + body := []byte(strings.Replace(cleanPage, `

A title

`, + `

A title

A key press answers `+fixtureNumbers[0].Stated()+`.

`, 1)) + + var refused []string + for _, r := range Rules(fixtureNumbers) { + if len(r.decide(body)) > 0 { + refused = append(refused, r.ID) + } + } + if len(refused) != 1 || refused[0] != "client-budget-numbers-live-in-exactly-one-file" { + t.Errorf("the typed limit refused %v, want only client-budget-numbers-live-in-exactly-one-file", refused) + } +} + func TestMotionInTheCascadeRedsExactlyOneRow(t *testing.T) { moving := []byte(strings.Replace(cleanPage, ``, " \n ", 1)) var refused []string - for _, r := range Rules() { + for _, r := range Rules(fixtureNumbers) { if len(r.decide(moving)) > 0 { refused = append(refused, r.ID) } diff --git a/internal/site/designsystem.go b/internal/site/designsystem.go index b68a921..521c065 100644 --- a/internal/site/designsystem.go +++ b/internal/site/designsystem.go @@ -491,6 +491,11 @@ func endingIn(values tokens.Values, segment string) []string { // number are prose about the file, which is exactly what a budget row needs // beside a limit, and they are the reason the rule above keeps them rather than // discarding them. +// +// How a limit is spelled is asked of the token package rather than decided here. +// The invariant row that refuses a second copy of one of these numbers has to +// look for the same strings this table prints, and a page and a row each +// carrying their own spelling would part company on the day either moved. func clientBudget(values tokens.Values) (budgetTable, []string) { t := budgetTable{ Whose: "What a native client has to meet", @@ -498,27 +503,12 @@ func clientBudget(values tokens.Values) (budgetTable, []string) { "with the values above because they are the same class of fact, and a client " + "that has not been measured has not met one.", } - var reasons []string - for _, limit := range endingIn(values, "limit") { - if !strings.HasPrefix(limit, "budget.numbers.") { - continue - } - at := strings.TrimSuffix(limit, ".limit") - name := strings.TrimPrefix(at, "budget.numbers.") - unit, ok := values[at+".unit"] - if !ok { - reasons = append(reasons, fmt.Sprintf("%s carries a limit and no unit, so the number states nothing", at)) - continue - } - comparison, ok := values[at+".comparison"] - if !ok { - reasons = append(reasons, fmt.Sprintf("%s carries a limit and no comparison, so whether it is a ceiling or a value is not stated", at)) - continue - } + numbers, reasons := tokens.Numbers(values) + for _, n := range numbers { t.Rows = append(t.Rows, budgetRow{ - Name: name, - Limit: stated(comparison, values[limit], unit), - Means: strings.TrimSpace(values[at+".what"] + " " + values[at+".why"]), + Name: n.Name, + Limit: n.Stated(), + Means: strings.TrimSpace(n.What + " " + n.Why), }) } if len(t.Rows) == 0 { @@ -527,20 +517,6 @@ func clientBudget(values tokens.Values) (budgetTable, []string) { return t, reasons } -// stated turns a limit and its comparison into what the page says. The file -// carries the two apart, and a page printing the number alone would state a -// ceiling and a required value in the same words, which are opposite claims. -func stated(comparison, limit, unit string) string { - switch comparison { - case "below": - return "under " + limit + " " + unit - case "equal": - return "exactly " + limit + " " + unit - default: - return comparison + " " + limit + " " + unit - } -} - // siteBudget is what this page itself has to fit inside, read from the constants // the row that refuses a page reads. The published budget and the enforced one // are one set of numbers here, so the page cannot go on stating a limit nothing diff --git a/internal/site/designsystem_test.go b/internal/site/designsystem_test.go index 8da4e52..3b280d8 100644 --- a/internal/site/designsystem_test.go +++ b/internal/site/designsystem_test.go @@ -403,6 +403,46 @@ func TestThePageStatesBothBudgetsAndSaysWhichIsWhich(t *testing.T) { } } +// The budget line on the page and the number the copy carries are one value. +// This is the same statement the case above makes about a token, and it is worth +// making twice because the failure is not the same one: a colour rendered from +// somewhere other than the copy looks wrong to anybody who opens the page, and a +// latency ceiling rendered from somewhere else looks exactly like the right one. +func TestABudgetNumberChangedInTheCopyChangesExactlyThatLineOnThePage(t *testing.T) { + before := designPage(t, designTokens) + after := designPage(t, strings.Replace(designTokens, `"limit": 80,`, `"limit": 95,`, 1)) + + if !strings.Contains(before, "focus-changeunder 80 ms") { + t.Fatal("the page did not carry the limit the copy gives it") + } + if !strings.Contains(after, "focus-changeunder 95 ms") { + t.Error("the limit moved in the copy and did not move on the page") + } + if strings.Contains(after, "80 ms") { + t.Error("the page still carries the limit the copy no longer has") + } + if rows(before) != rows(after) { + t.Errorf("changing one limit moved the table from %d row(s) to %d", rows(before), rows(after)) + } +} + +// How a limit is spelled follows the copy as well. The file carries the number +// and the comparison apart, and a page that stated a ceiling and a required +// value in the same words would be printing two opposite claims identically. +func TestTheComparisonInTheCopyDecidesHowTheLimitIsStated(t *testing.T) { + page := designPage(t, strings.Replace(designTokens, `"comparison": "below",`, `"comparison": "equal",`, 1)) + + if !strings.Contains(page, "focus-changeexactly 80 ms") { + t.Error("the comparison moved in the copy and the page went on stating a ceiling") + } +} + +// rows counts the budget lines the page carries, so a case about one number can +// say that it moved nothing else. +func rows(page string) int { + return strings.Count(page, "") +} + // A limit whose comparison the file does not carry is a number that states // nothing: a ceiling and a required value are opposite claims and the page would // print them in the same words. diff --git a/internal/tokens/tokens.go b/internal/tokens/tokens.go index 4046820..f5c59fc 100644 --- a/internal/tokens/tokens.go +++ b/internal/tokens/tokens.go @@ -44,6 +44,7 @@ import ( "path/filepath" "sort" "strconv" + "strings" "time" ) @@ -122,6 +123,95 @@ func Read(body []byte) (Values, error) { return values, nil } +// budgetPrefix is where the numbers a client is held to sit in the file. It is +// a prefix rather than a list of the five, because the set is the file's to +// decide and a list here would be a second declaration of it. +const budgetPrefix = "budget.numbers." + +// Number is one of the numbers a client has to meet, read out of the copy. It +// carries the limit as the digits the file was written with rather than as an +// integer, for the reason Read keeps them: a limit reported back in another +// spelling is a difference a reader has to decode before they can see there is +// none. +type Number struct { + Name string + Limit string + Unit string + Comparison string + // What and Why are the file's own sentences beside the number. They are + // prose about the value rather than the value, which is why nothing + // compares them and why a page prints them beside the limit. + What string + Why string +} + +// Stated is how the number is written where somebody reads it. The file carries +// the limit and the comparison apart, and printing the number alone would state +// a ceiling and a required value in the same words, which are opposite claims. +// +// This is the one place that mapping is made. The page states a number by asking +// here, and the row that refuses a second copy of one asks here too, so a +// spelling the page uses and a spelling the row looks for cannot part company. +func (n Number) Stated() string { + switch n.Comparison { + case "below": + return "under " + n.Limit + " " + n.Unit + case "equal": + return "exactly " + n.Limit + " " + n.Unit + default: + return n.Comparison + " " + n.Limit + " " + n.Unit + } +} + +// Bare is the number with its unit and nothing in front of it. It is the other +// spelling the same value arrives in, and it is the one somebody types when they +// are quoting a limit inside a sentence rather than stating it in a table. +func (n Number) Bare() string { + return n.Limit + " " + n.Unit +} + +// Numbers reads the client budget out of a file that has been flattened. It +// returns one reason per number it could not make sense of rather than the +// first, because a file with three broken numbers is three repairs. +// +// An empty result carries no reason. Whether a file with no such number is a +// failure depends on what the caller was going to do with them, and the two +// callers answer it differently: a page with no budget table and a rule with +// nothing to compare against are refused in different words. +func Numbers(values Values) ([]Number, []string) { + var at []string + for p := range values { + if strings.HasPrefix(p, budgetPrefix) && strings.HasSuffix(p, ".limit") { + at = append(at, strings.TrimSuffix(p, ".limit")) + } + } + sort.Strings(at) + + var out []Number + var reasons []string + for _, a := range at { + unit, ok := values[a+".unit"] + if !ok { + reasons = append(reasons, fmt.Sprintf("%s carries a limit and no unit, so the number states nothing", a)) + continue + } + comparison, ok := values[a+".comparison"] + if !ok { + reasons = append(reasons, fmt.Sprintf("%s carries a limit and no comparison, so whether it is a ceiling or a value is not stated", a)) + continue + } + out = append(out, Number{ + Name: strings.TrimPrefix(a, budgetPrefix), + Limit: values[a+".limit"], + Unit: unit, + Comparison: comparison, + What: values[a+".what"], + Why: values[a+".why"], + }) + } + return out, reasons +} + // flatten walks the document. An array is indexed rather than joined, because // the order of a font stack is part of what it says and a reordering has to // read as a difference.