ci: a docs-only PR still bought three OS matrices, and four guards a comment could satisfy (#378, #379)

internal/db/facebook_ui_drift_test.go

@@ -2,10 +2,10 @@ package db
 
 import (
 	"encoding/json"
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
+
+	"github.com/rainmanjam/polyemesis/internal/testenv"
 )
 
 /* ===========================================================================
@@ -40,59 +40,13 @@ import (
 
    =========================================================================== */
 
-// stripJSComments blanks out comments so a marker left behind in one cannot
-// satisfy a guard that is asking whether a control renders. That is not
-// hypothetical: the honest way to keep a substring guard green while deleting
-// the thing it watches is to leave the words in a comment, and this branch's
-// own audit records that temptation being declined by hand rather than by a
-// guard.
-//
-// Block comments (`/* */`, and the `{/* */}` JSX form, which is a block comment
-// inside an expression container) are removed outright. Line comments are
-// removed only when nothing before the `//` on that line is quoted, so
-// a URL in a string literal or a template literal is left alone rather than
-// truncated at the scheme separator.
-//
-// Newlines are preserved so a line number quoted in a failure still means
-// something to whoever goes to look.
-func stripJSComments(src string) string {
-	var b strings.Builder
-	b.Grow(len(src))
-	for i := 0; i < len(src); {
-		if strings.HasPrefix(src[i:], "/*") {
-			end := strings.Index(src[i+2:], "*/")
-			if end < 0 {
-				break // unterminated; the rest is comment
-			}
-			for _, r := range src[i : i+2+end+2] {
-				if r == '\n' {
-					b.WriteByte('\n')
-				}
-			}
-			i += 2 + end + 2
-			continue
-		}
-		if strings.HasPrefix(src[i:], "//") && !quotedBefore(src, i) {
-			end := strings.IndexByte(src[i:], '\n')
-			if end < 0 {
-				break
-			}
-			i += end // leave the newline for the next iteration
-			continue
-		}
-		b.WriteByte(src[i])
-		i++
-	}
-	return b.String()
-}
-
-// quotedBefore reports whether a quote character appears between the start of
-// the line containing i and i itself -- the cheap test for "this `//` is inside
-// a string literal or JSX attribute rather than starting a comment".
-func quotedBefore(src string, i int) bool {
-	start := strings.LastIndexByte(src[:i], '\n') + 1
-	return strings.ContainsAny(src[start:i], "\"'`")
-}
+// READING THE SOURCE AND BLANKING ITS COMMENTS both live in internal/testenv
+// now -- testenv.ReadUI and testenv.StripJSComments, #379. They were written
+// here, and they were the only copy, which is why the guards in internal/oauth
+// spent their whole life defeatable by a comment: the alternative to importing
+// them was pasting forty lines into a second package. The reasoning that used to
+// sit here in full is in internal/testenv/uisource.go, next to the code, so
+// there is one place to read it and one place to change it.
 
 // jsxBlockUnder returns the source of the subtree a JSX conditional renders,
 // given the WHOLE head of that conditional including its opening paren -- e.g.
@@ -113,7 +67,7 @@ func jsxBlockUnder(t *testing.T, src, head, file string) string {
 		t.Fatalf("guard bug: %q is not a whole conditional head; it must end with the "+
 			"opening paren so the block can be bounded", head)
 	}
-	stripped := stripJSComments(src)
+	stripped := testenv.StripJSComments(src)
 	switch n := strings.Count(stripped, head); {
 	case n == 0:
 		t.Fatalf("%s no longer contains %s\n\n"+
@@ -145,17 +99,6 @@ func jsxBlockUnder(t *testing.T, src, head, file string) string {
 	return ""
 }
 
-// readUI reads a file under ui/src, from internal/db.
-func readUI(t *testing.T, parts ...string) string {
-	t.Helper()
-	path := filepath.Join(append([]string{"..", "..", "ui", "src"}, parts...)...)
-	raw, err := os.ReadFile(path)
-	if err != nil {
-		t.Fatalf("cannot read %s: %v", path, err)
-	}
-	return string(raw)
-}
-
 // The head of the Facebook create-time settings block. Everything the crosspost
 // list, the donate field and the backup toggle need is inside it, so all three
 // guards below bound themselves the same way.
@@ -176,7 +119,7 @@ const facebookBlockHead = `{platform === "facebook" && (`
 // searched the whole file for "Crosspost to Pages" and `id="dest-fb-donate"`,
 // both of which survive that mutation untouched, and stayed green.
 func TestFacebookCrosspostAndDonateAreOfferedByTheDestinationEditor(t *testing.T) {
-	src := readUI(t, "components", "DestinationDialog.tsx")
+	src := testenv.ReadUI(t, "components", "DestinationDialog.tsx")
 	block := jsxBlockUnder(t, src, facebookBlockHead, "DestinationDialog.tsx")
 
 	// The key, not the English. Where the WORDS live is a separate question,
@@ -212,7 +155,7 @@ func TestFacebookCrosspostAndDonateAreOfferedByTheDestinationEditor(t *testing.T
 //
 // This one was already bounded to the right span and is left as it was.
 func TestDestinationDialogSavePayloadCarriesTheFacebookBlock(t *testing.T) {
-	src := readUI(t, "components", "DestinationDialog.tsx")
+	src := testenv.ReadUI(t, "components", "DestinationDialog.tsx")
 
 	marker := "const payload: Partial<Destination> = {"
 	start := strings.Index(src, marker)
@@ -246,7 +189,7 @@ func TestDestinationDialogSavePayloadCarriesTheFacebookBlock(t *testing.T) {
 // whole card for the href template and stayed green through it, because the
 // href is still written inside the block that no longer renders.
 func TestTheCardLinksToTheScheduledBroadcast(t *testing.T) {
-	src := readUI(t, "components", "DestinationCard.tsx")
+	src := testenv.ReadUI(t, "components", "DestinationCard.tsx")
 	block := jsxBlockUnder(t, src, "{dest.facebookBroadcastId && (", "DestinationCard.tsx")
 
 	if !strings.Contains(block, "facebook.com/${dest.facebookBroadcastId}") {
@@ -285,7 +228,7 @@ func TestTheCardLinksToTheScheduledBroadcast(t *testing.T) {
 // regression -- a backup that died while the primary was offline is exactly the
 // case the operator needs to see.
 func TestTheCardShowsTheBackupFeedsState(t *testing.T) {
-	src := readUI(t, "components", "DestinationCard.tsx")
+	src := testenv.ReadUI(t, "components", "DestinationCard.tsx")
 
 	state := jsxBlockUnder(t, src, "{dest.backupProcess && (", "DestinationCard.tsx")
 	if !strings.Contains(state, "dest.backupProcess.state") {
@@ -312,7 +255,7 @@ func TestTheCardShowsTheBackupFeedsState(t *testing.T) {
 // destination beside the endpoint it gates, and a guard still spelling the old
 // shape would be a guard requiring the defect.
 func TestTheDialogOffersTheBackupIngestToggle(t *testing.T) {
-	src := readUI(t, "components", "DestinationDialog.tsx")
+	src := testenv.ReadUI(t, "components", "DestinationDialog.tsx")
 	block := jsxBlockUnder(t, src, facebookBlockHead, "DestinationDialog.tsx")
 
 	if !strings.Contains(block, "setBackupIngestWanted(e.target.checked)") {
@@ -362,7 +305,7 @@ func TestTheDialogOffersTheBackupIngestToggle(t *testing.T) {
 // twice the upload, and will find out during a broadcast.
 func TestTheFacebookCopyLivesInTheCatalogue(t *testing.T) {
 	var en map[string]string
-	if err := json.Unmarshal([]byte(readUI(t, "lib", "i18n", "en.json")), &en); err != nil {
+	if err := json.Unmarshal([]byte(testenv.ReadUI(t, "lib", "i18n", "en.json")), &en); err != nil {
 		t.Fatalf("en.json is not a flat string map: %v", err)
 	}
 

internal/db/ingest_header_drift_test.go

@@ -3,6 +3,8 @@ package db
 import (
 	"strings"
 	"testing"
+
+	"github.com/rainmanjam/polyemesis/internal/testenv"
 )
 
 // WHY THIS IS IN internal/db, stated rather than left to be discovered.
@@ -13,12 +15,16 @@ import (
 // owned by internal/engine (reconcileIngest, which returns early for SRT) and
 // internal/stats (the received-byte counter the bitrate series is sampled from).
 //
-// It lives here to reuse readUI and stripJSComments from facebook_ui_drift_test.go,
-// which is helper locality rather than a reason, and the honest consequence is
-// that someone changing engine's ingest reconciliation will not see a db test in
-// their package. `go test ./...` still runs it, so the guard holds; only its
-// discoverability is worse. Moving it to internal/engine means copying both
-// helpers there, so the fix is to promote them to a shared test helper first.
+// It landed here to reuse readUI and stripJSComments from
+// facebook_ui_drift_test.go, which is helper locality rather than a reason, and
+// the honest consequence is that someone changing engine's ingest reconciliation
+// will not see a db test in their package. `go test ./...` still runs it, so the
+// guard holds; only its discoverability is worse. What blocked the move was that
+// internal/engine would need its own copy of both helpers -- and #379 has now
+// removed that blocker: they are testenv.ReadUI and testenv.StripJSComments, and
+// any package can import them. What remains is the move itself, which is left to
+// a change of its own because it changes which package a failure points at, and
+// that is a decision about who gets paged rather than a tidy-up.
 //
 // The header's ingest indicator must not decide health from the ingest PROCESS.
 //
@@ -59,7 +65,7 @@ import (
 // removing the thing it watches is to leave the words behind, so the words are
 // not what is read.
 func TestTheHeaderAsksTheAppsOneQuestionAboutBeingLive(t *testing.T) {
-	src := stripJSComments(readUI(t, "components", "AppLayout.tsx"))
+	src := testenv.StripJSComments(testenv.ReadUI(t, "components", "AppLayout.tsx"))
 
 	if !strings.Contains(src, "useIngestLive()") {
 		t.Error("AppLayout no longer calls useIngestLive. An SRT source has no ingest " +
@@ -83,7 +89,7 @@ func TestTheHeaderAsksTheAppsOneQuestionAboutBeingLive(t *testing.T) {
 // at status.ingest.progress would silently restore the original bug in a place
 // nobody would think to look.
 func TestIngestLiveIsDerivedFromArrivingBytes(t *testing.T) {
-	src := stripJSComments(readUI(t, "hooks", "useLiveData.ts"))
+	src := testenv.StripJSComments(testenv.ReadUI(t, "hooks", "useLiveData.ts"))
 
 	// Not `src[strings.Index(...):]` unguarded: a rename made that a slice-bounds
 	// panic rather than a failure anyone could read, which is the guard going

internal/oauth/capabilities_drift_test.go

@@ -1,11 +1,11 @@
 package oauth
 
 import (
-	"os"
-	"path/filepath"
 	"regexp"
 	"strings"
 	"testing"
+
+	"github.com/rainmanjam/polyemesis/internal/testenv"
 )
 
 // The capability matrix exists twice, and nothing was checking the copies agree.
@@ -26,13 +26,15 @@ import (
 // and each capability's support value -- and not the prose, because the reason
 // strings are written for two different audiences and forcing them identical
 // would make the guard fight the thing it protects.
+// COMMENTS ARE BLANKED FIRST, #379. This guard reads the TypeScript as text, so
+// until now a row deleted from the matrix and left behind as a comment satisfied
+// it exactly as well as a row that renders -- and a commented-out platform is
+// the single most likely way a row leaves this file. The stripper existed in
+// internal/db and this package had no way to reach it; it is testenv.
+// StripJSComments now.
 func TestTheUICapabilityMatrixAgreesWithGo(t *testing.T) {
-	path := filepath.Join("..", "..", "ui", "src", "lib", "capabilities.ts")
-	raw, err := os.ReadFile(path)
-	if err != nil {
-		t.Fatalf("cannot read %s: %v", path, err)
-	}
-	ui := parseUICapabilities(t, string(raw))
+	ui := parseUICapabilities(t, testenv.StripJSComments(
+		testenv.ReadUI(t, "lib", "capabilities.ts")))
 
 	for _, row := range platformCapabilities {
 		got, ok := ui[row.PresetID]
@@ -100,6 +102,11 @@ var (
 // which would make this guard skip itself on any machine where that is missing
 // -- and a guard that skips silently is worse than no guard, because the next
 // person reads green and believes it.
+//
+// src is expected to have been through testenv.StripJSComments. That is not
+// merely defensive: every regex below would otherwise read a commented-out row
+// as a live one, which turns "the UI still ships this platform" into "somebody
+// once typed this platform".
 func parseUICapabilities(t *testing.T, src string) map[string]uiRow {
 	t.Helper()
 

internal/oauth/composer_tags_drift_test.go

@@ -1,12 +1,25 @@
 package oauth
 
 import (
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
+
+	"github.com/rainmanjam/polyemesis/internal/testenv"
 )
 
+// dashboardSource is Dashboard.tsx with its comments blanked, #379.
+//
+// Both guards below are substring searches over a window of this file, and a
+// substring search is satisfied by a comment. Neither of them stripped, so
+// either could have been kept green by deleting the code and leaving the words
+// -- and for the compliance guard, whose window is the whole file, a comment
+// anywhere at all would have done it. Blanking happens here, once, so a third
+// guard added to this file cannot forget.
+func dashboardSource(t *testing.T) string {
+	t.Helper()
+	return testenv.StripJSComments(testenv.ReadUI(t, "pages", "Dashboard.tsx"))
+}
+
 // The composer must be able to SEND tags, not merely render them back.
 //
 // TestUITypesCanNameEveryMetadataField walks the field NAMES a push result can
@@ -19,12 +32,7 @@ import (
 // Dashboard.tsx for unrelated reasons, so a whole-file search would pass on a
 // composer that still cannot send them.
 func TestTheComposerCanSendFacebookTags(t *testing.T) {
-	path := filepath.Join("..", "..", "ui", "src", "pages", "Dashboard.tsx")
-	raw, err := os.ReadFile(path)
-	if err != nil {
-		t.Fatalf("cannot read %s: %v", path, err)
-	}
-	src := string(raw)
+	src := dashboardSource(t)
 	body := strings.Index(src, `metaFetch<MetaJob>("/metadata/push"`)
 	if body < 0 {
 		t.Fatal("cannot find the metadata push call in Dashboard.tsx; this guard " +
@@ -54,12 +62,7 @@ func TestTheComposerCanSendFacebookTags(t *testing.T) {
 // Matches the derived list's use rather than its definition, because the name
 // appears at both and only the uses do anything.
 func TestTheComposerSaysWhenAPushCarriesStoredCompliance(t *testing.T) {
-	path := filepath.Join("..", "..", "ui", "src", "pages", "Dashboard.tsx")
-	raw, err := os.ReadFile(path)
-	if err != nil {
-		t.Fatalf("cannot read %s: %v", path, err)
-	}
-	src := string(raw)
+	src := dashboardSource(t)
 	if !strings.Contains(src, "withCompliance.length > 0") {
 		t.Error("the composer never mentions stored compliance, so a push sends a COPPA " +
 			"declaration or a privacy setting with nothing on screen having said so")

internal/oauth/ui_drift_test.go

@@ -1,11 +1,11 @@
 package oauth
 
 import (
-	"os"
-	"path/filepath"
 	"regexp"
 	"strings"
 	"testing"
+
+	"github.com/rainmanjam/polyemesis/internal/testenv"
 )
 
 // The same guard internal/db keeps over Rendition, one layer over.
@@ -23,15 +23,10 @@ import (
 // or a blank -- it is a row that reports nothing, on the screen an operator is
 // looking at seconds before going live.
 func TestUITypesCanNameEveryMetadataField(t *testing.T) {
-	path := filepath.Join("..", "..", "ui", "src", "lib", "types.ts")
-	raw, err := os.ReadFile(path)
-	if err != nil {
-		t.Fatalf("cannot read %s: %v", path, err)
-	}
-	union, ok := tsUnion(string(raw), "MetaField")
+	union, ok := tsUnion(testenv.StripJSComments(testenv.ReadUI(t, "lib", "types.ts")), "MetaField")
 	if !ok {
-		t.Fatalf("no `export type MetaField = ...` in %s. It was moved there from "+
-			"Dashboard.tsx precisely so this guard has one canonical place to read", path)
+		t.Fatal("no `export type MetaField = ...` in ui/src/lib/types.ts. It was moved " +
+			"there from Dashboard.tsx precisely so this guard has one canonical place to read")
 	}
 
 	for _, f := range AllMetadataFields {
@@ -100,11 +95,28 @@ func TestEveryMetadataFieldIsAdvertisedBySomePlatform(t *testing.T) {
 	}
 }
 
-// tsUnion returns the body of `export type <name> = ...;`, comments included.
+// tsUnion returns the body of `export type <name> = ...;`.
+//
+// IT USED TO SAY comments were kept deliberately, on the grounds that the union
+// carries a note per group about which push path produces those fields and that
+// stripping them "would make the file harder to read in exchange for nothing".
+// That reasoning was wrong twice over and #379 is the correction.
+//
+// It was not in exchange for nothing. The forward check below is
+// `strings.Contains(union, "\"tags\"")`, so deleting a member and leaving
+// `// "tags" -- removed, see ...` behind kept this guard green over a union that
+// could no longer name the field. That is the whole failure mode this family of
+// guards exists to catch, and this one was open to it.
+//
+// It also was not free in the other direction: the body is bounded by the first
+// `;` after the type name, and a semicolon inside one of those explanatory
+// comments truncates the union early, hiding every member after it from a check
+// that would then fail while naming the wrong cause.
 //
-// Comments are kept deliberately: the union carries a note per group explaining
-// which push path produces those fields, and a helper that stripped them would
-// make the file harder to read in exchange for nothing.
+// Callers pass source that has already been through testenv.StripJSComments, so
+// what is returned is the union as the compiler sees it. Nobody's reading
+// experience changes -- the notes are still in types.ts, this just stops them
+// counting as declarations.
 func tsUnion(src, name string) (string, bool) {
 	start := strings.Index(src, "export type "+name+" =")
 	if start < 0 {