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+`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+`
`, 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 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, "