diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c9f9d1..e80b537 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,3 +105,12 @@ there is no earlier bundle to compare it against. counted and said, so a page listing the generations some of the releases state is not read as one listing all of them. The record refuses a plugin recorded as shipping with nothing saying which server it is for. +- The gate refuses a server generation, a token value or a client limit written + into the files the build reads values out of, which it did not before. Those + files sat outside all three rows: each of them was sorted into the population + asking whether it is present at all, and being sorted there ended its walk + before it could be counted as something the build reads, so the three rows + judged the frame and the prose and nothing else. A generation typed into the + roster passed the whole gate. Each row now reads every file the build reads + except the one file that row is the authority for, so the file declaring a + value is not refused for declaring it. diff --git a/internal/invariant/invariant.go b/internal/invariant/invariant.go index f1a4232..47b50ce 100644 --- a/internal/invariant/invariant.go +++ b/internal/invariant/invariant.go @@ -89,6 +89,20 @@ const ( // file is a copy of output rather than a source of it. Only what the // build reads can put a wrong value on a page. BuildInputs = "every tracked file the build reads to render a page" + // BuildInputsOutsideTheTokenFile and BuildInputsOutsideTheClientsFile are + // the same population with one file removed, and the file removed is the + // one the row using it is the authority for. A row refusing a second copy + // of a value cannot read the first copy as a second one, so each of those + // two rows needs the build inputs minus its own source rather than the + // build inputs minus every source there is. + // + // Cutting one population for all three was what put the roster, the + // clients file, the token copy and the security contact outside every one + // of them: a file sorted into a population of its own left the walk before + // it could be counted here as well, so the rows judged the templates and + // the prose and nothing the build reads a value out of. + BuildInputsOutsideTheTokenFile = "every tracked file the build reads to render a page, outside the file that holds the token values" + BuildInputsOutsideTheClientsFile = "every tracked file the build reads to render a page, outside the file that holds the client claim" ) // vocabularyFile is the path excluded from TrackedText, and it is derived from @@ -531,14 +545,14 @@ func Rules(numbers []tokens.Number) []Rule { }, { ID: "design-tokens-live-in-exactly-one-file", - Subject: BuildInputs, + Subject: BuildInputsOutsideTheTokenFile, Reason: "a value typed into a template is a second definition of a value published somewhere else, and the day the published one moves the page goes on rendering the old one perfectly, so nobody sees it; the file carries lengths, weights and font stacks beside the colours, and a wrong length is the harder one to see because a wrong colour at least looks wrong", 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, + Subject: BuildInputsOutsideTheClientsFile, 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), @@ -1300,6 +1314,8 @@ func gather(root string) (map[string][]file, error) { SourceFiles: tracked.sources, Workflows: tracked.workflows, BuildInputs: tracked.buildInputs, + BuildInputsOutsideTheTokenFile: tracked.outsideTokens, + BuildInputsOutsideTheClientsFile: tracked.outsideClients, TrackedTextOutsideTheVersionRegister: tracked.outsideVersion, }, nil } @@ -1314,6 +1330,8 @@ type tracked struct { sources []file workflows []file buildInputs []file + outsideTokens []file + outsideClients []file tokenCopies []file securitySources []file clientClaims []file @@ -1369,24 +1387,34 @@ func trackedText(root string) (tracked, error) { if !versionRegister(name) { found.outsideVersion = append(found.outsideVersion, f) } + // These four sort a file into the population that asks whether it + // is there at all. None of them ends the file's walk: a file that + // is one of these is also something the build reads, and returning + // here is what took every one of them out of the populations below + // while the subject of those populations went on saying it held + // them. if name == tokens.File { found.tokenCopies = append(found.tokenCopies, f) - continue } if name == security.File { found.securitySources = append(found.securitySources, f) - continue } if name == site.ClientsFile { found.clientClaims = append(found.clientClaims, f) - continue } if name == site.RosterFile { found.rosterCopies = append(found.rosterCopies, f) - continue } - if strings.HasPrefix(name, site.TemplatesDir+"/") || strings.HasPrefix(name, site.ContentDir+"/") { + if strings.HasPrefix(name, site.TemplatesDir+"/") || + strings.HasPrefix(name, site.ContentDir+"/") || + strings.HasPrefix(name, site.DataDir+"/") { found.buildInputs = append(found.buildInputs, f) + if name != tokens.File { + found.outsideTokens = append(found.outsideTokens, f) + } + if name != site.ClientsFile { + found.outsideClients = append(found.outsideClients, f) + } } } return found, nil diff --git a/internal/invariant/invariant_test.go b/internal/invariant/invariant_test.go index d736c54..9ad3ca1 100644 --- a/internal/invariant/invariant_test.go +++ b/internal/invariant/invariant_test.go @@ -2238,3 +2238,112 @@ func TestTheLandingImageRowCountsNoOtherLinkElement(t *testing.T) { } } } + +// The population each build-input row is decided over holds every file the +// build reads, and the file a row is the authority for is out of that row's +// population and out of no other. +// +// This is the case the rest of the suite could not make. Every other proof of +// these three rows hands bytes straight to the decider, so the decider is +// judged and the selection that chooses which files it ever meets is judged by +// nothing. The selection was cutting one population for all three rows and +// removing every authority file from it, and a file sorted into a population of +// its own left the walk before it could be counted as a build input at all, so +// the roster, the clients file, the token copy and the security contact were +// outside all three rows while the subject of those rows said they were in. +// A generation typed into the roster passed the whole gate. +func TestEachBuildInputRowSeesEveryFileTheBuildReadsExceptItsOwnSource(t *testing.T) { + root := tree(t, goodTemplate) + found, err := gather(root) + if err != nil { + t.Fatalf("gathering the populations: %v", err) + } + + holds := func(subject, name string) bool { + for _, f := range found[subject] { + if f.name == name { + return true + } + } + return false + } + + for _, c := range []struct { + subject string + name string + want bool + why string + }{ + {BuildInputs, site.RosterFile, true, + "the roster is what every plugin row and every plugin page is rendered from"}, + {BuildInputs, site.ClientsFile, true, + "the claim about the clients is a sentence on the landing page"}, + {BuildInputs, tokens.File, true, + "the token copy is what the design system page renders"}, + {BuildInputs, security.File, true, + "the reporting route is written into what the build produces"}, + {BuildInputs, releases.File, true, + "the recorded releases decide the word each plugin row carries"}, + {BuildInputs, "templates/page.html.tmpl", true, + "the frame is the file every page is rendered through"}, + + {BuildInputsOutsideTheTokenFile, tokens.File, false, + "the row over this population refuses a second copy of a token value, and the first copy is not a second one"}, + {BuildInputsOutsideTheTokenFile, site.RosterFile, true, + "the roster is not the authority for a token value, so a colour typed into it is a second copy"}, + {BuildInputsOutsideTheTokenFile, site.ClientsFile, true, + "the clients file is not the authority for a token value either"}, + + {BuildInputsOutsideTheClientsFile, site.ClientsFile, false, + "the row over this population refuses a second copy of a client limit, and the first copy is not a second one"}, + {BuildInputsOutsideTheClientsFile, site.RosterFile, true, + "the roster is not the authority for a client limit"}, + {BuildInputsOutsideTheClientsFile, tokens.File, true, + "the token copy is where the client limits are declared, and this row reads the copy rather than being the copy"}, + } { + if got := holds(c.subject, c.name); got != c.want { + t.Errorf("%s in %q is %v, want %v: %s", c.name, c.subject, got, c.want, c.why) + } + } +} + +// The end of the same case, over a run rather than over a population. A server +// generation typed into the roster reds the gate and the refusal names the +// file, the line and the number, which is what issue #91's last condition asks +// for and what a green run reported instead. +func TestRunRefusesAServerGenerationTypedIntoTheRoster(t *testing.T) { + root := tree(t, goodTemplate) + if err := os.WriteFile(filepath.Join(root, filepath.FromSlash(site.RosterFile)), + []byte(`[{"id":"alpha","repository":"Flowfin/jellyfin-plugin-alpha",`+ + `"summary":"What alpha does on Jellyfin 10.11","state":"build-up"}]`), 0o644); err != nil { + t.Fatalf("rewriting the roster: %v", err) + } + git(t, root, "add", "-A") + + var log bytes.Buffer + if err := Run(root, &log); err == nil { + t.Fatalf("Run accepted a roster stating a server generation:\n%s", log.String()) + } + for _, want := range []string{ + "build-input-carries-no-server-generation: REFUSED, 1 violation(s)", + site.RosterFile + ": line 1", + "states the server generation 10.11", + } { + 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 generation taken back out. Without this the case above +// would pass over a run that refuses every roster, which proves the opposite of +// what it is for. +func TestRunAcceptsARosterThatStatesNoGeneration(t *testing.T) { + var log bytes.Buffer + if err := Run(tree(t, goodTemplate), &log); err != nil { + t.Fatalf("Run refused a roster stating no generation: %v\n%s", err, log.String()) + } + if !strings.Contains(log.String(), "build-input-carries-no-server-generation: ok") { + t.Errorf("the run did not report the row as examined; it said:\n%s", log.String()) + } +} diff --git a/internal/site/site.go b/internal/site/site.go index b7e8a44..422da17 100644 --- a/internal/site/site.go +++ b/internal/site/site.go @@ -28,9 +28,17 @@ import ( ) // The directories the build reads and the one it writes. +// +// DataDir is where the values a page states are read from, as against the words +// a page is written in, which are in ContentDir. It is a directory rather than a +// list of the files in it because every tracked file under it is read to render +// a page, and a list here would have to be widened by hand on the day a page +// gains a value - which is how the population that judges these files came to +// leave every one of them out. const ( TemplatesDir = "templates" ContentDir = "content" + DataDir = "data" AssetsDir = "assets" OutputDir = "dist" )