From b4818b6406298c9ac4607510396325d21d4aa0b1 Mon Sep 17 00:00:00 2001 From: settings-as-code-release Date: Fri, 11 Sep 2026 23:25:15 -0400 Subject: [PATCH 01/10] style(tests): comments carry only what the code cannot show Every comment under test/ (except test/e2e) and in the colocated section tests judged against the code-standards comment rules: narration of what the test shows deleted, the one input or bug a guard pins kept, stacks of one-fact sentences reshaped as a two-sentence tell plus rows. Three pre-existing false comments corrected (api rate-limit classification, orchestrate repository section tolerance, the flag-pairing home). No code changed. --- src/sections/actions/actions.test.ts | 61 ++-- .../actions_secrets/actions_secrets.test.ts | 41 +-- .../actions_variables.test.ts | 8 +- .../agents_secrets/agents_secrets.test.ts | 10 +- .../agents_variables/agents_variables.test.ts | 10 +- src/sections/branches/branches.test.ts | 55 ++-- .../check_suite_preferences.test.ts | 13 +- .../codespaces_secrets.test.ts | 6 +- .../collaborators/collaborators.test.ts | 2 +- .../custom_properties.test.ts | 5 +- .../dependabot_secrets.test.ts | 6 +- src/sections/deploy_keys/deploy_keys.test.ts | 11 +- .../environments/environments.test.ts | 133 +++------ src/sections/environments/pins.test.ts | 73 ++--- .../interaction_limits.test.ts | 23 +- src/sections/labels/mock.test.ts | 12 +- src/sections/pages/mock.test.ts | 14 +- src/sections/pages/pages.test.ts | 12 +- src/sections/repository/repository.test.ts | 70 ++--- src/sections/rulesets/mock.test.ts | 6 +- src/sections/rulesets/rulesets.test.ts | 31 +- .../secret_scanning_custom_patterns.test.ts | 18 +- src/sections/webhooks/webhooks.test.ts | 18 +- src/sections/workflows/workflows.test.ts | 11 +- test/action/action-yml.test.ts | 20 +- test/action/artifact.test.ts | 7 +- test/action/inputs.test.ts | 5 +- test/action/io.test.ts | 13 +- test/action/run.test.ts | 44 +-- test/architecture/architecture.test.ts | 12 +- test/discovery/central.test.ts | 3 +- test/discovery/discover.test.ts | 5 +- test/discovery/targets.test.ts | 6 +- test/docs/biome-schema.test.ts | 5 +- test/docs/checks-sections-gate.test.ts | 8 +- test/docs/checks-workflow.test.ts | 37 +-- test/docs/ci-workflow.test.ts | 29 +- test/docs/claims.ts | 54 ++-- test/docs/coverage.test.ts | 22 +- test/docs/diagrams.test.ts | 37 +-- test/docs/e2e-nightly-workflow.test.ts | 22 +- test/docs/guides.test.ts | 21 +- test/docs/head-ref.ts | 8 +- test/docs/markdown.ts | 10 +- test/docs/permissions.test.ts | 11 +- test/docs/post-green-workflow.test.ts | 45 +-- test/docs/readme.test.ts | 15 +- test/docs/settings-examples.ts | 4 +- test/docs/version-pins.ts | 13 +- test/engine/check-purity.test.ts | 63 ++-- test/engine/execute.test.ts | 73 ++--- test/engine/layers.test.ts | 22 +- test/engine/orchestrate.test.ts | 51 +--- test/engine/secret-refs.test.ts | 10 +- test/engine/secrets.test.ts | 27 +- test/engine/validate.test.ts | 7 +- test/fixtures/environment-flag-pairing.ts | 17 +- test/flows/multi.test.ts | 118 ++------ test/flows/redact.test.ts | 19 +- test/flows/settings-read.test.ts | 15 +- test/github/api.test.ts | 226 ++++----------- test/github/graphql.test.ts | 47 +-- test/github/repo-visibility.test.ts | 4 - test/github/stub.ts | 14 +- test/io/mask-pair.test.ts | 5 +- test/library/public-surface.test.ts | 7 +- test/mock-api.ts | 8 +- test/package-pins.test.ts | 9 +- test/private.test.ts | 11 +- test/published-schema.test.ts | 57 +--- test/report/artifact-report.test.ts | 6 +- test/report/composer.test.ts | 6 +- test/report/delivery.test.ts | 3 +- test/report/issue-report.test.ts | 35 +-- test/schema-corpus.test.ts | 50 +--- test/schema-slices.test.ts | 19 +- test/scripts/changed-sections.test.ts | 99 ++----- test/scripts/check-endpoint-coverage.test.ts | 6 - test/scripts/fetch-retry.test.ts | 6 +- test/scripts/file-fuzz-issue.test.ts | 32 +-- test/scripts/gen-action-docs.test.ts | 33 +-- test/scripts/gen-docs.test.ts | 43 +-- test/scripts/generated-regions.test.ts | 27 +- test/scripts/graduate-upstream-gaps.test.ts | 31 +- test/scripts/release-pipeline.test.ts | 269 +++++------------- test/scripts/relocated-region.test.ts | 3 - test/scripts/relocated-region.ts | 7 +- test/scripts/schema-descriptions.test.ts | 4 - test/sections/contract.test.ts | 100 ++----- test/sections/docs-registry.test.ts | 68 ++--- test/sections/fragment-fake.ts | 9 +- test/sections/graphql-contract.test.ts | 39 +-- test/sections/graphql-queries.test.ts | 41 +-- test/sections/list-section.test.ts | 8 +- test/sections/loosen.test.ts | 11 +- test/sections/plan-idempotence.test.ts | 41 +-- test/sections/plan-idempotence.ts | 31 +- test/sections/registry.test.ts | 149 ++-------- test/sections/roles.test.ts | 4 +- test/sections/sealed-box.test.ts | 10 +- test/sections/secret-family.ts | 9 +- test/sections/secrets-engine.test.ts | 23 +- test/sections/section-run.ts | 20 +- test/sections/setup-section.test.ts | 24 +- 104 files changed, 788 insertions(+), 2393 deletions(-) diff --git a/src/sections/actions/actions.test.ts b/src/sections/actions/actions.test.ts index a97c31c3c..42eeabced 100644 --- a/src/sections/actions/actions.test.ts +++ b/src/sections/actions/actions.test.ts @@ -9,12 +9,10 @@ import { describeProblem } from "../../problem.js"; import { PermissionDenied } from "../contract/errors.js"; import { grantFor } from "../contract/permissions.js"; import { actionsSection, endpointRouted } from "./index.js"; -// The casts on some plan() inputs below simulate keys GitHub adds: the -// shape passes unknown keys through verbatim, which the static config type -// cannot spell without giving up typo-checking on the known keys. +// The `as ActionsConfig` casts below simulate keys GitHub adds: the shape passes unknown keys through verbatim, which the static config type cannot +// spell without giving up typo-checking on the known keys. import type { ActionsConfig } from "./schema.js"; -/** The verdict's error prose, or null when the document validated. */ function shapeError(doc: Record, sourceLabel: string): string | null { return validateSectionShapes(doc, sourceLabel).match(() => null, describeProblem); } @@ -116,14 +114,11 @@ describe("actions", () => { notes: [], drift: [], }); - // Planning reads each declared key's own endpoint and never writes. expect(roles(api)).toEqual([PERMISSIONS, WORKFLOW, SELECTED, ACCESS]); }); test("a selected policy already live with no allowlist plans only the allowlist PUT", async () => { - // The base permissions match, so no policy PUT is due; the allowlist GET - // still 404s (none exists yet) and only putSelected is planned - which - // is exactly what its drift line promises. + // The allowlist GET 404s (none exists yet) while the policy already matches, so only putSelected is due. const api = new MockApi({ [PERMISSIONS]: { data: { enabled: true, allowed_actions: "selected" } }, }); @@ -179,15 +174,13 @@ describe("actions", () => { }); const result = await plan(api, { selected_actions: { github_owned_allowed: true } }); expect(result.ops[0]?.payload).toEqual({ allowed_actions: "selected", enabled: true }); - // The contradiction is a shape rejection (both modes, before any section - // writes), not a plan()-time throw. + // A shape rejection (both modes, before any section writes), not a plan()-time throw. const error = shapeError( { actions: { allowed_actions: "all", selected_actions: { github_owned_allowed: true } } }, "f.yml", ); expect(error).toContain("actions.selected_actions"); expect(error).toContain('an allowlist only applies under allowed_actions: "selected"'); - // The valid pairing and the inferred form both pass validation. expect( shapeError( { @@ -214,8 +207,6 @@ describe("actions", () => { artifact_and_log_retention: { days: 30 }, cache: { max_cache_retention_days: 3, max_cache_size_gb: 25 }, }); - // Deterministic: one table pass, so retention precedes both cache PUTs - // and the two cache limits go in CACHE_ENDPOINT_BY_KEY order. expect(result.ops).toEqual([ { role: "putRetention", @@ -252,9 +243,8 @@ describe("actions", () => { }); test("the shape rejects unrecognized, null, and scalar cache declarations upfront", () => { - // Inherited names like "constructor" must be caught too: an `in`-based - // check would walk the prototype chain and let them silently no-op. An - // own "__proto__" key (JSON.parse creates one) is unrecognized as well. + // An `in`-based check would walk the prototype chain and let "constructor" silently no-op; an own "__proto__" key (JSON.parse creates one) is + // unrecognized as well. for (const cache of [ { max_cache_size: 25 }, { constructor: 5 }, @@ -312,10 +302,8 @@ describe("actions", () => { }); test("an omitted claim-key list on a custom template, and any list on the default one, are not compared", async () => { - // {use_default: false} with no list is the documented opt-in to the - // ORGANIZATION template, whose keys then appear live; comparing the - // omitted list against them would be permanent false drift. And GitHub - // ignores include_claim_keys under use_default: true. + // {use_default: false} with no list is the documented opt-in to the ORGANIZATION template, whose keys then appear live, so comparing the omitted + // list would be permanent false drift; GitHub ignores include_claim_keys under use_default: true. const custom = new MockApi({ [OIDC]: { data: { use_default: false, include_claim_keys: ["repo", "context"] } }, }); @@ -335,9 +323,7 @@ describe("actions", () => { }); test("a declared use_immutable_subject rides the remainder diff", async () => { - // The flag flips the whole subject format, so a declared false against - // a live true must drift; undeclared, the inherited org/date default - // stays uncompared like every other undeclared key. + // The flag flips the whole subject format; undeclared, the inherited org/date default stays uncompared like every other undeclared key. const api = new MockApi({ [OIDC]: { data: { use_default: false, include_claim_keys: ["repo"], use_immutable_subject: true }, @@ -356,8 +342,7 @@ describe("actions", () => { }); test("the oidc shape rejects quoted booleans upfront", () => { - // A YAML '"false"' is truthy on the wire; both boolean fields must - // fail validation before any section writes. + // A YAML '"false"' is truthy on the wire. for (const bad of [ { use_default: "false" }, { use_default: true, use_immutable_subject: "false" }, @@ -367,10 +352,8 @@ describe("actions", () => { }); test("a denied fork-pr-private read renders the ambiguity denialHint", async () => { - // If GitHub denies this pair on a public repository, this one sentence - // is the whole mitigation - and the mechanism (denialHint on the - // permission branch) has silently broken once before, so pin that a - // denial actually renders it. + // If GitHub denies this pair on a public repository this sentence is the whole mitigation, and denialHint rendering has silently broken once + // before. const api = new MockApi({ [FORK_PRIVATE]: { error: { status: 403, message: "Forbidden", body: "" } }, }); @@ -403,10 +386,8 @@ describe("actions", () => { } expect(thrown).toBeInstanceOf(PermissionDenied); const denied = thrown as PermissionDenied; - // The failing call is a GET, but the advice grades by the SECTION's need - // on the override permission: the OIDC PUT sibling writes with the same - // Actions permission, so read-only advice would cost a second round trip - // (grant read, pass preflight, fail on the write). + // The advice grades by the SECTION's need on the override permission: the OIDC PUT sibling writes with the same Actions permission, so read-only + // advice would cost a second round trip. expect(denied.detail).toContain(grantFor({ repo: ["actions"] })); expect(denied.detail).not.toContain('"Administration"'); }); @@ -440,9 +421,7 @@ describe("actions", () => { ["putForkPrApproval", approval, "applied the fork PR contributor approval policy"], ["putForkPrPrivate", privateRepos, "applied the private-repo fork PR workflow settings"], ]); - // The shape requires all four toggles and every one is compared: with - // every live value flipped, all four must drift - an omitted comparison - // cannot pass here; the passthrough field drifts as unknown to GitHub. + // Every live value is flipped, so an omitted comparison cannot pass here; the passthrough field drifts as unknown to GitHub. expect(result.ops[1]?.drift).toEqual([ "actions.fork_pr_workflows_private_repos.run_workflows_from_fork_pull_requests: true != false", "actions.fork_pr_workflows_private_repos.send_write_tokens_to_workflows: false != true", @@ -456,9 +435,8 @@ describe("actions", () => { }); test("the private-repos shape requires the complete policy and stays loose otherwise", () => { - // GitHub does not document whether an omitted toggle is preserved or - // reset by the PUT, so the shape demands all four booleans (a YAML-quoted - // "true" included) before any section writes. + // GitHub does not document whether an omitted toggle is preserved or reset by the PUT, so the shape demands all four booleans (a YAML-quoted + // "true" included). for (const bad of [ { send_secrets_and_variables: false }, { @@ -489,7 +467,6 @@ describe("actions", () => { }, }).success, ).toBe(true); - // The approval object requires its policy string the same way. expect(actionsSection.shape.safeParse({ fork_pr_contributor_approval: {} }).success).toBe( false, ); @@ -560,9 +537,7 @@ describe("actions", () => { }); test("a routed key's endpoint pair must share a name, and a scalar key must say how it becomes a body", () => { - // Compile-time only: the table wiring is checked at the call, so a GET - // paired with another key's PUT, or an enum-valued key PUT bare, never - // reaches the routing table. + // Compile-time only: a GET paired with another key's PUT, or an enum-valued key PUT bare, never reaches the routing table. const paired: ReturnType> = endpointRouted< "access_level", "Access" diff --git a/src/sections/actions_secrets/actions_secrets.test.ts b/src/sections/actions_secrets/actions_secrets.test.ts index b76de7232..9a77f1729 100644 --- a/src/sections/actions_secrets/actions_secrets.test.ts +++ b/src/sections/actions_secrets/actions_secrets.test.ts @@ -1,9 +1,3 @@ -/** - * actions_secrets tests: existence planning, the one cannot-verify note, the - * keep/delete knob, the sealed PUT per declared secret, and no plaintext - * anywhere but inside the sealed payload the executor sends. - */ - import { describe, expect, test } from "bun:test"; import { executePlan } from "../../../src/engine/execute.js"; import { runForRepo, validateSettingsDoc } from "../../../src/engine/orchestrate.js"; @@ -147,7 +141,6 @@ describe("actions_secrets planning", () => { notes: [CANNOT_VERIFY], drift: [], }); - // Planning reads the list and the sealing key, and writes nothing. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ "GET /repos/o/r/actions/secrets?per_page=100&page=1", "GET /repos/o/r/actions/secrets/public-key", @@ -161,8 +154,7 @@ describe("actions_secrets planning", () => { { name: "A", value: "$A" }, { name: "B", value: "$B" }, ]); - // The references live inside the thunks, and nothing at plan time could - // have resolved one: the resolver is first consulted when a thunk runs. + // Nothing at plan time could have resolved a reference: the resolver is first consulted when a thunk runs. const exec = tools({ $A: "value-a", $B: "value-b" }); const payloads = result.ops.map((op) => typeof op.payload === "function" ? sealedPayload({ payload: op.payload(exec) }) : null, @@ -229,8 +221,7 @@ describe("actions_secrets planning", () => { expect(api.calls).toEqual([]); }); - // orchestrate.ts validates references through engine/secret-refs.ts (tested in - // test/engine/secret-refs.test.ts); the section only extracts and looks up values. + // Reference validation lives in engine/secret-refs.ts (test/engine/secret-refs.test.ts); the section only extracts and looks up values. }); describe("actions_secrets execution", () => { @@ -255,8 +246,6 @@ describe("actions_secrets execution", () => { "/repos/o/r/actions/secrets/EXISTING", "/repos/o/r/actions/secrets/BRAND_NEW", ]); - // The body is exactly {encrypted_value, key_id}, and the sealed value - // unseals back to the resolved plaintext (the client-side crypto proof). for (const [index, expected] of [ [0, "value-a"], [1, "value-b"], @@ -269,9 +258,7 @@ describe("actions_secrets execution", () => { }); test("the sealed PUT recurs on every plan by declaration; deletions and creates converge", async () => { - // No compare is possible (values cannot be read back), and the rewrite is - // what propagates a rotated source value: the proof requires the PUT to - // fire on every pass and everything else to settle. + // No compare is possible (values cannot be read back), and the rewrite is what propagates a rotated source value, so the PUT fires on every pass. const api = liveRepo(["ROTATED", "STALE"]); const { first, second, changes } = await provePlanIdempotent( actionsSecretsSection, @@ -282,8 +269,7 @@ describe("actions_secrets execution", () => { expect(changes).toEqual(['updated secret "ROTATED"', 'DELETED undeclared secret "STALE"']); expect(first.ops.map((op) => op.role)).toEqual(["put", "remove"]); expect(second.ops.map((op) => op.role)).toEqual(["put"]); - // Two executions: the first pass writes and purges, the converged pass - // re-seals the declared secret and nothing else. + // provePlanIdempotent executes the converged plan too, hence the second PUT. expect(api.writes).toEqual([ "PUT /repos/o/r/actions/secrets/ROTATED", "DELETE /repos/o/r/actions/secrets/STALE", @@ -307,9 +293,8 @@ describe("actions_secrets execution", () => { expect(kept.plan.notes.join("\n")).toContain("unrecoverable"); expect(api.mutations()).toEqual([]); - // The purge form: nothing declared means no references, so the engine - // provisions a resolver that refuses every lookup - deletion must never - // touch it, and there is nothing to seal, so no public key is read. + // Nothing declared means no references, so the engine provisions a resolver that refuses every lookup; deletion must never touch it, and nothing + // needs sealing. const api2 = new MockApi({ [LIST]: listOf("STALE") }).allowMutations( "DELETE /repos/o/r/actions/secrets/STALE", ); @@ -342,8 +327,6 @@ describe("actions_secrets execution", () => { expect(rendered).not.toContain(fragment); const put = api.mutations().find((call) => call.method === "PUT"); expect(put?.path).toBe("/repos/o/r/actions/secrets/EDGY"); - // Only the SEALED form travels; it carries no fragment of the plaintext - // and unseals back to it exactly (round-trip fidelity, hostile chars kept). const payload = sealedPayload(put); expect(payload.encrypted_value).not.toContain(fragment); expect(unsealSecretValue(payload.encrypted_value)).toBe(hostile); @@ -385,9 +368,7 @@ describe("actions_secrets execution", () => { }); test("the engine masks every plaintext before the first sealed PUT leaves the client", async () => { - // The plan path's twin of the orchestrator's masking-order proof: the - // apply context (and its masks) exists before any section is planned, - // let alone executed. + // The masks are registered after the read-only preflight and before the first mutation; test/engine holds the orchestrator's twin of this proof. const api = new MockApi({ [LIST]: listOf(), [PUBLIC_KEY]: KEY_ROUTE }).allowMutations( "PUT /repos/o/r/actions/secrets/DEPLOY_TOKEN", ); @@ -444,8 +425,7 @@ describe("actions_secrets sealing key", () => { /no usable \{key_id, key\} pair \(key_id is missing\)/, ); - // An empty key_id is as unusable as a missing one: GitHub requires it in - // the PUT body to route the ciphertext to the right key. + // GitHub requires key_id in the PUT body to route the ciphertext, so an empty one is as unusable as a missing one. const emptyId = new MockApi({ [LIST]: listOf(), [PUBLIC_KEY]: { data: { key_id: "", key: MOCK_SECRETS_PUBLIC_KEY } }, @@ -473,9 +453,8 @@ describe("actions_secrets contract", () => { }); test("a planned operation names a declared write role; only the sealed PUT may go without drift", () => { - // Compile-time only: the plans are never executed. Each rejected shape - // is built first and assigned on one line, so the directive anchors to - // the assignment whichever property the compiler blames. + // Compile-time only. Each rejected shape is built first and assigned on one line, so the @ts-expect-error anchors to the assignment whichever + // property the compiler blames. type Op = PlannedOp; const sealed: Op = { role: "put", diff --git a/src/sections/actions_variables/actions_variables.test.ts b/src/sections/actions_variables/actions_variables.test.ts index 95eb9aa0e..ba11cd2af 100644 --- a/src/sections/actions_variables/actions_variables.test.ts +++ b/src/sections/actions_variables/actions_variables.test.ts @@ -94,7 +94,6 @@ describe("actions_variables", () => { { role: "update", params: { name: "DEPLOY_REGION" }, - // The update sends the value only. payload: { value: "eu-west-1" }, drift: [ 'actions_variables[DEPLOY_REGION].value: declared "eu-west-1" != live "us-east-1"; apply will set the declared value', @@ -103,7 +102,6 @@ describe("actions_variables", () => { }, { role: "create", - // The create sends name + value. payload: { name: "BUILD_MODE", value: "release" }, drift: [ "actions_variables[BUILD_MODE]: missing - declared in the settings file but not on the repo; apply will create it", @@ -122,7 +120,6 @@ describe("actions_variables", () => { notes: [], drift: [], }); - // Planning reads and never writes. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ "GET /repos/o/r/actions/variables?per_page=30&page=1", ]); @@ -209,9 +206,8 @@ describe("actions_variables", () => { }); test("the list request asks for the endpoint's 30-per-page cap", async () => { - // The variables list caps per_page at 30; a 100 would be silently clamped - // and a 30-item first page would wrongly end the walk. The second page - // proves the loop continues past a FULL page of 30. + // The variables list caps per_page at 30; a 100 would be silently clamped and a 30-item first page would wrongly end the walk, so the second page + // proves the loop continues past a full page. const page1 = Array.from({ length: 30 }, (_, i) => ({ name: `VAR_${i}`, value: "x" })); const api = new MockApi({ "GET /repos/o/r/actions/variables?per_page=30&page=1": { diff --git a/src/sections/agents_secrets/agents_secrets.test.ts b/src/sections/agents_secrets/agents_secrets.test.ts index 1cf836625..7d4b326cf 100644 --- a/src/sections/agents_secrets/agents_secrets.test.ts +++ b/src/sections/agents_secrets/agents_secrets.test.ts @@ -1,11 +1,5 @@ -/** - * agents_secrets section tests: one pinSecretFamily() call carrying this - * family's own facts (routes, noun, keep-by-default); the shared skeleton - * and the engine pins live in test/sections/. The factory and engine - * behavior (reconciliation verbs, sealing, the resolver contract, hostile - * values, key validation) stays pinned by the actions_secrets suite over - * the same repoSecretsSection() factory. - */ +// The factory and engine behavior stays pinned by the actions_secrets suite over the same repoSecretsSection() factory; this file pins only this +// family's own facts. import { pinSecretFamily } from "../../../test/sections/secret-family.js"; import { agentsSecretsSection } from "./index.js"; diff --git a/src/sections/agents_variables/agents_variables.test.ts b/src/sections/agents_variables/agents_variables.test.ts index 0cee97baa..52894a518 100644 --- a/src/sections/agents_variables/agents_variables.test.ts +++ b/src/sections/agents_variables/agents_variables.test.ts @@ -1,11 +1,5 @@ -/** - * agents_variables section tests: this section is one repoVariablesSection() - * factory call, and the factory/engine behavior (reconciliation verbs, - * case-insensitive matching, the undeclared knob, pagination, phantom keys) - * is pinned by the actions_variables suite. These tests pin what is THIS - * section's own: its routes, its "Copilot agents variable" noun, and its - * delete-by-default posture. - */ +// The factory and engine behavior is pinned by the actions_variables suite over the same repoVariablesSection() factory; this file pins only this +// section's routes, noun, and delete-by-default posture. import { describe, expect, test } from "bun:test"; import { executePlan } from "../../../src/engine/execute.js"; diff --git a/src/sections/branches/branches.test.ts b/src/sections/branches/branches.test.ts index 6fbd4505f..5d1c98626 100644 --- a/src/sections/branches/branches.test.ts +++ b/src/sections/branches/branches.test.ts @@ -300,8 +300,7 @@ describe("branches", () => { ] as const)( "%s enabled live but omitted is the replacing PUT's only justification; declared, it plans nothing", async (_what, liveProtection, omitting, keyPath, declaring) => { - // The old unconditional PUT reset the setting silently; the plan resets - // it too, and says so. Empty nested lists are nothing to preserve. + // Empty nested lists (dismissal_restrictions) are nothing to preserve, so they earn no line. const api = new MockApi({ [PROTECTION]: { data: liveProtection } }); const result = await plan(api, [{ name: "main", protection: omitting }]); expect(result.ops.map((op) => [op.role, op.drift])).toEqual([ @@ -336,8 +335,7 @@ describe("branches", () => { }); test("a live control whose fields are all defaults is still a setting the replacing PUT would remove", async () => { - // Presence is the setting: reviews are required even with a zero count - // and every flag off. + // Presence is the setting: reviews are required even with a zero count and every flag off. expect( (await plan(liveDefaultReviews(), declared)).ops.map((op) => [op.role, op.drift]), ).toEqual([ @@ -368,9 +366,7 @@ describe("branches", () => { ); test("the GET's own metadata and its second spelling of the status-check list never read as omitted settings", async () => { - // name, enabled, and enforcement_level exist only in the GET shape; the - // checks list mirrors contexts. None of them is a setting the PUT would - // reset, so a converged repository plans nothing. + // name, enabled, and enforcement_level exist only in the GET shape, and checks mirrors contexts; none is a setting the PUT would reset. const api = new MockApi({ [PROTECTION]: { data: { @@ -479,8 +475,7 @@ describe("branches", () => { { name: "main", protection: { enforce_admins: true, required_signatures: declaredValue } }, ]); expect(result).toEqual({ ops: [...ops], notes: [], drift: [] }); - // The sub-endpoint is the toggle's ONLY carrier: the PUT body never - // smuggles the key GitHub would silently drop. + // The sub-endpoint is the toggle's only carrier: the PUT body never smuggles the key GitHub would silently drop. expect(result.ops.some((op) => op.role === "putProtection")).toBe(false); }); @@ -520,17 +515,13 @@ describe("branches", () => { }); test('a quoted "true" fails the shape upfront, with the YAML gotcha named', () => { - // The toggle is typed in the zod shape so document validation rejects it - // before ANY section writes - not a plan-time throw after earlier - // sections already applied. + // Typed in the zod shape so document validation rejects it before any section writes, not as a plan-time throw after earlier sections applied. const parsed = branchesSection.shape.safeParse([ { name: "main", protection: { enforce_admins: true, required_signatures: "true" } }, ]); expect(parsed.success).toBe(false); const messages = parsed.success ? [] : parsed.error.issues.map((issue) => issue.message); expect(messages.some((m) => m.includes("unquoted true or false"))).toBe(true); - // The passthrough survives the typed key: unknown protection fields and - // a proper boolean both validate. expect( branchesSection.shape.safeParse([ { @@ -576,8 +567,7 @@ describe("branches GraphQL-routed keys", () => { ], ], ]); - // The id is late (no rule exists at plan time) and the change line renders - // from the response (the deployment read-back), so both seal at execution. + // The id is late (no rule exists at plan time) and the change line renders from the deployment read-back, so both seal at execution. expect(typeof result.ops[1]?.variables).toBe("function"); expect(typeof result.ops[1]?.change).toBe("function"); expect(api.writes).toEqual([]); @@ -609,7 +599,6 @@ describe("branches GraphQL-routed keys", () => { expect(input.bypassForcePushActorIds).toHaveLength(3); expect(input.requiresDeployments).toBe(true); expect(input.requiredDeploymentEnvironments).toEqual(["prod"]); - // Converged: the re-plan reads the rule the update wrote and finds nothing to do. expect(await plan(api, routedDesired)).toEqual({ ops: [], notes: [], drift: [] }); }); @@ -652,8 +641,6 @@ describe("branches GraphQL-routed keys", () => { const result = await plan(api, [entry]); const execution = await executePlan(result, branchesSection, api, REPO, NO_SECRETS); expect(execution.status).toBe("failed"); - // Every request landed (the failing one included), but only the ones - // before the failed read-back rendered a line. expect([execution.changes, execution.notes, execution.landed]).toEqual([ changesBefore, [], @@ -696,8 +683,7 @@ describe("branches GraphQL-routed keys", () => { "branches[main].protection.required_deployments: the settings file requires deployments to [prod] but the live rule requires [qa]; apply will set the declared list", ], }); - // The actor's node id is an execution-time input: the plan issues no - // lookup (check mode never does), the sealed variables carry the id. + // The actor's node id is an execution-time input: the plan issues no lookup (check mode never does), the sealed variables carry the id. expect(api.calls.filter((c) => c.path.startsWith("BranchProtectionActor"))).toHaveLength(0); const variables = result.ops[0]?.variables; expect(typeof variables).toBe("function"); @@ -768,8 +754,7 @@ describe("branches GraphQL-routed keys", () => { ], ], ]); - // The PUT carries the actor resolution, so a bad actor fails before the - // live protection is replaced; the update's variables seal the ids. + // The PUT carries the actor resolution, so a bad actor fails before the live protection is replaced; the update's variables seal the ids. expect(typeof result.ops[0]?.before).toBe("function"); expect(typeof result.ops[1]?.variables).toBe("function"); @@ -837,8 +822,7 @@ describe("branches GraphQL-routed keys", () => { ], ], ]); - // With no rule id in hand the update looks it up at execution, where the - // still-unreadable view fails the operation by name instead of silently. + // With no rule id in hand the update looks it up at execution, where the still-unreadable view fails the operation by name instead of silently. const execution = await executePlan(result, branchesSection, api, REPO, NO_SECRETS); expect(execution.status).toBe("failed"); expect(String((execution as { error: Error }).error.message)).toMatch( @@ -896,9 +880,8 @@ describe("branches GraphQL-routed keys", () => { }); test("a misspelled actor on a LATER entry fails before an EARLIER entry's write lands", async () => { - // main drifts on the REST half and carries no actors; dev declares the - // bad actor. The section's first operation (main's PUT) resolves every - // planned actor first, so nothing is written for either branch. + // main drifts on the REST half with no actors and dev declares the bad actor; the section's first operation (main's PUT) resolves every planned + // actor first. const api = new MockApi( { [PROTECTION]: { data: { enforce_admins: { enabled: false } } }, @@ -1011,8 +994,7 @@ describe("branches wildcard entries", () => { ]); expect(result.notes).toEqual([]); expect(result.drift).toEqual([]); - // The create needs the repository's node id, an execution-time input: the - // plan issues no lookup for it, the sealed variables carry it. + // The create needs the repository's node id, an execution-time input: the plan issues no lookup for it, the sealed variables carry it. expect(create).toMatchObject({ role: "createRule", describe: 'creating the protection rule "release/*"', @@ -1125,9 +1107,7 @@ describe("branches wildcard entries", () => { }); test("a scalar structured key on a wildcard entry fails the shape, not apply", () => { - // Without this rejection the value passes the looseObject and crashes - // translateWildcardProtection mid-plan with a raw TypeError - a config - // that survives check mode must never blow up on apply. + // Without this rejection the value passes the looseObject and crashes translateWildcardProtection mid-plan with a raw TypeError. for (const bad of [ { required_status_checks: true }, { required_pull_request_reviews: 5 }, @@ -1138,8 +1118,7 @@ describe("branches wildcard entries", () => { const messages = parsed.success ? [] : parsed.error.issues.map((issue) => issue.message); expect(messages.some((m) => m.includes("must be a mapping of its sub-keys"))).toBe(true); } - // The same scalar on a LITERAL entry stays a passthrough (GitHub is the - // authority on the REST payload). + // The same scalar on a LITERAL entry stays a passthrough (GitHub is the authority on the REST payload). expect( branchesSection.shape.safeParse([ { name: "main", protection: { required_status_checks: true } }, @@ -1236,7 +1215,6 @@ describe("branches plan contract", () => { "GRAPHQL UpdateBranchProtectionRule", "GRAPHQL DeleteBranchProtectionRule", ]); - // The undeclared rule's note survives both plans; nothing else does. const note = 'undeclared classic protection rule "legacy/*" exists on the repo - declare it to manage it (this action never deletes undeclared rules)'; expect(first.notes).toEqual([note]); @@ -1270,9 +1248,8 @@ describe("branches plan contract", () => { }); test("a planned operation can only name a declared write role, with the facets its route demands", () => { - // Compile-time only: the plans are never executed. Each rejected shape - // is built first and assigned on one line, so the directive anchors to - // the assignment whichever property the compiler blames. + // Compile-time only. Each rejected shape is built first and assigned on one line, so the @ts-expect-error anchors to the assignment whichever + // property the compiler blames. type Op = PlannedOp; const rest: Op = { role: "sigPost", params: MAIN, drift: ["x"], change: "" }; const mutation: Op = { role: "deleteRule", variables: { input: {} }, drift: ["x"], change: "" }; diff --git a/src/sections/check_suite_preferences/check_suite_preferences.test.ts b/src/sections/check_suite_preferences/check_suite_preferences.test.ts index 7574c0594..7fe6b8bc4 100644 --- a/src/sections/check_suite_preferences/check_suite_preferences.test.ts +++ b/src/sections/check_suite_preferences/check_suite_preferences.test.ts @@ -52,9 +52,8 @@ describe("check_suite_preferences", () => { drift: [], }); expect(api.calls).toEqual([]); - // The change line counts the ECHOED entries: three against two declared - // can only come from GitHub's post-state, so the fallback cannot be what - // rendered it. A shapeless echo falls back to the declared list. + // Three echoed entries against two declared can only come from GitHub's post-state, so the fallback cannot be what rendered it; a shapeless echo + // falls back to the declared list. expect( rendered(result, { preferences: { auto_trigger_checks: [...declared.auto_trigger_checks, { app_id: 1 }] }, @@ -86,8 +85,7 @@ describe("check_suite_preferences", () => { expect(second.ops.map((op) => op.role)).toEqual(["update"]); expect(first.notes).toEqual([note]); expect(second.notes).toEqual([note]); - // Both executions (the proof also runs the converged plan) sent the - // declaration verbatim; nothing was ever read. + // provePlanIdempotent executes the converged plan too, hence two PATCHes. expect(api.calls).toEqual([ { method: "PATCH", path, payload: declared }, { method: "PATCH", path, payload: declared }, @@ -118,9 +116,8 @@ describe("check_suite_preferences", () => { ctx.read.update; // @ts-expect-error nor the raw client ctx.api; - // Compile-time only: the operations are never executed. Each rejected - // shape is built first and assigned on one line, so the directive - // anchors to the assignment whichever property the compiler blames. + // Compile-time only. Each rejected shape is built first and assigned on one line, so the @ts-expect-error anchors to the assignment whichever + // property the compiler blames. type Op = PlannedOp; const rewrite: Op = { role: "update", payload: declared, drift: [], change: "" }; expect(rewrite.drift).toEqual([]); diff --git a/src/sections/codespaces_secrets/codespaces_secrets.test.ts b/src/sections/codespaces_secrets/codespaces_secrets.test.ts index 4c4e2d8e0..ddeaae0cb 100644 --- a/src/sections/codespaces_secrets/codespaces_secrets.test.ts +++ b/src/sections/codespaces_secrets/codespaces_secrets.test.ts @@ -1,8 +1,4 @@ -/** - * codespaces_secrets section tests: one pinSecretFamily() call carrying this - * family's own facts (routes, noun, keep-by-default); the shared skeleton - * and the engine pins live in test/sections/. - */ +// This family's own facts only; the shared skeleton and the engine pins live in test/sections/. import { pinSecretFamily } from "../../../test/sections/secret-family.js"; import { codespacesSecretsSection } from "./index.js"; diff --git a/src/sections/collaborators/collaborators.test.ts b/src/sections/collaborators/collaborators.test.ts index 8255c1699..2ed5072db 100644 --- a/src/sections/collaborators/collaborators.test.ts +++ b/src/sections/collaborators/collaborators.test.ts @@ -69,7 +69,7 @@ describe("collaborators", () => { notes: [], drift: [], }); - // The owner "O" is never removed, and planning only reads. + // The owner "O" is never removed. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([LIST, INVITATIONS]); }); diff --git a/src/sections/custom_properties/custom_properties.test.ts b/src/sections/custom_properties/custom_properties.test.ts index 9331834f7..9a43de00e 100644 --- a/src/sections/custom_properties/custom_properties.test.ts +++ b/src/sections/custom_properties/custom_properties.test.ts @@ -25,8 +25,8 @@ function changeLines(op: SectionPlan["ops"][number]): readonly string[] { } /** - * The derived fake refuses the org probe: the dispatcher resolves GET /orgs/{org} to teams, the - * first section declaring it. The probe only reads the seeded org, so it is answered here. + * The derived fake refuses the org probe (the dispatcher resolves GET /orgs/{org} to teams, the first section declaring it), so it is answered here + * from the seeded org. */ function orgFake(values: Array<{ property_name: string; value: unknown }>) { const fake = fragmentFake(customPropertiesSection, customPropertiesMockHandlers, { @@ -133,7 +133,6 @@ describe("custom_properties", () => { 'custom_properties[compliance]: declared ["soc2","hipaa"] != live ["soc2"]; apply will set the declared value', ], ]); - // "pilot" is live, declared nowhere, and kept; "compliance" is managed. expect(result.notes).toEqual([ 'custom property "pilot" is set on the repo but not declared in the settings file; kept under "_undeclared: keep" - add it to the settings file to manage it, or set "_undeclared: delete" to have apply UNSET it', 'custom property "tier" is set on the repo but not declared in the settings file; kept under "_undeclared: keep" - add it to the settings file to manage it, or set "_undeclared: delete" to have apply UNSET it', diff --git a/src/sections/dependabot_secrets/dependabot_secrets.test.ts b/src/sections/dependabot_secrets/dependabot_secrets.test.ts index 3eac87190..ce31d7519 100644 --- a/src/sections/dependabot_secrets/dependabot_secrets.test.ts +++ b/src/sections/dependabot_secrets/dependabot_secrets.test.ts @@ -1,8 +1,4 @@ -/** - * dependabot_secrets section tests: one pinSecretFamily() call carrying this - * family's own facts (routes, noun, keep-by-default); the shared skeleton - * and the engine pins live in test/sections/. - */ +// This family's own facts only; the shared skeleton and the engine pins live in test/sections/. import { pinSecretFamily } from "../../../test/sections/secret-family.js"; import { dependabotSecretsSection } from "./index.js"; diff --git a/src/sections/deploy_keys/deploy_keys.test.ts b/src/sections/deploy_keys/deploy_keys.test.ts index 1158f66ae..b28aa6f34 100644 --- a/src/sections/deploy_keys/deploy_keys.test.ts +++ b/src/sections/deploy_keys/deploy_keys.test.ts @@ -25,8 +25,6 @@ describe("normalizeKeyMaterial", () => { expect(normalizeKeyMaterial("ssh-ed25519 AAAAC3blob deploy@host")).toBe( "ssh-ed25519 AAAAC3blob", ); - // A multi-word comment is stripped whole, and surrounding whitespace is - // irrelevant to the compared material. expect(normalizeKeyMaterial(" ssh-rsa AAAAB3blob a b c ")).toBe("ssh-rsa AAAAB3blob"); }); @@ -231,7 +229,6 @@ describe("deploy_keys reconcile", () => { { title: "deploy-bot", key: BOT_KEY, read_only: false }, ]); expect(forced.ops.map((op) => op.role)).toEqual(["remove", "create"]); - // The control: the same live toggle, undeclared, is not drift. const ignored = await plan(new MockApi({ [LIST]: { data: live } }), [ { title: "deploy-bot", key: BOT_KEY }, ]); @@ -239,9 +236,8 @@ describe("deploy_keys reconcile", () => { }); test('a declared passthrough field named "material" earns the phantom-key note, diffed against the RAW api body', async () => { - // The normalized material replaces the live `key` in place and never - // lands under another name, so a user field called "material" reads as - // absent live (phantom), not as a synthetic field it would match. + // The normalized material replaces the live `key` in place and never lands under another name, so a user field called "material" reads as absent + // live rather than matching a synthetic field. const api = new MockApi({ [LIST]: { data: [liveKey(10, "deploy-bot", BOT_KEY)] } }); const result = await plan(api, [ { title: "deploy-bot", key: BOT_KEY, material: "whatever" } as never, @@ -340,8 +336,7 @@ describe("deploy_keys convergence", () => { "DELETE /repos/o/r/keys/20", ]); expect(second).toEqual({ ops: [], notes: [], drift: [] }); - // The mock stores comment-free material the way GitHub does; the rotated - // key kept its live read_only through the recreate. + // The mock stores comment-free material the way GitHub does; the rotated key kept its live read_only through the recreate. expect(api.state.deploy_keys.map((k) => [k.title, k.key, k.read_only])).toEqual([ ["deploy-bot", BOT_KEY, true], ["mirror-pull", MIRROR_KEY, false], diff --git a/src/sections/environments/environments.test.ts b/src/sections/environments/environments.test.ts index 625616c3e..ff1645198 100644 --- a/src/sections/environments/environments.test.ts +++ b/src/sections/environments/environments.test.ts @@ -70,7 +70,6 @@ describe("environments plan", () => { }, ]; const planned = await plan(api, declared); - // Planning reads and never writes, whatever the fake would accept. expect(api.mutations()).toEqual([]); expect(planned.ops.map((op) => op.role)).toEqual([ "update", @@ -98,9 +97,7 @@ describe("environments plan", () => { 'updated variable "UPD" in environment "prod"', 'DELETED undeclared variable "GONE" from environment "prod"', ]); - // The strip builds a fresh object: the caller's entry keeps its - // variables, so the duplicate pre-pass (which reads env.variables - // across all entries) can never observe a mutated declaration. + // The strip builds a fresh object, so the duplicate pre-pass (which reads env.variables across all entries) never sees a mutated declaration. expect(declared[0]?.variables).toEqual([ { name: "NEW", value: "v1" }, { name: "UPD", value: "v2" }, @@ -115,7 +112,6 @@ describe("environments plan", () => { }); const planned = await plan(api, [{ name: "prod", wait_timer: 5 }]); expect(planned).toEqual({ ops: [], notes: [], drift: [] }); - // The variables endpoints are never contacted, not even the list read. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ "GET /repos/o/r/environments/prod", ]); @@ -222,8 +218,7 @@ describe("environments variables check mode", () => { const result = await check(api, [ { name: "prod", wait_timer: 5, variables: [{ name: "A", value: "1" }] }, ]); - // Exactly the nested lines: a variables key leaking into subsetDiff would - // add an "environments[prod].variables: declared ..." line here. + // A variables key leaking into subsetDiff would add an "environments[prod].variables: declared ..." line here. expect(result.drift).toEqual([ 'environments[prod].variables[A].value: declared "1" != live "2"; apply will set the declared value', "environments[prod].variables[B]: undeclared - not in the settings file, so apply will DELETE it; add it to the settings file to keep it", @@ -252,7 +247,6 @@ describe("environments variables case-insensitive matching", () => { const result = await apply(api, [ { name: "prod", variables: [{ name: "deploy_region", value: "same" }] }, ]); - // Matched despite the case difference: no create, no update, no delete. expect(result.changes).toEqual([]); }); @@ -339,23 +333,18 @@ describe("environments variables shape", () => { }, ]).success, ).toBe(true); - // Loose like the repository actions_variables entries: an extra field - // rides the POST/PATCH verbatim, so a field GitHub ships tomorrow can - // be declared the day it appears (the passthrough-first tenet). + // An extra entry field rides the POST/PATCH verbatim, so a field GitHub ships tomorrow can be declared the day it appears. expect( shape.safeParse([{ name: "prod", variables: [{ name: "A", value: "1", future: "x" }] }]) .success, ).toBe(true); - // The WRAPPER stays strict: its keys are this action's own vocabulary. + // The wrapper stays strict: its keys are this action's own vocabulary. expect( shape.safeParse([{ name: "prod", variables: { entires: [], entries: [] } }]).success, ).toBe(false); }); test("an extra entry field rides the POST and PATCH verbatim, with a phantom note", async () => { - // The passthrough is behavioral, not just a parse rule: the field - // reaches the wire on create AND update, and a field GitHub does not - // echo back earns the phantom note instead of eternal silent drift. const api = new MockApi({ "GET /repos/o/r/environments/prod": liveEnv("prod"), [VARIABLES_LIST]: variablesBody([{ name: "UPD", value: "old" }]), @@ -381,9 +370,6 @@ describe("environments variables shape", () => { value: "new", extra_field: "y", }); - // The fake echoes no extra_field back, so the update notes the phantom - // rather than pretending it converged - in check mode too, beside the - // drift line that names the field the response lacks. const phantom = 'environments[prod].variables[UPD]: declared key(s) "extra_field" do not exist on the live variable, so if GitHub ignores them this update will re-run on every apply without converging. Fix the key name, or remove it from the settings file'; expect(result.notes).toEqual([phantom]); @@ -422,9 +408,8 @@ function secretsBody(names: string[]) { describe("environments nested secrets apply mode", () => { test("same-named secrets in sibling environments seal each environment's OWN value, with the key read after the PUT", async () => { - // A lookup keyed by secret name alone would seal one value into both - // scopes; staging's key is readable only after its PUT, so the thunk reads - // it then, never at plan time. + // A lookup keyed by secret name alone would seal one value into both scopes; staging's key is readable only after its PUT, so the thunk reads it + // then, never at plan time. await mockSodiumReady(); const api = new MockApi({ "PUT /repos/o/r/environments/staging": { data: { name: "staging" } }, @@ -441,7 +426,6 @@ describe("environments nested secrets apply mode", () => { { name: "prod", secrets: [{ name: "DEPLOY_TOKEN", value: "$PRD" }] }, ]; const planned = await plan(api, declared); - // Check mode never touches the sealing key (nor the missing environment's secrets list). expect(api.calls.some((c) => c.path.endsWith("/public-key"))).toBe(false); expect(api.calls.some((c) => c.path.startsWith("/repos/o/r/environments/staging/"))).toBe( false, @@ -469,10 +453,8 @@ describe("environments nested secrets apply mode", () => { unsealSecretValue((c.payload as { encrypted_value: string }).encrypted_value), ); expect(unsealed).toEqual(["staging-plaintext", "prod-plaintext"]); - // Each scope sealed against ITS environment's key_id. expect(puts.map((c) => (c.payload as { key_id: string }).key_id)).toEqual(["k-stg", "k-prod"]); - // Change lines place every write in its environment (verb from the - // per-environment listing: staging creates, prod updates). + // The verb comes from the per-environment listing: staging has no DEPLOY_TOKEN, prod does. expect(result.changes).toEqual([ 'applied environment "staging"', 'created secret "DEPLOY_TOKEN" in environment "staging"', @@ -514,7 +496,6 @@ describe("environments nested secrets apply mode", () => { { name: "prod", secrets: { _undeclared: "delete", entries: [] } }, ]); expect(deleted.changes).toEqual(['DELETED undeclared secret "LEGACY" in environment "prod"']); - // Nothing declared, so no resolver was needed and no public key fetched. expect(api2.calls.some((c) => c.path.endsWith("/public-key"))).toBe(false); }); @@ -569,7 +550,6 @@ describe("environments nested secrets check mode", () => { expect(cannotVerify).toHaveLength(1); expect(cannotVerify[0]).toContain("prod environment secret values"); expect(api.mutations()).toEqual([]); - // Check mode never touches the sealing key. expect(api.calls.some((c) => c.path.endsWith("/public-key"))).toBe(false); }); }); @@ -601,13 +581,11 @@ describe("environments nested secrets validation and shape", () => { { name: "prod", secrets: { _undeclared: "delete", entries: [{ name: "A", value: "$A" }] } }, ]).success, ).toBe(true); - // An extra entry key has no destination (the PUT body is the sealed - // value alone), so it is rejected rather than silently doing nothing. + // An extra entry key has no destination (the PUT body is the sealed value alone), so it is rejected rather than silently doing nothing. expect( shape.safeParse([{ name: "prod", secrets: [{ name: "A", value: "$A", typo: 1 }] }]).success, ).toBe(false); - // The misplacement pin: a singular `secret` would ride the environment - // PUT verbatim and configure nothing. + // A singular `secret` would ride the environment PUT verbatim and configure nothing. const misplaced = shape.safeParse([{ name: "prod", secret: [{ name: "A", value: "$A" }] }]); expect(misplaced.success).toBe(false); expect(JSON.stringify(misplaced.error?.issues)).toContain( @@ -616,8 +594,7 @@ describe("environments nested secrets validation and shape", () => { }); test("secretValues walks every entry's secrets list and survives malformed containers", () => { - // The double cast feeds secretValues a PRE-VALIDATION document slice on - // purpose: its contract is defensiveness against any merged value. + // The double cast feeds secretValues a pre-validation document slice on purpose: its contract is defensiveness against any merged value. const values = environmentsSection.secretValues?.([ { name: "a", secrets: [{ name: "X", value: "$X" }] }, { name: "b", secrets: { entries: [{ name: "Y", value: "$Y" }] } }, @@ -687,8 +664,7 @@ describe("environments deployment branch policies apply mode", () => { "POST /repos/o/r/environments/prod/deployment-branch-policies", "DELETE /repos/o/r/environments/prod/deployment-branch-policies/42", ]); - // The recreate carries the declared type; the plain create omits it (the - // upstream default "branch" applies). + // The plain create omits type (the upstream default "branch" applies); the recreate carries the declared one. const posts = api.calls.filter((c) => c.method === "POST"); expect(posts[0]?.payload).toEqual({ name: "release/*" }); expect(posts[1]?.payload).toEqual({ name: "v*", type: "tag" }); @@ -701,8 +677,7 @@ describe("environments deployment branch policies apply mode", () => { }); test("a missing environment plans its patterns as creates without listing them", async () => { - // The pattern routes 404 until the PUT lands, so the plan reads nothing - // and the declared patterns follow the PUT as creates. + // The pattern routes 404 until the PUT lands, so the plan reads nothing and the declared patterns follow the PUT as creates. const api = new MockApi({ "PUT /repos/o/r/environments/prod": { data: { name: "prod" } }, }).allowMutations("POST /repos/o/r/environments/prod/deployment-branch-policies"); @@ -719,8 +694,7 @@ describe("environments deployment branch policies apply mode", () => { test("a matching live pattern (type defaulted to branch) is a no-op", async () => { const api = new MockApi({ "GET /repos/o/r/environments/prod": liveProdWithFlag(true), - // The spec marks every field optional; a live policy without a type - // reads as the upstream default "branch". + // The spec marks every field optional; a live policy without a type reads as the upstream default "branch". [POLICIES_LIST]: policiesBody([{ id: 41, name: "release/*" }]), }); const result = await apply(api, [envWithPolicies([{ name: "release/*" }])]); @@ -728,10 +702,8 @@ describe("environments deployment branch policies apply mode", () => { }); test("a live policy without a name fails loudly instead of being silently skipped", async () => { - // A nameless policy has no identity to reconcile by; dropping it would - // let the default delete policy neither remove nor note it, and check - // could report falsely clean. The spec marks the field optional, so the - // extraction fails as a contract violation naming the endpoint. + // A nameless policy has no identity to reconcile by; skipping it would let the default delete policy neither remove nor note it, so check could + // report falsely clean. const api = new MockApi({ "GET /repos/o/r/environments/prod": liveProdWithFlag(true), [POLICIES_LIST]: policiesBody([{ id: 41, type: "branch" }]), @@ -780,9 +752,7 @@ describe("environments deployment branch policies check mode", () => { "GET /repos/o/r/environments/prod": liveProdWithFlag(false), }); const result = await check(api, [envWithPolicies([{ name: "release/*" }])]); - // The flag drift itself comes from the environment subsetDiff; the - // pattern follows as a create whose line claims nothing about what the - // flag hides. + // The flag drift comes from the environment subsetDiff; the pattern's line claims nothing about what the flag hides. expect(result.drift).toEqual([ "environments[prod].deployment_branch_policy.protected_branches: false != true", "environments[prod].deployment_branch_policy.custom_branch_policies: true != false", @@ -797,12 +767,9 @@ describe("environments deployment branch policies check mode", () => { describe("environments deployment branch policies validation and shape", () => { test("the flag pairing is a SHAPE rule: declaring the list without custom_branch_policies: true fails validation", () => { - // In the shape, not the plan() hook, on purpose: upfront document - // validation rejects the document in both modes before ANY section - // writes (the apply-mode preflight swallows non-permission hook errors, - // so a hook check would fire only after earlier sections wrote). The - // fixtures are the SHARED set the published-schema test also runs, so - // the zod refinement and the schema's if/then face the same cases. + // A shape rule, not a plan() hook, so upfront validation rejects the document in both modes before any section writes (the apply-mode preflight + // swallows non-permission hook errors). + // The fixtures are the set the published-schema test also runs, so the zod refinement and the schema's if/then face the same cases. const shape = environmentsSection.shape; for (const { name, entry, valid } of FLAG_PAIRING_FIXTURES) { const parsed = shape.safeParse([entry]); @@ -815,8 +782,7 @@ describe("environments deployment branch policies validation and shape", () => { `the "${FIXTURE_ENV_NAME}" entry declares deployment_branch_policies`, ); expect(messages).toContain("custom_branch_policies: true"); - // The issue points at the offending key, so the document-validation - // error names environments[N].deployment_branch_policies. + // The issue points at the offending key, so the document-validation error names environments[N].deployment_branch_policies. const paths = (parsed.error?.issues ?? []).map((issue) => issue.path.join(".")); expect(paths).toContain("0.deployment_branch_policies"); } @@ -912,13 +878,9 @@ describe("environments deployment protection rules apply mode", () => { }, ]; await plan(api, declared); - // The apps listing is an apply-time resolver; planning never reads it. expect(api.calls.some((c) => c.path.includes("/apps"))).toBe(false); const result = await apply(api, declared); - // The nested key never reaches the PUT body. expect(api.calls.find((c) => c.method === "PUT")?.payload).toEqual({ wait_timer: 5 }); - // One apps fetch, after the environment PUT; one create, resolved to the - // App's integration id; region-guard already enabled, so no second POST. const order = api.calls.map((c) => `${c.method} ${c.path}`); expect(order.filter((c) => c.includes("/apps"))).toHaveLength(1); expect(order.indexOf("PUT /repos/o/r/environments/prod")).toBeLessThan( @@ -927,7 +889,6 @@ describe("environments deployment protection rules apply mode", () => { const posts = api.calls.filter((c) => c.method === "POST"); expect(posts).toHaveLength(1); expect(posts[0]?.payload).toEqual({ integration_id: 3515 }); - // The undeclared change-window rule is KEPT (the default): a note, no DELETE. expect(api.calls.some((c) => c.method === "DELETE")).toBe(false); expect(result.notes.join("\n")).toContain( 'deployment protection rule "change-window" is enabled on environment "prod" but is not declared', @@ -979,9 +940,8 @@ describe("environments deployment protection rules apply mode", () => { apply(api, [ { name: "prod", - // The resolvable deploy-gate entry comes FIRST: every missing slug - // resolves before the first POST, so the unknown sibling aborts the - // whole list and the environment is never half-reconciled. + // deploy-gate comes first on purpose: every missing slug resolves before the first POST, so the unknown sibling aborts before anything is + // enabled. deployment_protection_rules: [{ app: "deploy-gate" }, { app: "not-installed" }], }, ]), @@ -990,13 +950,11 @@ describe("environments deployment protection rules apply mode", () => { 'environment "prod" (the available Apps are "deploy-gate", "region-guard"). Install the ' + "GitHub App providing the rule on this repository, or declare one of the available slugs", ); - // Nothing was enabled, not even the resolvable entry. expect(api.calls.some((c) => c.method === "POST")).toBe(false); }); test("an EMPTY apps listing says no Apps are available at all", async () => { - // A real user state, not a contract break: no protection-rule App is - // installed on the repository, so there is nothing to list in the error. + // A real user state, not a contract break: no protection-rule App is installed, so the error lists nothing. const api = new MockApi({ "GET /repos/o/r/environments/prod": liveEnv("prod"), [RULES_LIST]: rulesBody([]), @@ -1037,8 +995,7 @@ describe("environments deployment protection rules apply mode", () => { }); test("a PRESENT non-array envelope value is a loud contract violation, never an empty list", async () => { - // null is present-but-not-a-list too: the spec types the key as a plain - // array, so only a genuinely ABSENT key may read as empty. + // null is present-but-not-a-list too: the spec types the key as a plain array, so only an absent key may read as empty. for (const garbage of ["garbage", null]) { const api = new MockApi({ "GET /repos/o/r/environments/prod": liveEnv("prod"), @@ -1053,8 +1010,7 @@ describe("environments deployment protection rules apply mode", () => { }); test("a live rule with a non-numeric id fails loudly before any disable", async () => { - // A null or string id would otherwise serialize into the DELETE path - // (".../deployment_protection_rules/null") and address nothing. + // A null or string id would otherwise serialize into the DELETE path (".../deployment_protection_rules/null") and address nothing. for (const id of [null, "41"]) { const api = new MockApi({ "GET /repos/o/r/environments/prod": liveEnv("prod"), @@ -1072,8 +1028,7 @@ describe("environments deployment protection rules apply mode", () => { }); test("a live rule reported as disabled does not satisfy its declared gate", async () => { - // The endpoint documents enabled rules only, so this is a belt over the - // contract: a declared gate whose live rule says enabled: false must be + // The endpoint documents enabled rules only, so this is a belt over the contract: a declared gate whose live rule says enabled: false is // re-enabled, never read as clean. const api = new MockApi({ "GET /repos/o/r/environments/prod": liveEnv("prod"), @@ -1089,10 +1044,8 @@ describe("environments deployment protection rules apply mode", () => { }); test("a disabled undeclared rule is not an active gate: neither noted nor disabled", async () => { - // The other half of the enabled-false skip: under _undeclared: delete the - // goal is "no undeclared gate is on", which a disabled rule already - // satisfies - and a DELETE aimed at a disabled id would likely 404 - // mid-apply for a no-op. + // Under _undeclared: delete the goal is "no undeclared gate is on", which a disabled rule already satisfies; a DELETE aimed at a disabled id + // would likely 404 mid-apply for a no-op. const api = new MockApi({ "GET /repos/o/r/environments/prod": liveEnv("prod"), [RULES_LIST]: rulesBody([{ ...liveRule(41, "change-window"), enabled: false }]), @@ -1117,7 +1070,6 @@ describe("environments deployment protection rules check mode", () => { "environments[prod].deployment_protection_rules[deploy-gate]: missing - declared in the settings file but not enabled on the environment; apply will enable it if the App is available to this environment", ]); expect(kept.notes.join("\n")).toContain('deployment protection rule "change-window"'); - // The apps listing is an apply-time resolver; check mode never reads it. expect(api.calls.some((c) => c.path.includes("/apps"))).toBe(false); expect(api.mutations()).toEqual([]); @@ -1135,9 +1087,7 @@ describe("environments deployment protection rules check mode", () => { }); describe("environments missing-environment planning across the nested families", () => { - // One skeleton for the four families: the missingNote, the sub-resource - // path, and the create's drift line vary. The empty MockApi 404s the - // environment GET, so every family plans creates without a sub-resource read. + // The empty MockApi 404s the environment GET, so every family plans creates without a sub-resource read. test.each([ [ "variables", @@ -1215,8 +1165,7 @@ describe("environments deployment protection rules validation and shape", () => }, ]).success, ).toBe(true); - // An extra entry key has no destination (the enable POST sends only the - // resolved integration_id), so it is rejected rather than silently doing + // An extra entry key has no destination (the enable POST sends only the resolved integration_id), so it is rejected rather than silently doing // nothing. expect( shape.safeParse([ @@ -1231,10 +1180,8 @@ describe("environments deployment protection rules validation and shape", () => }); test("a custom-rule protection_rules entry flattens without leaking keys", () => { - // The environment GET surfaces an enabled custom rule as the spec's third - // protection_rules variant ({id, node_id, type}); flattenEnvironment's - // generic branch filters exactly those keys, so the entry adds nothing to - // the flattened object and can never produce false environment drift. + // The environment GET surfaces an enabled custom rule as the spec's third protection_rules variant ({id, node_id, type}); flattenEnvironment's + // generic branch filters exactly those keys, so the entry can never produce false environment drift. const flattened = flattenEnvironment({ name: "prod", protection_rules: [{ id: 41, node_id: "DPR_41", type: "deploy-gate" }], @@ -1246,8 +1193,7 @@ describe("environments deployment protection rules validation and shape", () => // --- Convergence over the e2e mock's own handlers ------------------------------ /** - * A stateful GithubClient over the section's e2e mock fragment and seeded - * MockState, so the idempotence proof runs against the scenarios' own model. + * A stateful GithubClient over the section's e2e mock fragment and seeded MockState, so the idempotence proof runs against the scenarios' own model. */ function liveRepo(liveState: LiveState): GithubClient & { writes: string[] } { const state = buildState(liveState, "org", REPO.slug); @@ -1311,9 +1257,8 @@ describe("environments convergence", () => { const secretEnv = { $PRD: "prod-plaintext", $NEW: "new-plaintext" }; test("executing the plan converges: the re-plan over applied state carries only the sealed secret PUTs", async () => { - // Every family at once, against the mock's own state: an existing - // environment with settings drift and every nested knob diverging, plus - // a missing pinned environment whose node id must come from its PUT. + // Every family at once: an existing environment with settings drift and every nested knob diverging, plus a missing pinned environment whose node + // id must come from its PUT. await mockSodiumReady(); const api = liveRepo({ environments: { @@ -1438,9 +1383,8 @@ describe("environments convergence", () => { }); test("patterns hidden behind a flag that is off reconcile on the run after the one that sets it", async () => { - // The mock keeps patterns behind an off flag (the list route 404s): the - // first apply sets the flag and creates the declared ones (the hidden - // same-name answers 303), the next apply converges on what was revealed. + // The mock keeps patterns behind an off flag (the list route 404s): the first apply sets the flag and creates the declared ones (the hidden + // same-name answers 303), the next converges on what was revealed. const api = liveRepo({ environments: { prod: { @@ -1482,9 +1426,6 @@ describe("environments convergence", () => { }); test("a secret created alongside its environment is rewritten as an update by the next plan", async () => { - // The first pass creates the environment and its secret (key read after - // the PUT); the second plans exactly the recurring PUT, now an update - // with no drift. await mockSodiumReady(); const api = liveRepo({}); const desired = [{ name: "staging", secrets: [{ name: "NEW", value: "$NEW" }] }]; diff --git a/src/sections/environments/pins.test.ts b/src/sections/environments/pins.test.ts index 37591e598..11ca0d35d 100644 --- a/src/sections/environments/pins.test.ts +++ b/src/sections/environments/pins.test.ts @@ -1,8 +1,3 @@ -/** - * Unit tests for the routed `pinned` scalar: the GraphQL pin, unpin, and - * reorder plans over the pins connection, check-mode drift, and the schema cap. - */ - import { describe, expect, test } from "bun:test"; import { MockApi } from "../../../test/mock-api.js"; import { NO_SECRETS, REPO, sectionRunners } from "../../../test/sections/section-run.js"; @@ -13,9 +8,8 @@ import { environmentsSection } from "./index.js"; const { plan, check, apply } = sectionRunners(environmentsSection); /** - * A pins-connection body. `pins` are either names (contiguous positions - * 1..N) or {name, position} pairs for the hole-y layouts live GitHub - * produces after an unpin. + * A pins-connection body; `pins` are names at contiguous positions 1..N, or {name, position} pairs for the hole-y layouts live GitHub produces after + * an unpin. */ function pinsBody(pins: Array) { return { @@ -49,8 +43,7 @@ function graphqlWrites(api: MockApi) { describe("environments pinned apply mode", () => { test("pinned never reaches the PUT body, and a created environment's pin addresses the node id its PUT answered with", async () => { - // The environment does not exist yet, so its node id is known only once - // the PUT has run: the PUT's capture hands it to the pin mutation. + // The environment does not exist yet, so its node id is known only once the PUT has run: the PUT's capture hands it to the pin mutation. const api = new MockApi({ "PUT /repos/o/r/environments/prod": envBody("prod"), "GRAPHQL EnvironmentPins": pinsBody([]), @@ -87,11 +80,8 @@ describe("environments pinned apply mode", () => { }); test("minimal mutations in cap-safe order: unpin, then pin, then leftward reorders", async () => { - // Live [c, b]; declared order pins a then b, and c declares pinned: - // false. The unpin runs FIRST (a swap can never transiently exceed the - // cap), the missing a is pinned to the tail, and one reorder pulls a - // left to position 1 - b then already sits at position 2, so no second - // reorder is issued. + // Live [c, b]; declared pins a then b, c pinned: false. The unpin runs first (a swap can never transiently exceed the cap); after a is pinned to + // the tail, one reorder pulls it to 1 and b already sits at 2. const api = new MockApi({ "PUT /repos/o/r/environments/a": envBody("a"), "GET /repos/o/r/environments/b": envBody("b"), @@ -130,10 +120,8 @@ describe("environments pinned apply mode", () => { }); test("hole-y live positions in the right order are converged: rank, not literal numbers", async () => { - // Verified live behavior: unpinning leaves a hole (positions 1 and 3 - // with nothing at 2), and re-pins append via a monotonic counter - so a - // list whose RANK order matches the declaration must read converged, - // never as position drift. + // Verified live: unpinning leaves a hole (positions 1 and 3, nothing at 2) and re-pins append via a monotonic counter, so rank order, not the + // literal numbers, is what converges. const api = new MockApi({ "GET /repos/o/r/environments/a": envBody("a"), "GET /repos/o/r/environments/b": envBody("b"), @@ -150,9 +138,7 @@ describe("environments pinned apply mode", () => { }); test("two fresh pins land in declaration order with zero reorders (tail appends)", async () => { - // Pins append at the tail (verified live behavior), so pinning a then b - // onto an empty list already realizes the declared order - the plan - // must not emit compensating reorders. + // Pins append at the tail (verified live), so pinning a then b onto an empty list already realizes the declared order. const api = new MockApi({ "PUT /repos/o/r/environments/a": envBody("a"), "PUT /repos/o/r/environments/b": envBody("b"), @@ -169,11 +155,8 @@ describe("environments pinned apply mode", () => { }); test("live pins nobody declared count toward the cap: overflow fails BEFORE any mutation", async () => { - // The shape's upfront cap sees only declared entries; ten live undeclared - // pins (which the section never unpins) plus one declared pin overflow - // GitHub's cap, and discovering that on the pin mutation would leave the - // list half-applied. The first pin mutation's thunk refuses before its - // request leaves; the environment PUT before it has landed. + // The shape's upfront cap sees only declared entries; ten live undeclared pins (never unpinned) plus one declared overflow GitHub's cap, so the + // first pin thunk refuses before its request leaves, after the environment PUT landed. const api = new MockApi({ "PUT /repos/o/r/environments/prod": envBody("prod"), "GRAPHQL EnvironmentPins": pinsBody([ @@ -190,8 +173,7 @@ describe("environments pinned apply mode", () => { ]), }).allowMutations("GRAPHQL PinEnvironment"); const planned = await plan(api, [{ name: "prod", pinned: true }]); - // The note sits beside the drift in both modes (u1 also earns the - // interleaving note: it leads the list the declared pin should lead). + // u1 also earns the interleaving note: it leads the list the declared pin should lead. expect(planned.notes).toEqual([ 'pinned environment(s) "u1" have no pinned declaration in the settings file; they stay pinned (only a pinned: false entry unpins) and apply moves them after the declared pins', "apply will fail: pinning the 1 declared environment(s) not yet pinned would leave 11 " + @@ -209,10 +191,8 @@ describe("environments pinned apply mode", () => { }); test("a raced-full pinned list surfaces GitHub's cap rejection on the pin mutation", async () => { - // Nine live pins pass the overflow gate (9 + 1 = 10), but a pin raced in - // between the read and the mutation makes GitHub reject with - // UNPROCESSABLE - the belt under the gate, classified like any other - // rejected request and naming the operation. + // Nine live pins pass the overflow gate (9 + 1 = 10); a pin raced in between the read and the mutation makes GitHub reject with UNPROCESSABLE, + // the belt under the gate. const api = new MockApi({ "PUT /repos/o/r/environments/prod": envBody("prod"), "GRAPHQL EnvironmentPins": pinsBody(["u1", "u2", "u3", "u4", "u5", "u6", "u7", "u8", "u9"]), @@ -231,8 +211,7 @@ describe("environments pinned apply mode", () => { }); test("a PUT response without a node_id fails that operation, records no line, and fires no pin", async () => { - // The capture extracts the id synchronously as the PUT lands, so the - // failure is the PUT's own (no change line) and the pin never runs. + // The capture extracts the id synchronously as the PUT lands, so the failure is the PUT's own (no change line) and the pin never runs. const api = new MockApi({ "PUT /repos/o/r/environments/prod": { data: { name: "prod" } }, "GRAPHQL EnvironmentPins": pinsBody([]), @@ -251,9 +230,7 @@ describe("environments pinned apply mode", () => { }); test("EVERY planned mutation's id resolves before the FIRST one fires", async () => { - // Two pins are planned and the SECOND environment's PUT body lacks its - // node_id: resolve-before-write means the first pin must not have fired - // when the resolution throws, or the list would be half-applied. + // The SECOND environment's PUT body lacks its node_id: resolve-before-write means the first pin must not have fired when the resolution throws. const api = new MockApi({ "PUT /repos/o/r/environments/a": envBody("a"), "PUT /repos/o/r/environments/b": { data: { name: "b" } }, @@ -269,9 +246,7 @@ describe("environments pinned apply mode", () => { }); test("a converged run never resolves ids, so a missing node_id cannot fail it", async () => { - // The plan carries no pin mutation, so no thunk resolves an id: an API - // that stopped carrying node_id must not break a repository that is - // already in the declared state. + // No pin mutation is planned, so no thunk resolves an id: an API that stopped carrying node_id must not break a converged repository. const api = new MockApi({ "GET /repos/o/r/environments/a": { data: { name: "a", protection_rules: [] } }, "GRAPHQL EnvironmentPins": pinsBody(["a"]), @@ -324,9 +299,7 @@ describe("environments pinned check mode", () => { }); test("every reorder after the first is a continuation of the one order line, never per-environment pin drift", async () => { - // Live [c, b, a], all three declared pinned: a moves to 1, then b to 2 (c - // slid right). The order line explains the sequence once; a misplaced pin - // never earns an environments[x].pinned line. + // After a moves to 1, c has slid right, so b still needs its own move to 2. const api = new MockApi({ "GET /repos/o/r/environments/a": envBody("a"), "GET /repos/o/r/environments/b": envBody("b"), @@ -350,7 +323,6 @@ describe("environments pinned check mode", () => { 'environments.pinned: apply will also move "b" to position 2 in that reordering', ], ]); - // No per-environment pin drift joins the two order lines. expect(planDrift(planned)).toEqual([ "environments.pinned: the declared pin order is [a, b, c] but the live pinned order is [c, b, a]; apply will reorder the pins so the declared ones lead in declaration order", 'environments.pinned: apply will also move "b" to position 2 in that reordering', @@ -367,11 +339,8 @@ describe("environments pinned check mode", () => { { name: "a", pinned: true }, { name: "b", pinned: true }, ]); - // Also the diff-leak pin: a routed `pinned` reaching subsetDiff would - // add an "environments[a].pinned: declared true ..." line here. + // A routed `pinned` reaching subsetDiff would add an "environments[a].pinned: declared true ..." line here. expect(result.drift).toEqual([]); - // legacy sits AFTER the declared block, so apply would not move it: no - // interleaving note, check and apply agree exactly. expect(result.notes).toEqual([]); }); @@ -416,10 +385,8 @@ describe("environments pinned check mode", () => { }); test("a tolerated NOT_FOUND on the pins read reads as no pins, never a permission error", async () => { - // The fine-grained-denial disguise: GraphQL conceals a denied repository - // as NOT_FOUND, which the pins read declares as an outcome - the same - // absent posture as the section's REST probe, so check reports drift - // and the denial surfaces on the first write in apply mode. + // GraphQL conceals a denied repository as NOT_FOUND, which the pins read declares as an outcome (the same absent posture as the REST probe), so + // check reports drift and the denial surfaces on the first write in apply mode. const api = new MockApi({ "GET /repos/o/r/environments/prod": envBody("prod"), "GRAPHQL EnvironmentPins": { diff --git a/src/sections/interaction_limits/interaction_limits.test.ts b/src/sections/interaction_limits/interaction_limits.test.ts index e563b31b8..fc1d789df 100644 --- a/src/sections/interaction_limits/interaction_limits.test.ts +++ b/src/sections/interaction_limits/interaction_limits.test.ts @@ -29,11 +29,7 @@ async function apply(api: GithubClient, desired: Desired) { return executePlan(await plan(api, desired), interactionLimitsSection, api, REPO, TOOLS); } -/** - * A stateful fake of the interaction-limits API: the base PUT stores a - * repository-origin limit with a fixed expires_at (a byte-stable re-arm), - * the cap PATCH merges, and the bypass PUT/DELETE reconcile the login list. - */ +/** A stateful fake of the interaction-limits API; the base PUT stores a fixed expires_at, so a re-arm is byte-stable. */ function liveRepo(seed: { limit?: Record | null; cap?: Record; @@ -95,7 +91,6 @@ describe("interaction_limits", () => { 'armed the "contributors_only" interaction limit (expiry: one_week)', ], ]); - // The declared expiry produces the cannot-verify note. expect(result.notes).toEqual([ "interaction_limits.expiry: GitHub reports only the computed expires_at, so the declared duration cannot be verified; apply re-arms it on every run", ]); @@ -109,8 +104,7 @@ describe("interaction_limits", () => { 'interaction_limits.limit: "contributors_only" != "existing_users"', ]); const matching = await plan(api, { limit: "existing_users" }); - // alwaysRewrite by declaration: the op stands with no drift to report, - // so check reads clean while apply re-arms the ticking limit. + // alwaysRewrite by declaration: check reads clean while apply re-arms the ticking limit. expect(matching.ops.map((op) => [op.role, op.drift, op.change])).toEqual([ [ "put", @@ -189,8 +183,7 @@ describe("interaction_limits", () => { test.each([null, [], "none"])( "a malformed live body (%p) is a loud failure, never an absent limit", async (body) => { - // An empty object is GitHub's "no limit"; anything else that is not a - // limit object must not read as absence and plan a re-arm over it. + // An empty object is GitHub's "no limit"; anything else that is not a limit object must not read as absence and plan a re-arm over it. const api = new MockApi({ [GET]: { data: body } }); await expect(plan(api, { limit: "existing_users" })).rejects.toThrow( /interaction_limits: GET .*interaction-limits returned a body outside the documented shape/, @@ -236,8 +229,7 @@ describe("interaction_limits", () => { "bypassAdd", ]); expect(second.ops.map((op) => [op.role, op.drift])).toEqual([["put", []]]); - // Two executions (the proof also runs the converged plan): the re-arm - // each time, the cap and bypass writes once. + // provePlanIdempotent executes the converged plan too, hence two re-arms. expect(api.writes.filter((w) => w === `PUT ${BASE}`)).toHaveLength(2); expect(api.writes.filter((w) => w !== `PUT ${BASE}`)).toEqual([ `PATCH ${BASE}/pulls/creation-cap`, @@ -327,8 +319,7 @@ describe("interaction_limits pull request creation cap", () => { test("a declared cap key absent from the live cap is noted as a phantom key", async () => { const api = new MockApi({ [CAP_GET]: { data: CAP_LIVE } }); const result = await plan(api, { - // The cast simulates a future/mistyped cap key riding through the - // passthrough shape; the note under test is how plan() surfaces it. + // The cast simulates a future or mistyped cap key riding through the passthrough shape. pull_request_creation_cap: { enabled: true, max_open_prs: 5 }, } as InteractionLimitsConfig); expect(result.notes).toEqual([ @@ -361,9 +352,7 @@ describe("interaction_limits pull request creation bypass list", () => { test("the undeclared logins are removed FIRST, then the missing ones added, case-insensitively", async () => { const api = new MockApi({ [BYPASS_GET]: { data: liveUsers } }); const result = await plan(api, { pull_request_creation_bypass: ["Keeper", "newcomer"] }); - // Removal first: the list holds at most 100 users, so adding before - // removing could transiently overflow it. "Keeper" matches the live - // "keeper" case-insensitively: neither written. + // Removal first: the list holds at most 100 users, so adding before removing could transiently overflow it. expect(result.ops).toEqual([ { role: "bypassRemove", diff --git a/src/sections/labels/mock.test.ts b/src/sections/labels/mock.test.ts index b53feedf2..c5aa9e902 100644 --- a/src/sections/labels/mock.test.ts +++ b/src/sections/labels/mock.test.ts @@ -1,7 +1,5 @@ /** - * Handler-level tests for the labels mock fragment: identity minting rules - * the e2e assertions do not read directly (no scenario asserts on a served - * node_id or url), pinned here against the handler. + * No scenario asserts on a served node_id or url, so the labels fragment's identity minting is pinned here against the handler. */ import { describe, expect, test } from "bun:test"; @@ -21,10 +19,8 @@ function create(state: MockState, body: Record): Record { test("node_id encodes the label's OWN id, matching the generateLabels pattern", () => { - // Seed the state through the generate sugar so the created label's ids - // come from the same monotonic pool as the seeded ones: the old - // post-increment bug (id used, node_id encoding id+1) collided a created - // label's node_id with the NEXT id in that shared pool. + // Seeding through the generate sugar puts the created labels in the same monotonic id pool as the seeded ones; the old post-increment bug (id + // used, node_id encoding id+1) collided a created label's node_id with the next id in that pool. const state = buildStateForSlug( "acme/private", { @@ -35,8 +31,6 @@ describe("labels.create identity minting", () => { ); create(state, { name: "bug", color: "d73a4a" }); create(state, { name: "docs", color: "0075ca" }); - // The pool starts at 90_000_000 and each label, seeded or created, takes - // the next id; its node_id encodes that same id. const identity = (label: unknown) => { const body = label as Record; return [body.name, body.id, body.node_id]; diff --git a/src/sections/pages/mock.test.ts b/src/sections/pages/mock.test.ts index 9088e0218..8e0534fa6 100644 --- a/src/sections/pages/mock.test.ts +++ b/src/sections/pages/mock.test.ts @@ -1,9 +1,6 @@ /** - * Handler-level tests for the pages mock fragment: the GitHub-fidelity - * behaviors the e2e engine flow cannot reach. The action never PUTs the - * Pages config after a delete (an absent site takes the POST create path), - * so the update-after-delete contract is pinned here, directly against the - * handler. + * The action never PUTs the Pages config after a delete (an absent site takes the POST create path), so the update-after-delete contract is pinned + * here, directly against the handler. */ import { describe, expect, test } from "bun:test"; @@ -11,8 +8,7 @@ import { handlerTestContext } from "../../../test/e2e/mock/handler-test-ctx.js"; import { buildStateForSlug, type MockState } from "../../../test/e2e/mock/state.js"; import { pagesMockHandlers } from "./mock.js"; -// The fragment's exact key union makes a typo'd key a compile error, so no -// runtime not-found guard is needed here. +// The fragment's exact key union makes a typo'd key a compile error, so no runtime not-found guard is needed. function handler(key: K): (typeof pagesMockHandlers)[K] { return pagesMockHandlers[key]; } @@ -31,7 +27,6 @@ describe("pages mock handlers", () => { handlerTestContext("pages.update", state, { body: { build_type: "legacy" } }), ); expect(response.status).toBe(404); - // The load-bearing half: the PUT must NOT have re-created the site. expect(state.pages).toBeNull(); }); @@ -43,8 +38,7 @@ describe("pages mock handlers", () => { expect(response.status).toBe(204); expect(state.pages).toMatchObject({ build_type: "legacy", - // The seeded site carried no url, so the update completed it from the - // state slug (an existing stored url would win, matching create). + // The seeded site carried no url, so the update completed it from the state slug (an existing stored url would win, matching create). url: "https://api.github.com/repos/acme/private/pages", }); }); diff --git a/src/sections/pages/pages.test.ts b/src/sections/pages/pages.test.ts index 729dc2133..e5f050323 100644 --- a/src/sections/pages/pages.test.ts +++ b/src/sections/pages/pages.test.ts @@ -8,10 +8,7 @@ import { pagesSection } from "./index.js"; const GET = "GET /repos/o/r/pages"; -/** - * A stateful fake of the Pages API: the GET reflects every create, update, - * and delete, so a plan over executed state sees the converged site. - */ +/** A stateful fake of the Pages API, so a plan over executed state sees the converged site. */ function liveRepo(site: Record | null): GithubClient & { writes: string[] } { let live = site; return { @@ -75,7 +72,6 @@ describe("pages", () => { notes: [], drift: [], }); - // Planning reads and never writes. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([GET]); }); @@ -112,8 +108,7 @@ describe("pages", () => { }); test("a passthrough key named like a prototype member still reaches the second PUT", async () => { - // The create body is a plain object, so an `in` check would see - // Object.prototype's members and drop such a key from the remainder. + // The create body is a plain object, so an `in` check would see Object.prototype's members and drop such a key from the remainder. const api = new MockApi({}); const result = await plan(api, { source: { branch: "main" }, @@ -126,8 +121,7 @@ describe("pages", () => { }); test("a passthrough value JSON cannot carry is a BUG naming its path, never a wire body", async () => { - // Every pages payload crosses plainData(): the loose shape lets an - // arbitrary passthrough value through, and only a JSON-plain one may leave. + // The loose shape lets an arbitrary passthrough value through; plainData() is where only a JSON-plain one may leave. const api = new MockApi({ [GET]: { data: {} } }); await expect( plan(api, { cname: "docs.example.com", hook: () => "x" } as unknown as Parameters< diff --git a/src/sections/repository/repository.test.ts b/src/sections/repository/repository.test.ts index 9c0c601e6..6a8e22c19 100644 --- a/src/sections/repository/repository.test.ts +++ b/src/sections/repository/repository.test.ts @@ -10,7 +10,6 @@ import { describeProblem } from "../../problem.js"; import { PermissionDenied } from "../contract/errors.js"; import { FEATURE_TOGGLES, normalizeTopics, repositorySection } from "./index.js"; -/** The verdict's error prose, or null when the document validated. */ function shapeError(doc: Record, sourceLabel: string): string | null { return validateSectionShapes(doc, sourceLabel).match(() => null, describeProblem); } @@ -62,11 +61,7 @@ const echo = (fields: Record) => ({ "GRAPHQL UpdateRepositoryFeatures": { data: { updateRepository: { repository: fields } } }, }); -/** - * A stateful fake of everything the repository section touches: the repo - * body, the four readable toggles (each GET answers as GitHub does), the - * write-only LFS pair (stored nowhere), and the two GraphQL-only fields. - */ +/** A stateful fake of the repository API: each toggle GET answers as GitHub does, and the write-only LFS pair is stored nowhere. */ function liveRepo(seed: { repo?: Record; toggles?: Record; @@ -186,8 +181,7 @@ describe("repository", () => { }); test("a declared field the repository GET does not return is noted as a phantom key", async () => { - // The PATCH is diff-gated: such a key would re-PATCH on every apply - // without ever converging, so the note says so alongside the drift. + // The PATCH is diff-gated, so such a key would re-PATCH on every apply without converging. const api = new MockApi({ [GET]: { data: { description: "d" } } }); const result = await plan(api, { description: "d", extra_field: "x" }); expect(result.notes).toEqual([ @@ -205,8 +199,7 @@ describe("repository", () => { ] as const)( "a toggle GET body off the documented shape fails loudly (%s: %p)", async (feature, body) => { - // Neither a definite on nor a definite off may be read off a body the - // contract does not document, since either would drive a write. + // Neither a definite on nor a definite off may be read off a body the contract does not document, since either would drive a write. const key = { "vulnerability-alerts": "enable_vulnerability_alerts", "automated-security-fixes": "enable_automated_security_fixes", @@ -253,9 +246,8 @@ describe("repository", () => { }); /** - * Every toggle's endpoint pair, spelled explicitly so a swapped production - * role fails here instead of being read back as the expectation; - * `enabledBody` is the GET's answer when the feature is on (LFS has none). + * Every toggle's endpoint pair, spelled out so a swapped production role fails here instead of being read back as the expectation; `enabledBody` is + * the GET's answer when the feature is on (LFS has none). */ const TOGGLE_CASES = [ ["enable_vulnerability_alerts", "vulnerability alerts", "vulnerability-alerts", null], @@ -289,9 +281,8 @@ describe("repository", () => { test.each(TOGGLE_CASES)( "%s toggles its own endpoint: PUT on true, DELETE on false, never in the PATCH", async (key, label, feature, enabledBody) => { - // Readable toggles start opposite each declaration; LFS has no live - // state. So every direction plans its write, and the exact-mutations - // assertion proves no toggle leaks into the repository PATCH. + // Readable toggles start opposite each declaration and LFS has no live state, so every direction plans its write; the exact-mutations assertion + // proves no toggle leaks into the PATCH. const path = `/repos/o/r/${feature}`; const on = new MockApi({ [GET]: { data: {} } }).allowMutations(`PUT ${path}`); const enabled = await apply(on, { [key]: true }); @@ -350,9 +341,6 @@ describe("repository", () => { ] as const)( "%s is a note, never a change line", async (_what, desired, live, write, status, note) => { - // The feature changed under the plan (or is enforced above the repo): - // the tolerated status says nothing changed, in the status's declared - // meaning, and the run goes on. const api = new MockApi({ [GET]: { data: {} }, ...live, @@ -395,8 +383,7 @@ describe("repository", () => { test.each([404, 422])( "a %i on the private vulnerability reporting probe reads as not applicable, so off", async (status) => { - // A repo where the feature does not apply (private repos): a matching - // declared false is clean, a declared true is drift with the PUT due. + // A repo where the feature does not apply (private repos): a matching declared false is clean, a declared true is drift with the PUT due. const check = new MockApi({ [GET]: { data: {} }, "GET /repos/o/r/private-vulnerability-reporting": { @@ -449,10 +436,8 @@ describe("repository", () => { }); test("a cyclic toggle value is rejected with a message, never a formatter throw", () => { - // A YAML alias cycle (enable_git_lfs: &v { self: *v }) reaches the shape - // as a self-referential object; the error text must be built without - // JSON.stringify on it, or validation itself would die and the run would - // lose its normal failed result. + // A YAML alias cycle (enable_git_lfs: &v { self: *v }) reaches the shape as a self-referential object; the error text must be built without + // JSON.stringify on it, or validation itself would die. const cyclic: Record = {}; cyclic.self = cyclic; const error = shapeError({ repository: { enable_git_lfs: cyclic } }, "f.yml"); @@ -461,15 +446,12 @@ describe("repository", () => { }); test("the section accepts plain mappings only, like the record shape always did", () => { - // requirePlainMapping guards the passthrough mapping: a YAML !!timestamp - // document parses to a Date, which zod's object schemas would accept as - // an empty mapping, so it must fail shape validation instead. + // A YAML !!timestamp document parses to a Date, which zod's object schemas would accept as an empty mapping. expect(shapeError({ repository: new Date("2020-01-01") }, "f.yml")).toContain("repository"); expect(shapeError({ repository: [1, 2] }, "f.yml")).toContain("repository"); }); test("immutable releases reads the {enabled} body, treats 404 as off, and names owner enforcement", async () => { - // Live enabled, declared false: ordinary drift with the apply promise. const liveOn = new MockApi({ [GET]: { data: {} }, "GET /repos/o/r/immutable-releases": { data: { enabled: true, enforced_by_owner: false } }, @@ -483,8 +465,6 @@ describe("repository", () => { ], ], ]); - // The probe's 404 reads as off: drift against declared true, clean against - // declared false. const liveOff = new MockApi({ [GET]: { data: {} }, "GET /repos/o/r/immutable-releases": { @@ -495,8 +475,7 @@ describe("repository", () => { "repository.enable_immutable_releases: declared true != live false; apply will set the declared value", ]); expect((await plan(liveOff, { enable_immutable_releases: false })).ops).toEqual([]); - // Owner-enforced: the drift says apply cannot change it (the write is - // still planned; its 409 is covered by the tolerated-status cases). + // The write is still planned under enforcement; its 409 is covered by the tolerated-status cases. const enforced = new MockApi({ [GET]: { data: {} }, "GET /repos/o/r/immutable-releases": { data: { enabled: true, enforced_by_owner: true } }, @@ -510,7 +489,6 @@ describe("repository", () => { ], ], ]); - // A matching declaration stays clean even under enforcement. expect((await plan(enforced, { enable_immutable_releases: true })).ops).toEqual([]); }); @@ -548,8 +526,7 @@ describe("repository", () => { ]); expect(second.ops.map((op) => op.role)).toEqual(["lfsPut"]); expect(second.notes).toEqual(first.notes); - // Two executions (the proof also runs the converged plan): every write - // once, the LFS re-assertion each time. + // provePlanIdempotent executes the converged plan too, hence the second LFS PUT. expect(api.writes).toEqual([ "PATCH /repos/o/r", "PUT /repos/o/r/topics", @@ -646,7 +623,6 @@ describe("repository GraphQL-routed keys", () => { notes: [], landed: 1, }); - // The complete call sequence: the repo GET, ONE features read, then the mutation. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ GET, "GRAPHQL RepositoryFeatures", @@ -655,9 +631,6 @@ describe("repository GraphQL-routed keys", () => { }); test("partial divergence: the mutation and the change lines carry only the diverged key", async () => { - // enable_sponsorships already matches live; only the policy moves. A - // change line for the untouched sponsor button would be a false claim - // (the section's own 409 rule: a note, never a false change line). const api = new MockApi({ [GET]: { data: {} }, ...features({ hasSponsorshipsEnabled: true }), @@ -691,8 +664,7 @@ describe("repository GraphQL-routed keys", () => { }); test("an echo reporting the old value, or no echo at all, fails the write loudly after it landed", async () => { - // "Accepted but silently ignored" is the REST failure mode that forced - // these keys onto GraphQL; the mutation's echoed post-state is the guard. + // "Accepted but silently ignored" is the REST failure mode that forced these keys onto GraphQL; the mutation's echoed post-state is the guard. const stale = new MockApi({ [GET]: { data: {} }, ...features(), @@ -740,10 +712,8 @@ describe("repository GraphQL-routed keys", () => { }); test("an unreadable value on a DECLARED key fails loudly instead of folding to a default", async () => { - // A null issueCreationPolicy (the SDL marks the field nullable) or a - // non-boolean sponsorship flag must never read as "all"/false - that - // could report a clean check against state the section does not - // understand. + // A null issueCreationPolicy (the SDL marks the field nullable) or a non-boolean sponsorship flag must never fold to "all"/false: that could + // report a clean check against state the section does not understand. const nullPolicy = new MockApi({ [GET]: { data: {} }, ...features({ issueCreationPolicy: null }), @@ -768,8 +738,7 @@ describe("repository GraphQL-routed keys", () => { }); test("an unreadable value on an UNDECLARED key never fails the run", async () => { - // The strictness is scoped to declared keys: a null policy (SDL-nullable) - // must not fail a run that only declared the sponsor button. + // The strictness is scoped to declared keys: a null policy (SDL-nullable) must not fail a run that only declared the sponsor button. const api = new MockApi({ [GET]: { data: {} }, ...features({ hasSponsorshipsEnabled: true, issueCreationPolicy: null }), @@ -810,9 +779,8 @@ describe("repository GraphQL-routed keys", () => { }); test("prototype-chain property names never pass the policy vocabulary", () => { - // `"constructor" in ISSUE_CREATION_POLICIES` is true via the prototype - // chain; the vocabulary check must be an own-property check or these - // would validate and then map to garbage at the GraphQL boundary. + // `"constructor" in ISSUE_CREATION_POLICIES` is true via the prototype chain, so the vocabulary check must be an own-property check or these + // would map to garbage at the GraphQL boundary. for (const name of ["constructor", "toString", "__proto__"]) { expect( shapeError({ repository: { issue_creation_policy: name } }, "f.yml"), diff --git a/src/sections/rulesets/mock.test.ts b/src/sections/rulesets/mock.test.ts index 58a09bba8..102ae605b 100644 --- a/src/sections/rulesets/mock.test.ts +++ b/src/sections/rulesets/mock.test.ts @@ -1,6 +1,5 @@ /** - * Handler-level tests for the rulesets mock fragment: the bypass_actors - * visibility rule, which no scenario reads off the wire directly. + * No scenario reads the bypass_actors visibility rule off the wire directly, so it is pinned here against the handler. */ import { describe, expect, test } from "bun:test"; @@ -46,8 +45,7 @@ describe("rulesets bypass_actors visibility", () => { } test("a ruleset stored without the key reads bypass_actors: [] at write grade, like GitHub", () => { - // Otherwise an admin declaring a non-empty list against such a ruleset - // would see the hidden-key notice instead of the genuine drift. + // Otherwise an admin declaring a non-empty list against such a ruleset would see the hidden-key notice instead of the genuine drift. const bare = buildStateForSlug( "acme/repo", { settingsYaml: null, liveState: { rulesets: [withoutBypass] } }, diff --git a/src/sections/rulesets/rulesets.test.ts b/src/sections/rulesets/rulesets.test.ts index abe1068a6..859c2e24b 100644 --- a/src/sections/rulesets/rulesets.test.ts +++ b/src/sections/rulesets/rulesets.test.ts @@ -38,11 +38,7 @@ describe("normalizeRuleset", () => { /** A live ruleset as the list summary and the by-id read return it. */ type LiveRuleset = Record & { id: number; name: string; source_type?: string }; -/** - * A stateful fake of the rulesets API: reads reflect every write, so a plan - * over executed state sees the converged repository. `ignoredKeys` are - * accepted on a write and dropped, as GitHub does with a key it does not know. - */ +/** A stateful fake of the rulesets API; `ignoredKeys` are accepted on a write and dropped, as GitHub does with a key it does not know. */ function liveRepo( rulesets: LiveRuleset[], ignoredKeys: readonly string[] = [], @@ -133,7 +129,6 @@ describe("rulesets", () => { ], drift: [], }); - // Planning reads and never writes, even against a client that would accept one. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([listRoute]); }); @@ -180,17 +175,14 @@ describe("rulesets", () => { }); test("a declared key the live ruleset lacks is drift plus a phantom-key note", async () => { - // A declared key the read-back lacks is either ignored (a typo) or - // write-only; one read cannot tell, so the update still runs and the - // note warns that it will keep running until the key is fixed or removed. + // One read cannot tell a typo from a write-only key, so the update still runs and the note warns it recurs until the key is fixed or removed. const api = writable({ [listRoute]: { data: [{ id: 9, name: "main", source_type: "Repository" }] }, "GET /repos/o/r/rulesets/9": { data: { id: 9, name: "main", target: "branch", enforcement: "active" }, }, }); - // A variable, not a literal, so the extra key is a passthrough field to - // the type checker rather than an excess property. + // A variable, not a literal, so the extra key is a passthrough field to the type checker rather than an excess property. const misspelled = { name: "main", target: "branch" as const, enforcemant: "evaluate" }; const result = await plan(api, [misspelled]); expect(result).toEqual({ @@ -214,8 +206,7 @@ describe("rulesets", () => { }); test("a key GitHub drops re-plans the identical update and note on every pass: documented non-convergence", async () => { - // One read cannot tell a typo from a field GitHub omits until set, so the - // write is never withheld; the note is what tells the user it recurs. + // One read cannot tell a typo from a field GitHub omits until set, so the write is never withheld. const api = liveRepo( [{ id: 9, name: "main", source_type: "Repository", target: "branch", enforcement: "active" }], ["enforcemant"], @@ -259,8 +250,7 @@ describe("rulesets", () => { }); describe("bypass_actors visibility", () => { - // GitHub answers a non-admin GET without the bypass_actors KEY (never `[]`), - // so the declared list cannot be judged: converged, one note, no phantom. + // GitHub answers a non-admin GET without the bypass_actors key (never `[]`), so the declared list cannot be judged. const BASE = { id: 9, name: "main", target: "branch", enforcement: "active" }; const HIDDEN_NOTE = "rulesets[main]: bypass_actors is not visible to this token (GitHub returns it only to a token with write access to the ruleset), so drift on it cannot be judged here; grant Administration write to check it"; @@ -331,8 +321,6 @@ describe("rulesets", () => { } test("a hidden bypass_actors beside real drift: the note, the drift, and the full declared payload", async () => { - // The write path is unchanged: the PUT carries the whole declaration, - // hidden list included, and the hidden key is not a phantom. const api = writable({ [listRoute]: { data: [{ id: 9, name: "main", source_type: "Repository" }] }, "GET /repos/o/r/rulesets/9": { data: { ...BASE, enforcement: "evaluate" } }, @@ -452,9 +440,7 @@ describe("rulesets", () => { }); test("_undeclared:delete never deletes a ruleset without an explicit Repository source", async () => { - // source_type is optional in the API type; a missing field is not proof - // of repository ownership, and deletion cannot be undone. Organization - // and enterprise rulesets never enter the managed list at all. + // source_type is optional in the API type; a missing field is not proof of repository ownership, and deletion cannot be undone. const api = writable({ [listRoute]: { data: [ @@ -545,9 +531,8 @@ describe("rulesets", () => { }); test("a planned operation can only name a declared write role, and must justify itself", () => { - // Compile-time only: the plans are never executed. Each rejected shape - // is built first and assigned on one line, so the directive anchors to - // the assignment whichever property the compiler blames. + // Compile-time only. Each rejected shape is built first and assigned on one line, so the @ts-expect-error anchors to the assignment whichever + // property the compiler blames. type Op = PlannedOp; const _create: Op = { role: "create", payload: { name: "x" }, drift: ["missing"], change: "" }; const read = { role: "get", params: { ruleset_id: "1" }, drift: ["x"], change: "" } as const; diff --git a/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts b/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts index bf4637e1f..5f563a641 100644 --- a/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts +++ b/src/sections/secret_scanning_custom_patterns/secret_scanning_custom_patterns.test.ts @@ -171,8 +171,7 @@ describe("secret_scanning_custom_patterns", () => { }); test("a rename is create plus bulk delete under _undeclared:delete, never a PATCH (no rename inference)", async () => { - // The declared pattern carries the SAME fields as the live one, only the - // name differs: the name is the identity. + // The declared pattern carries the same fields as the live one, only the name differs: the name is the identity. const api = new MockApi( listRoute([livePattern({ id: 9, name: "old-name", custom_pattern_version: "v7" })]), ); @@ -205,7 +204,6 @@ describe("secret_scanning_custom_patterns", () => { ], }, ]); - // The default keep leaves the renamed-away pattern live, as a note. const kept = await plan(api, [{ name: "new-name", pattern: "int_[a-z0-9]{8}" }]); expect(kept.ops.map((op) => op.role)).toEqual(["create"]); expect(kept.notes).toHaveLength(1); @@ -213,8 +211,7 @@ describe("secret_scanning_custom_patterns", () => { }); test("the bulk DELETE always sends resolve_alerts and each pattern's version, omitting a version-less one", async () => { - // resolve_alerts is policy, not configuration: upstream's delete_alerts - // default is never sent and no knob exists. + // resolve_alerts is policy, not configuration: upstream's delete_alerts default is never sent and no knob exists. const api = new MockApi( listRoute([ livePattern({ id: 3, name: "stale-a", custom_pattern_version: "v3" }), @@ -253,8 +250,7 @@ describe("secret_scanning_custom_patterns", () => { test.each<[form: string, live: Record, at: RegExp]>([ ["no id", { name: "no-id" }, /\[0\]\.id/], ["a non-string name", { id: 1, name: 5 }, /\[0\]\.name/], - // string = concurrency token; null/absent = none offered; anything else - // must not quietly disable the 412 protection. + // string = concurrency token, null/absent = none offered; anything else must not quietly disable the 412 protection. [ "a numeric version", livePattern({ custom_pattern_version: 7 }), @@ -269,8 +265,7 @@ describe("secret_scanning_custom_patterns", () => { }); test("an empty delimiter is rejected at document validation (clearing is not expressible)", () => { - // "" cannot mean "clear it": the PATCH updates provided fields only, so - // the zod shape rejects the spelling before any repository is touched. + // "" cannot mean "clear it": the PATCH updates provided fields only. for (const key of ["start_delimiter", "end_delimiter"] as const) { const doc = { secret_scanning_custom_patterns: [{ ...INTERNAL, [key]: "" }] }; const invalid = validateSettingsDoc(doc, "test doc", new Set(), silentIo()); @@ -338,9 +333,8 @@ describe("secret_scanning_custom_patterns", () => { describe("secret_scanning_custom_patterns closed surface", () => { test("rejects the read-only state and push_protection_enabled keys BY NAME, before any call", () => { - // True by construction (closedSurface lists the six declared fields), - // but no other test names the two read-only fields a user would most - // plausibly try to declare. + // True by construction (closedSurface lists the six declared fields), but no other test names the two read-only fields a user would most + // plausibly declare. for (const key of ["state", "push_protection_enabled"]) { const error = validateSettingsDoc( { secret_scanning_custom_patterns: [{ ...INTERNAL, [key]: true }] }, diff --git a/src/sections/webhooks/webhooks.test.ts b/src/sections/webhooks/webhooks.test.ts index a5acee131..033bc072e 100644 --- a/src/sections/webhooks/webhooks.test.ts +++ b/src/sections/webhooks/webhooks.test.ts @@ -17,7 +17,6 @@ import { webhooksSection } from "./index.js"; import { webhooksMockHandlers } from "./mock.js"; import type { WebhookConfig } from "./schema.js"; -/** The verdict's error prose, or null when the document validated. */ function shapeError(doc: Record, sourceLabel: string): string | null { return validateSectionShapes(doc, sourceLabel).match(() => null, describeProblem); } @@ -73,8 +72,7 @@ const SECRET_NOTE = describe("webhooks shape", () => { test("an entry-level secret is rejected, pointing at config.secret", () => { - // The misplacement would otherwise pass the loose shape, ship the raw - // reference text verbatim, and create a silently unauthenticated hook. + // The misplacement would otherwise pass the loose shape, ship the raw reference text verbatim, and create a silently unauthenticated hook. const result = webhooksSection.shape.safeParse([ { config: { url: "https://t.test/h" }, secret: "$HOOK_SECRET" }, ]); @@ -122,10 +120,9 @@ describe("webhooks secretValues", () => { }); test("malformed containers return [] and leave the error to validation", () => { - // The extractor can face any merged value, so a malformed declaration - // must not throw here - validation is where the user gets the message. + // The extractor faces any merged value, so the double cast feeds it a pre-validation value on purpose; validation is where the user gets the + // message. for (const malformed of [null, "hooks", 42, { _undeclared: "keep" }, [null, "x"]]) { - // The double cast feeds the extractor a PRE-VALIDATION value on purpose. expect(webhooksSection.secretValues?.(malformed as unknown as WebhookConfig[])).toEqual([]); } }); @@ -292,8 +289,7 @@ describe("webhooks plan", () => { }); test("a declared url matching several live hooks fails BEFORE any operation is planned, naming their ids", async () => { - // A missing url declared BEFORE the ambiguous one must not become a - // create; the scan runs over the whole declaration first. + // A missing url declared before the ambiguous one must not become a create; the scan runs over the whole declaration first. const api = new MockApi({ [LIST]: { data: [liveHook(11, "https://dup.test/h"), liveHook(12, "https://dup.test/h")] }, }); @@ -359,8 +355,7 @@ describe("webhooks plan", () => { 'DELETED undeclared webhook "https://stray.test/hook"', ]); expect(notes).toEqual([]); - // The proof also executes the converged second plan, so the two - // secret-bearing config PATCHes land once more and nothing else does. + // provePlanIdempotent executes the converged second plan too, so the two secret-bearing config PATCHes land once more. expect(api.writes).toEqual([ "PATCH /repos/o/r/hooks/601/config", "PATCH /repos/o/r/hooks/601", @@ -369,8 +364,7 @@ describe("webhooks plan", () => { "PATCH /repos/o/r/hooks/601/config", expect.stringMatching(/^PATCH \/repos\/o\/r\/hooks\/\d+\/config$/), ]); - // The created hook now exists, so its secret recurs as a config PATCH: the - // facet with no lines, which check mode reads as clean plus the note. + // The created hook now exists, so its secret recurs as a config PATCH: the facet with no lines, which check mode reads as clean plus the note. expect(first.ops.map((op) => op.role)).toEqual(["updateConfig", "update", "create", "remove"]); expect(second.ops.map((op) => [op.role, op.params, op.drift])).toEqual([ [ diff --git a/src/sections/workflows/workflows.test.ts b/src/sections/workflows/workflows.test.ts index 70f867c7a..da67b6e6a 100644 --- a/src/sections/workflows/workflows.test.ts +++ b/src/sections/workflows/workflows.test.ts @@ -14,10 +14,7 @@ interface LiveWorkflow { state: string; } -/** - * A stateful fake of the workflows API: the list reflects every enable and - * disable, so a plan over executed state sees the converged repository. - */ +/** A stateful fake of the workflows API, so a plan over executed state sees the converged repository. */ function liveRepo(workflows: LiveWorkflow[]): GithubClient & { writes: string[] } { return { writes: [], @@ -81,7 +78,6 @@ describe("workflows", () => { notes: [], drift: [], }); - // Planning reads and never writes. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([route]); }); @@ -186,9 +182,8 @@ describe("workflows", () => { }); test("a planned operation can only name a declared write role, and must justify itself", () => { - // Compile-time only: the plans are never executed. Each rejected shape - // is built first and assigned on one line, so the directive anchors to - // the assignment whichever property the compiler blames. + // Compile-time only. Each rejected shape is built first and assigned on one line, so the @ts-expect-error anchors to the assignment whichever + // property the compiler blames. type Op = PlannedOp; const _enable: Op = { role: "enable", diff --git a/test/action/action-yml.test.ts b/test/action/action-yml.test.ts index 4785b0b8a..c92ce962e 100644 --- a/test/action/action-yml.test.ts +++ b/test/action/action-yml.test.ts @@ -1,8 +1,4 @@ -/** - * action.yml's hand-written half (name, runtime) and the input/output - * declarations its generated half renders from; the generated regions - * themselves are covered by test/scripts/gen-action-docs.test.ts. - */ +/** The generated regions of action.yml are covered by test/scripts/gen-action-docs.test.ts. */ import { describe, expect, test } from "bun:test"; import { readFileSync } from "node:fs"; @@ -33,11 +29,8 @@ describe("action.yml <-> README", () => { describe("action.yml runtime", () => { test("runs.using is node24 and AGENTS.md documents the same runtime", () => { - // The runtime is a conscious pin: bumping it changes what Node the - // built bundle must run on, so the change has to land here too. + // Bumping the runtime changes what Node the built bundle must run on, so the change has to land here too. expect(actionYml.runs.using).toBe("node24"); - // AGENTS.md tells agents which runtime the bundle targets; it must name - // the one action.yml declares. const agents = readFileSync(join(ROOT, "AGENTS.md"), "utf8"); expect( agents.includes(`(${actionYml.runs.using})`), @@ -48,9 +41,7 @@ describe("action.yml runtime", () => { describe("input declarations <-> discovery defaults", () => { test("each discovery filter declares an empty default and shows its effective one", () => { - // A filter is "explicitly set" when its raw input is not "", so a - // non-empty declared default would defeat that detection; the Inputs - // table shows the effective default instead and the description names it. + // A filter is "explicitly set" when its raw input is not "", so a non-empty declared default would defeat that detection. const effective: Partial> = { visibility: DEFAULT_DISCOVERY_FILTERS.visibility, archived: DEFAULT_DISCOVERY_FILTERS.archived, @@ -78,10 +69,7 @@ describe("input declarations <-> discovery defaults", () => { describe("output declarations", () => { test("the result description mentions every RepoResult value and the merge result", () => { - // REPO_RESULTS is the canonical value list exported next to worstOf() in - // src/engine/orchestrate.ts; a new RepoResult value added there but left - // out of the output docs fails here. MERGE_RESULT is the one value outside - // that list (a merge has no target), documented the same way. + // MERGE_RESULT is the one value outside REPO_RESULTS (a merge has no target). const missing = [...REPO_RESULTS, MERGE_RESULT].filter( (value) => !OUTPUT_DECLS.result.description.includes(value), ); diff --git a/test/action/artifact.test.ts b/test/action/artifact.test.ts index a9b4a0bc5..9d379b047 100644 --- a/test/action/artifact.test.ts +++ b/test/action/artifact.test.ts @@ -17,9 +17,8 @@ describe("the action's uploader without a runtime token", () => { test("missing token yields exactly ONE warning and never invokes the artifact client", async () => { delete process.env.ACTIONS_RUNTIME_TOKEN; - // Reaching the client would double-warn (it warns on its own before it - // throws). Its @actions/core import is a named binding a namespace spy - // cannot observe, so the client's entry point is what gets watched. + // Reaching the client would double-warn (it warns before it throws); its @actions/core import is a named binding a namespace spy cannot observe, + // so the client's entry point is watched. const uploadSpy = spyOn(DefaultArtifactClient.prototype, "uploadArtifact"); try { const recipient = await identityToRecipient(await generateX25519Identity()); @@ -29,8 +28,6 @@ describe("the action's uploader without a runtime token", () => { recipient, ); - // exactly one warning, and it is ours (the client's own text never - // appears, and neither does any report content) expect(result).toEqual({ warning: "could not upload the private report artifact: the artifact service is unavailable: no " + diff --git a/test/action/inputs.test.ts b/test/action/inputs.test.ts index db871e629..266b21276 100644 --- a/test/action/inputs.test.ts +++ b/test/action/inputs.test.ts @@ -3,10 +3,7 @@ import { ok } from "neverthrow"; import { parseActionConfig } from "../../src/action/inputs.js"; import { INPUT_DECLS, SectionSelection } from "../../src/index.js"; -/** - * The adapter reads INPUT_* through @actions/core and the GITHUB_* context - * from the environment, so the test sets exactly that env and restores it. - */ +// @actions/core keeps the dashes in INPUT_* names. const ENV_KEYS = [ ...Object.keys(INPUT_DECLS).map((name) => `INPUT_${name.toUpperCase()}`), "GITHUB_TOKEN", diff --git a/test/action/io.test.ts b/test/action/io.test.ts index d18f05c34..d71bdba40 100644 --- a/test/action/io.test.ts +++ b/test/action/io.test.ts @@ -5,11 +5,7 @@ import { join } from "node:path"; import { actionsIo } from "../../src/action/io.js"; import { writeSummary } from "../../src/flows/summary.js"; -/** - * Whether `source` names the runner module as a module specifier - a static - * import, a re-export, a dynamic import(), or a require() all quote it; a - * comment mentioning it bare does not. - */ +/** A static import, a re-export, a dynamic import(), or a require() all quote the specifier; a comment mentioning it bare does not. */ function namesActionsCore(source: string): boolean { return /["']@actions\/core["']/.test(source); } @@ -28,9 +24,7 @@ describe("the Io port boundary", () => { }); test("only src/action/ names @actions/core", () => { - // Every other layer reaches the runner through the Io port, so redaction - // and capture have one place to stand. A new direct dependency anywhere - // else is a leak this scan names. + // Every other layer reaches the runner through the Io port, so redaction and capture have one place to stand. const srcDir = join(import.meta.dir, "..", "..", "src"); const files = readdirSync(srcDir, { recursive: true }) as string[]; const offenders: string[] = []; @@ -45,8 +39,7 @@ describe("the Io port boundary", () => { } } expect(offenders).toEqual([]); - // The scan saw the tree (a wrong root would pass vacuously), and the one - // permitted importer is where the check expects it. + // The scan saw the tree (a wrong root would pass vacuously). expect(scanned).toBeGreaterThan(50); expect(namesActionsCore(readFileSync(join(srcDir, "action", "io.ts"), "utf8"))).toBe(true); }); diff --git a/test/action/run.test.ts b/test/action/run.test.ts index d8916c56d..68430fcaf 100644 --- a/test/action/run.test.ts +++ b/test/action/run.test.ts @@ -10,12 +10,8 @@ import type { ArtifactUploader } from "../../src/report/artifact-report.js"; import { SECTION_KEYS } from "../../src/schema.js"; import { MockApi } from "../mock-api.js"; -// Every run() below injects this capturing Io in place of the @actions/core -// sink, so a green suite prints no raw ::error::/::warning:: workflow -// commands - and the failure-path tests assert the exact captured text -// (annotations as ": ", log lines verbatim). The step summary -// and the action outputs are captured the same way, so the assertions read -// exactly what run() handed the port. +// A capturing Io replaces the @actions/core sink, so a green suite prints no raw workflow commands and the failure-path tests assert the exact +// captured text. let captured: string[] = []; let summaries: string[] = []; let outputs: Record = {}; @@ -221,9 +217,7 @@ describe("run in multi-repo mode (env glue)", () => { }, }); expect(await run({ api: failing, io: testIo })).toBe(1); - // The failure path's own reporting, captured instead of echoed to the test - // log: the error annotation for the failed target (redacted by default, - // since the 500 leaves its visibility unproven) and the outcome line. + // The error annotation is redacted by default, since the 500 leaves the target's visibility unproven. expect(captured).toContain( "error: private repository #1: failed - repository. details hidden: the repository is private or internal. Set private-repos: show to reveal them, or run the action inside that repository", ); @@ -269,7 +263,6 @@ describe("run in multi-repo mode (env glue)", () => { expect(await run({ api: api, io: testIo })).toBe(1); const output = outputs["repos-result"] ?? ""; const summary = summaries.join("\n"); - // neither the output nor the summary carries the private slug or values for (const text of [output, summary]) { expect(text).not.toContain("o/priv"); expect(text).not.toContain("SECRET-live"); @@ -293,20 +286,16 @@ describe("run in multi-repo mode (env glue)", () => { "GET /repos/o/priv": { data: { has_wiki: true, private: true } }, }); expect(await run({ api: api, io: testIo })).toBe(1); - // the single-repo redaction path registered the slug for masking expect(captured).toContain("mask: o/priv"); const summary = summaries.join("\n"); expect(summary).not.toContain("o/priv"); expect(summary).toContain("details hidden"); - // Finding F: the redacted single-repo summary renders the SAME per-section - // table the multi path does - the section key and its status are visible - // (the policy keeps statuses everywhere), the detail cell is hidden, and - // the live drift value never appears. + // The redacted single-repo summary renders the SAME per-section table the multi path does: statuses stay visible everywhere, only the detail cell + // is hidden. expect(summary).toContain("| Section | Status | Detail |"); expect(summary).toContain("repository"); expect(summary).toContain(":warning: drift"); expect(summary).toContain("hidden (private repository)"); - // the live value that drifted must not leak expect(summary).not.toContain("has_wiki"); }); @@ -321,18 +310,14 @@ describe("run in multi-repo mode (env glue)", () => { const api = new MockApi({ "GET /repos/o/self": { data: { has_wiki: false, private: true } } }); expect(await run({ api: api, io: testIo })).toBe(0); const summary = summaries.join("\n"); - // full detail: the section table renders normally, no redaction note expect(summary).not.toContain("details hidden"); expect(summary).toContain("repository"); - // and no visibility probe: the self carve-out skips it (only the engine GET) + // The self carve-out skips the visibility probe: the one GET is the engine's. const gets = api.calls.filter((c) => c.method === "GET" && c.path === "/repos/o/self"); expect(gets).toHaveLength(1); }); - // Every invalid private-repos/private-report/report-public-key combination - // is rejected at config parse - exit 1 before any API call - and the error - // annotation must name ITS OWN rule: exit code and call count alone would - // pass on a wrong-rule rejection. + // The error annotation must name ITS OWN rule: exit code and call count alone would pass on a wrong-rule rejection. test.each([ [ "private-report: issue with private-repos: show", @@ -409,7 +394,6 @@ describe("run in multi-repo mode (env glue)", () => { } const api = new MockApi({}); expect(await run({ api: api, io: testIo })).toBe(1); - // rejected at config parse, before any API call expect(api.calls).toHaveLength(0); expect(captured.filter((line) => line.startsWith("error: ")).join("\n")).toContain(fragment); }, @@ -529,10 +513,8 @@ describe("run in multi-repo mode (env glue)", () => { process.env["INPUT_PRIVATE-REPOS"] = "redact"; process.env["INPUT_PRIVATE-REPORT"] = "issue"; process.env.INPUT_MODE = "check"; - // repo GET body has neither private nor visibility -> unknown -> redact, no deliver const api = new MockApi({ "GET /repos/o/maybe": { data: { has_wiki: true } } }); expect(await run({ api: api, io: testIo })).toBe(1); // drift exits 1 - // no issue/label traffic: the report was withheld, and the withholding is said once, safely expect(api.calls.some((c) => c.path.includes("/issues"))).toBe(false); expect(api.calls.some((c) => c.method === "POST" && c.path.endsWith("/labels"))).toBe(false); const withheld = captured.find((line) => line.includes("visibility could not be verified")); @@ -735,8 +717,7 @@ describe("run in mode: merge", () => { test.each([ ["a mapping", { setting: true }], - // A null on an unknown key is no opt-out marker: nothing known is - // being removed, so it is the same misspelling and must name the file. + // A null on an unknown key is no opt-out marker: nothing known is being removed, so it is the same misspelling. ["null", null], ])( "a layer with an unknown top-level key set to %s fails naming the layer: a merge has no allowlist to tolerate it", @@ -756,9 +737,7 @@ describe("run in mode: merge", () => { ); test("a sections allowlist is rejected before any layer is read, so the written document is never narrower than the fold", async () => { - // The fixtures fold to three sections (THREE_LAYERS_MERGED above); an - // allowlist naming one of them is refused up front rather than - // narrowing the file, and a layer path that does not exist proves the + // An allowlist naming one of the three folded sections is refused up front rather than narrowing the file; the nonexistent layer path proves the // refusal precedes the read. setMergeEnv([layer("fleet.yml"), layer("repo.yml"), join(dir, "nope.yml")]); process.env.INPUT_SECTIONS = "labels"; @@ -774,9 +753,8 @@ describe("run in mode: merge", () => { }); test("a cyclic layer (a YAML anchor that includes itself) is refused by the fold, naming the layer", async () => { - // Raw YAML: a self-referencing anchor under a private key parses to a - // cyclic object; the standalone validation ignores the key, so the - // engine's boundary is what refuses the layer. + // A self-referencing anchor under a private key parses to a cyclic object; standalone validation ignores the key, so the engine's boundary is + // what refuses the layer. const top = join(dir, "top.yml"); writeFileSync(top, ["_notes: &loop", " self: *loop", "labels: []", ""].join("\n")); setMergeEnv([layer("fleet.yml"), top]); diff --git a/test/architecture/architecture.test.ts b/test/architecture/architecture.test.ts index 36c60ff7d..314f4437b 100644 --- a/test/architecture/architecture.test.ts +++ b/test/architecture/architecture.test.ts @@ -1,11 +1,6 @@ /** - * The architecture lint's contract: the tree's cross-layer imports are exactly - * the edges architecture.yml declares (the verdict `bun run lint:arch` prints, - * so CI's test job carries the gate), and each way the declaration can be - * wrong is seen failing: a forbidden edge names both files, a stale allowance - * names the edge, a file outside every layer is reported. Every import form - * the scanner must read has a control, since a missed form would let a - * forbidden import pass. + * The verdict `bun run lint:arch` prints, so CI's test job carries the gate; every import form the scanner must read has a control, since a missed + * form would let a forbidden import pass. */ import { describe, expect, test } from "bun:test"; @@ -84,8 +79,7 @@ describe("importSpecifiers", () => { }); test("a computed dynamic import throws rather than dropping the edge", () => { - // The whole message: the scanner is shared with changed-sections, so under - // lint:arch it must not blame that tool. + // The whole message: the scanner is shared with changed-sections, so under lint:arch it must not blame that tool. expect(() => importSpecifiers('const m = "./a.js"; await import(m);', "x.ts")).toThrow( new Error( "x.ts:1 loads a module through a computed specifier, which the import graph cannot follow - use a string literal", diff --git a/test/discovery/central.test.ts b/test/discovery/central.test.ts index 8afe24868..e62f265f6 100644 --- a/test/discovery/central.test.ts +++ b/test/discovery/central.test.ts @@ -4,8 +4,7 @@ import { resolveCentralTargets } from "../../src/discovery/central.js"; describe("resolveCentralTargets", () => { test("reads owner-shorthand and owner/name files, warns on strays", () => { - // The walk sorts directory entries, so the target and warning order is - // fixed: top-level files first, then each owner directory. + // The walk sorts directory entries, so the order is fixed: top-level files first, then each owner directory. expect(resolveCentralTargets("test/fixtures/repos", "viv")).toEqual( ok({ targets: [ diff --git a/test/discovery/discover.test.ts b/test/discovery/discover.test.ts index ca59a1e86..24d9d9cbd 100644 --- a/test/discovery/discover.test.ts +++ b/test/discovery/discover.test.ts @@ -242,9 +242,8 @@ describe("discoverRepos", () => { }); test("a rate-limit 403 gets re-run advice, not PAT advice", async () => { - // A primary rate limit arrives as a 403 whose message mentions the rate - // limit; isPermissionError excludes it, so discovery must NOT tell the - // operator to swap tokens and abandon "*" for a transient throttle. + // A primary rate limit is a 403 whose message names the rate limit; isPermissionError excludes it, so discovery must not tell the operator to + // swap tokens for a transient throttle. const api = new MockApi({ "GET /user/repos?affiliation=owner&per_page=100&page=1": { error: { status: 403, message: "API rate limit exceeded for user", body: "" }, diff --git a/test/discovery/targets.test.ts b/test/discovery/targets.test.ts index c1f62c3c0..5d1b9d545 100644 --- a/test/discovery/targets.test.ts +++ b/test/discovery/targets.test.ts @@ -39,16 +39,14 @@ describe("dedupeTargets", () => { (m) => notices.push(m), () => "private repository #1", ); - // The whole notice: the placeholder replaces the slug everywhere, the - // origin noun phrase is wrapped without a doubled article. + // Pinned whole: wrapping the origin noun phrase must not double its article. expect(notices).toEqual([ 'private repository #1: using the central file repos/x.yml; the entry for the same repository from the "repos" input is ignored', ]); }); test("a redacted target's central origin is rendered generically, never the file path", () => { - // The central file path (repos/secret.yml) can embed the real repo name, so for - // a redacted target it must not appear next to the placeholder. + // The central file path (repos/secret.yml) can embed the real repo name, so it must not appear next to the placeholder. const notices: string[] = []; dedupeTargets( [ diff --git a/test/docs/biome-schema.test.ts b/test/docs/biome-schema.test.ts index b88eada6f..9598283a9 100644 --- a/test/docs/biome-schema.test.ts +++ b/test/docs/biome-schema.test.ts @@ -1,7 +1,6 @@ /** - * biome.json's $schema URL names a biome version by hand, and `biome ci` only - * emits an info notice when it lags the installed CLI, so a dependency bump - * leaves it drifting silently. Pin the two together here. + * biome.json's $schema URL names a biome version by hand, and `biome ci` only emits an info notice when it lags the installed CLI, so a dependency + * bump leaves it drifting silently. */ import { describe, expect, test } from "bun:test"; diff --git a/test/docs/checks-sections-gate.test.ts b/test/docs/checks-sections-gate.test.ts index 59ddcba9f..aa6d488d5 100644 --- a/test/docs/checks-sections-gate.test.ts +++ b/test/docs/checks-sections-gate.test.ts @@ -1,10 +1,6 @@ /** - * Sentinel contract between the diff-aware section selector - * (.github/scripts/changed-sections.ts) and the checks.yml e2e-smoke job that - * branches on its printed token. The workflow compares raw strings in shell, - * so a renamed ALL/NONE constant (or an edited job script) would silently - * always-run or always-skip the smoke job; pin each comparison to the - * constants the selector actually prints. + * The checks.yml e2e-smoke job compares the selector's printed token as a raw shell string, so its literals are pinned to the constants the selector + * prints. */ import { describe, expect, test } from "bun:test"; diff --git a/test/docs/checks-workflow.test.ts b/test/docs/checks-workflow.test.ts index 7120b0766..d0fd5d3fe 100644 --- a/test/docs/checks-workflow.test.ts +++ b/test/docs/checks-workflow.test.ts @@ -1,9 +1,3 @@ -/** - * Workflow contract for the fetched, gitignored test artifacts: one composite owns both caches - * and miss-gated fetches, no workflow caches one inline, every loading job runs the composite - * first, and the drift-tripwire nightlies fetch the spec fresh instead. - */ - import { describe, expect, test } from "bun:test"; import { readdirSync, readFileSync } from "node:fs"; import { join, relative, resolve } from "node:path"; @@ -50,10 +44,7 @@ const FETCHED_ARTIFACTS: readonly FetchedArtifact[] = [ ]; const [OPENAPI, GRAPHQL] = FETCHED_ARTIFACTS as [FetchedArtifact, FetchedArtifact]; -/** - * Jobs whose spec fetch IS the upstream-drift tripwire: they call the script - * directly and never restore a cache; every other loading job uses the composite. - */ +/** Jobs whose spec fetch IS the upstream-drift tripwire: they call the script directly and never restore a cache. */ const UNCACHED_FETCH_JOBS: ReadonlySet = new Set([ "e2e-nightly.yml#nightly", "nightly-fuzz.yml#fuzz", @@ -76,10 +67,6 @@ function escapeRegExp(text: string): string { return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } -/** - * The scripts that spawn the bundle against the spec-validating mock: every - * non-test file that imports the e2e runner, directly or through another. - */ function harnessEntrypoints(): string[] { const files = ["test/e2e", ".github/scripts"].flatMap((dir) => sourceFilesUnder(join(ROOT, dir))); return [...transitiveDependents(reverseImportGraph(files), join(ROOT, "test/e2e/runner.ts"))] @@ -208,9 +195,8 @@ function hashFilesPatterns(key: string): string[] { } /** - * The repo-relative .ts files paths.ts imports route data from (compiled .js specifiers mapped - * back to source). Only single-line static imports are recognized; any other import-ish line - * fails the assertion below, so an unsupported form extends this parser instead of being skipped. + * The .ts files paths.ts imports route data from. Only single-line static imports are recognized; any other import-ish line fails, so an unsupported + * form extends this parser instead of being skipped. */ function routeDataImports(): string[] { const source = readFileSync(join(ROOT, PATHS_TS), "utf8"); @@ -351,10 +337,6 @@ function installs(run: string | undefined): boolean { ); } -/** - * A run scalar executing `bun