diff --git a/internal/budget/budget.go b/internal/budget/budget.go new file mode 100644 index 0000000..bc9949f --- /dev/null +++ b/internal/budget/budget.go @@ -0,0 +1,45 @@ +// Package budget holds the numbers decisions/0005-the-speed-budget.md fixes, and +// is the one place they are written. +// +// The record says the budget is written as numbers a build can miss rather than +// as an intention, and a number in a document that nothing reads is an intention +// again. So the numbers live here, the check that refuses a page reads them, and +// the page that publishes them reads the same constants. A published budget and +// an enforced budget that are two copies of one set are two copies that disagree +// the first time either moves, and the disagreement is invisible: the page goes on +// printing a limit nothing holds anybody to. +// +// What is here is only the part of the record a machine can decide by reading the +// bytes the build wrote. The record's last two lines are about what a browser does +// with those bytes, and neither can be decided from them, so neither is a constant +// here: putting them in this file would say a check reads them. +package budget + +// The lines of the record that are decidable by reading a produced page. +// +// The two sizes are uncompressed and per page, which is what the record measures +// and is the number a reader on a slow link pays. A limit written against a +// compressed size would move when whatever serves the file changes its +// compression, which is not a property of this repository. +const ( + // HTMLBytes is the whole document, markup and inlined stylesheet + // together, because that is what one request delivers. + HTMLBytes = 20 * 1024 + // InlineCSSBytes is what the document carries inside its own style + // elements. Inlining removes a round trip before anything renders, and + // this size is what keeps inlining cheaper than the request it replaced. + InlineCSSBytes = 12 * 1024 + // WebFontDownloads is zero. A downloaded face blocks or reflows the first + // text a reader sees, and the faces already on the reader's machine cost + // nothing and arrive first. + WebFontDownloads = 0 + // LandingImages is the most the page a reader arrives at may ask for + // after its document. The record counts the landing page's requests + // rather than every page's, because the first page has to be complete + // after the fewest exchanges. + LandingImages = 2 +) + +// Record is where each of these is argued, named so that a refusal can send a +// reader to the argument rather than to the number. +const Record = "decisions/0005-the-speed-budget.md" diff --git a/internal/invariant/invariant.go b/internal/invariant/invariant.go index 9a9b2a4..5afca96 100644 --- a/internal/invariant/invariant.go +++ b/internal/invariant/invariant.go @@ -37,6 +37,7 @@ import ( "strings" "time" + "github.com/Flowfin/site/internal/budget" "github.com/Flowfin/site/internal/changelog" "github.com/Flowfin/site/internal/markup" "github.com/Flowfin/site/internal/pins" @@ -97,6 +98,15 @@ type Rule struct { // carry rather than the page itself. It is what the run prints, so it // reads as a plural noun. Counted string + // Only narrows the population to one produced path, where the rule is + // about one page rather than about all of them. It exists because the + // speed budget counts the landing page's requests and says nothing about + // the other pages, so a row applying that line everywhere would refuse + // more than the record does, which is a rule nobody argued. A row that + // names a path the build did not write reports that it decided nothing + // rather than passing, so a page renamed out from under this does not + // leave a silently green row behind. + Only string // decide returns one detail per violation in body, or nothing. decide func(body []byte) []string // counts says how many things of the kind this row judges the body @@ -302,6 +312,37 @@ func Rules() []Rule { Refuses: "a produced page with no description element, or one whose content is missing or holds only whitespace", decide: decideDescription, }, + { + ID: "page-fits-the-markup-budget", + Subject: ProducedPages, + Reason: "the budget is written as numbers a build can miss rather than as an intention, and a page of prose that cannot be written inside that much markup is carrying structure the reader is not being shown", + Refuses: fmt.Sprintf("a produced page whose whole document is %d bytes or more, uncompressed", budget.HTMLBytes), + decide: decideMarkupBudget, + }, + { + ID: "page-fits-the-stylesheet-budget", + Subject: ProducedPages, + Reason: "inlining the stylesheet removes a round trip before anything renders, and the size is what keeps inlining cheaper than the request it replaced, so a page that outgrows it has given back what inlining bought", + Refuses: fmt.Sprintf("a produced page carrying %d bytes or more of inlined stylesheet", budget.InlineCSSBytes), + Counted: "inlined stylesheets", + decide: decideStylesheetBudget, + counts: countStylesheets, + }, + { + ID: "page-downloads-no-web-font", + Subject: ProducedPages, + Reason: "a downloaded face blocks or reflows the first text a reader sees, and the faces already on the reader's machine cost nothing and arrive first; the row about a foreign domain catches a face served from somebody else's host and says nothing about one this site would serve itself", + Refuses: "a produced page declaring a font face that fetches a file, wherever it is served from", + decide: decideWebFont, + }, + { + ID: "landing-page-asks-for-at-most-two-images", + Subject: ProducedPages, + Only: site.IndexPath, + Reason: "the first page has to be complete after the fewest exchanges, and a limit counted in requests is the one a reader on a slow link actually feels; the record counts this page's requests and says nothing about the others, so this row reads this page and no other", + Refuses: fmt.Sprintf("the produced landing page carrying more than %d image elements", budget.LandingImages), + decide: decideLandingImages, + }, { ID: "page-links-the-legal-notice", Subject: ProducedPages, @@ -804,6 +845,9 @@ func Run(root string, log io.Writer) error { if !ok { return fmt.Errorf("rule %s reads %q, which is not a population this run gathered", r.ID, r.Subject) } + if r.Only != "" { + files = onlyNamed(files, path.Join(site.OutputDir, r.Only)) + } var details []string for _, f := range files { for _, d := range r.decide(f.body) { @@ -820,6 +864,11 @@ func Run(root string, log io.Writer) error { } continue } + if len(files) == 0 && r.Only != "" { + fmt.Fprintf(log, " %s: the build wrote no %s, so this rule examined nothing\n", + r.ID, path.Join(site.OutputDir, r.Only)) + continue + } if len(files) == 0 { fmt.Fprintf(log, " %s: %s held no file, so this rule examined nothing\n", r.ID, r.Subject) continue @@ -1629,6 +1678,90 @@ func foreignHost(ref string) (string, bool) { return host, true } +// What the budget rows read. A style element is the only thing on these pages +// that carries a stylesheet, and a font face is the declaration that fetches one. +var ( + styleElement = regexp.MustCompile(`(?is)]*>(.*?)`) + fontFace = regexp.MustCompile(`(?is)@font-face\b[^{]*\{`) +) + +// decideMarkupBudget refuses a produced page larger than the record allows. +// +// The measured number and the limit are both in the refusal, because a budget +// failure that says only that the build failed makes the next person measure by +// hand, and the number they would reach for is the one this already has. +func decideMarkupBudget(body []byte) []string { + if len(body) < budget.HTMLBytes { + return nil + } + return []string{fmt.Sprintf( + "this page is %d bytes and the budget in %s puts a document under %d", + len(body), budget.Record, budget.HTMLBytes)} +} + +// decideStylesheetBudget refuses a produced page carrying more inlined stylesheet +// than the record allows. Every style element on the page is counted together, +// because what the reader waits for is the document and a second element is not a +// second budget. +func decideStylesheetBudget(body []byte) []string { + total := 0 + for _, m := range styleElement.FindAllSubmatchIndex(body, -1) { + total += m[3] - m[2] + } + if total < budget.InlineCSSBytes { + return nil + } + return []string{fmt.Sprintf( + "this page inlines %d bytes of stylesheet and the budget in %s puts it under %d", + total, budget.Record, budget.InlineCSSBytes)} +} + +// countStylesheets says how many style elements the page carries, so a page with +// none reports that rather than reporting ok against a limit it never approached. +func countStylesheets(body []byte) int { + return len(styleElement.FindAllIndex(body, -1)) +} + +// decideWebFont refuses a produced page declaring a face that fetches a file. +// +// It reads the declaration rather than the address, which is what makes it +// different from the row about a foreign domain: a face this site served itself +// would satisfy that row and cost the reader exactly what the budget puts at zero. +func decideWebFont(body []byte) []string { + var details []string + for _, m := range fontFace.FindAllIndex(body, -1) { + details = append(details, fmt.Sprintf( + "line %d declares a font face, and the budget in %s puts web font downloads at %d", + lineOf(body, m[0]), budget.Record, budget.WebFontDownloads)) + } + return details +} + +// decideLandingImages refuses the landing page for asking for more images than the +// record allows. The document itself is the one request the record counts beside +// them, and a page is one document by construction. +func decideLandingImages(body []byte) []string { + found := imgElement.FindAllIndex(body, -1) + if len(found) <= budget.LandingImages { + return nil + } + return []string{fmt.Sprintf( + "this page asks for %d image(s) after its document and the budget in %s allows %d", + len(found), budget.Record, budget.LandingImages)} +} + +// onlyNamed keeps the one file a narrowed row is about. A row naming a path the +// build did not write is left with nothing, which the run reports rather than +// passing over. +func onlyNamed(files []file, name string) []file { + for _, f := range files { + if f.name == name { + return []file{f} + } + } + return nil +} + // decideLegalLink refuses a produced page that offers no way to the page saying // who publishes this site. // diff --git a/internal/invariant/invariant_test.go b/internal/invariant/invariant_test.go index a18c803..4baffc3 100644 --- a/internal/invariant/invariant_test.go +++ b/internal/invariant/invariant_test.go @@ -18,10 +18,12 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "testing" "time" + "github.com/Flowfin/site/internal/budget" "github.com/Flowfin/site/internal/security" "github.com/Flowfin/site/internal/site" "github.com/Flowfin/site/internal/tokens" @@ -141,6 +143,28 @@ func TestEveryRowRefusesItsOwnViolationAndPassesTheNeighbour(t *testing.T) { // search result showing its address. "page-carries-a-description": []byte(strings.Replace(cleanPage, `content="What this page is, in one sentence."`, `content=""`, 1)), + // A document grown past the limit, which is what a page does when + // its prose keeps arriving and nobody measures. The padding is a + // comment so that nothing else on the page changes with it, and it + // is assembled rather than written out because a test source + // carrying twenty kilobytes of anything is unreadable. + "page-fits-the-markup-budget": []byte(strings.Replace(cleanPage, contentOpen, + contentOpen+"", 1)), + // A stylesheet that outgrew what inlining bought, which is what + // happens to an inlined one: the request it replaced is cheap + // again and nobody is watching the size. + "page-fits-the-stylesheet-budget": []byte(strings.Replace(cleanPage, ``, + " \n ", 1)), + // One face, of the shape a copied stylesheet arrives in, and + // served from this site so that the row about a foreign domain + // says nothing about it. + "page-downloads-no-web-font": []byte(strings.Replace(cleanPage, ``, + " \n ", 1)), + // One image more than the landing page may ask for, each carrying + // everything else the page rows require, so this row is the only + // one the fixture trips. + "landing-page-asks-for-at-most-two-images": []byte(strings.Replace(cleanPage, contentOpen, + contentOpen+strings.Repeat(`A picture of something`, budget.LandingImages+1), 1)), // The link taken out of the frame's footer, which is what // tidying a footer looks like. Every page still renders, and // whoever publishes the site is reachable from whichever pages @@ -1281,3 +1305,88 @@ func TestRunRefusesATreeCarryingNoSourceForTheReportingRoute(t *testing.T) { t.Errorf("the refusal reads %q, which does not name the file that is missing", err) } } + +// The measured number and the limit are both in the refusal. A budget failure +// that says only that the build failed makes the next person measure by hand, and +// the number they would reach for is the one the row already has. +func TestTheBudgetRowsNameTheMeasurementAndTheLimit(t *testing.T) { + oversize := []byte(strings.Replace(cleanPage, contentOpen, + contentOpen+"", 1)) + got := decideMarkupBudget(oversize) + if len(got) != 1 { + t.Fatalf("the row produced %d detail(s), want 1: %v", len(got), got) + } + for _, want := range []string{ + strconv.Itoa(len(oversize)), + strconv.Itoa(budget.HTMLBytes), + budget.Record, + } { + if !strings.Contains(got[0], want) { + t.Errorf("the refusal reads %q, which does not carry %q", got[0], want) + } + } + + heavy := []byte(strings.Replace(cleanPage, ``, + " \n ", 1)) + got = decideStylesheetBudget(heavy) + if len(got) != 1 { + t.Fatalf("the stylesheet row produced %d detail(s), want 1: %v", len(got), got) + } + for _, want := range []string{strconv.Itoa(budget.InlineCSSBytes), budget.Record} { + if !strings.Contains(got[0], want) { + t.Errorf("the refusal reads %q, which does not carry %q", got[0], want) + } + } +} + +// A page exactly at the limit is refused and a page one byte under is not, because +// the record writes each line as a limit rather than as a target and the byte at +// the boundary is the one nobody tests. +func TestTheMarkupBudgetRowRefusesTheBoundary(t *testing.T) { + at := make([]byte, budget.HTMLBytes) + if got := decideMarkupBudget(at); len(got) == 0 { + t.Errorf("the row passed a page of exactly %d bytes", budget.HTMLBytes) + } + under := make([]byte, budget.HTMLBytes-1) + if got := decideMarkupBudget(under); len(got) != 0 { + t.Errorf("the row refused a page one byte under the limit: %v", got) + } +} + +// The image line is written for the landing page and says nothing about the +// others, so a frame putting three images on every page reds this row once. A row +// applying it everywhere would refuse more than the record does, which is a rule +// nobody argued. +func TestTheLandingImageRowReadsTheLandingPageAndNoOther(t *testing.T) { + images := strings.Repeat(`A picture of something`, + budget.LandingImages+1) + root := tree(t, strings.Replace(goodTemplate, `

{{ .Title }}

`, + `

{{ .Title }}

`+images, 1)) + + var log bytes.Buffer + if err := Run(root, &log); err == nil { + t.Fatalf("Run accepted a frame asking for more images than the budget allows:\n%s", log.String()) + } + if !strings.Contains(log.String(), "landing-page-asks-for-at-most-two-images: REFUSED, 1 violation(s)") { + t.Errorf("the run does not refuse exactly the landing page; it said:\n%s", log.String()) + } + if !strings.Contains(log.String(), "dist/index.html: this page asks for 3 image(s)") { + t.Errorf("the run does not name the landing page and its count; it said:\n%s", log.String()) + } + if strings.Contains(log.String(), "dist/privacy/index.html: this page asks for") { + t.Errorf("the run judged a page this row is not about; it said:\n%s", log.String()) + } +} + +// A narrowed row whose page the build did not write is left with nothing to read. +// It reports that rather than passing, so a page renamed out from under one of +// these does not leave a green row behind it. +func TestANarrowedRowFindsNothingWhenThePageIsNotThere(t *testing.T) { + files := []file{{name: "dist/index.html", body: []byte("a")}} + if got := onlyNamed(files, "dist/index.html"); len(got) != 1 { + t.Errorf("the narrowing dropped the page it was asked for: %v", got) + } + if got := onlyNamed(files, "dist/install/index.html"); len(got) != 0 { + t.Errorf("the narrowing returned %d file(s) for a page the build did not write", len(got)) + } +}