From fa2c1ace2d5c30b6d4dab7627e0c913c060e6994 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Thu, 12 Nov 2026 04:58:07 -0500 Subject: [PATCH 1/3] test(e2e): check the GraphQL mock's answers against GitHub's schema The e2e mock refused a GraphQL request GitHub's schema refuses, but its answers went back to the product as the handler built them. A handler that had stored a null answered it in a Boolean! field, and the two rule mutations answered the whole rule node where their documents select four fields. The response guard now completes a handler's data with graphql-js under the request's own document, on the execution args the request check already prepared, so nothing is parsed twice. The product receives the completed data: the selection and nothing beside it, with null under a selected nullable key the handler left out. Data that does not complete is a violation naming the operation, the handler, the response path, and graphql-js's message. It is answered under the operation's own log entry, so a read still counts as a read. graphql-request.ts exports acceptedGraphqlRequest(), and graphqlRequestErrors() wraps it for the section suites' fake, which is unchanged. Not caught: a value graphql-js coerces (1 for a Boolean, "3" for an Int), and a stored off-schema value that no later request selects. --- test/e2e/mock/graphql-answer.test.ts | 207 +++++++++++++++++++++++++++ test/e2e/mock/graphql-answer.ts | 46 ++++++ test/e2e/mock/graphql-request.ts | 30 ++-- test/e2e/mock/routes.ts | 81 +++++++---- 4 files changed, 326 insertions(+), 38 deletions(-) create mode 100644 test/e2e/mock/graphql-answer.test.ts create mode 100644 test/e2e/mock/graphql-answer.ts diff --git a/test/e2e/mock/graphql-answer.test.ts b/test/e2e/mock/graphql-answer.test.ts new file mode 100644 index 00000000..043e6673 --- /dev/null +++ b/test/e2e/mock/graphql-answer.test.ts @@ -0,0 +1,207 @@ +/** + * The pipeline serves a handler's data as graphql-js completes it under the request's own document, and fails the run + * on data GitHub's schema could never serialize. Without it a handler that stored a null hands it back in a Boolean! + * field, a key the document never selected reaches the product, and every scenario passes on both. + */ + +import { describe, expect, spyOn, test } from "bun:test"; +import { rmSync } from "node:fs"; +import { allGraphqlOps, type SectionGraphqlKey } from "../../../src/sections/registry.js"; +import { ADMIN_OWNER as OWNER, ADMIN_REPO as REPO, VIOLATION_PREFIX } from "../constants.js"; +import { runScenario, type ScenarioReport } from "../runner/runner.js"; +import type { Scenario } from "../scenario.js"; +import { type LoggedRequest, newPipelineRunState, type PipelineResult } from "./contract.js"; +import { isWriteRequest } from "./dispatch.js"; +import { completedGraphqlAnswer } from "./graphql-answer.js"; +import { type AcceptedGraphqlRequest, acceptedGraphqlRequest } from "./graphql-request.js"; +import { GRAPHQL_HANDLERS } from "./handlers.js"; +import { handleGraphqlRequest } from "./routes.js"; +import { scenario } from "./server-test-support.js"; +import { buildState, completeRule, ruleWireNode } from "./state.js"; +import type { Json } from "./support.js"; + +const OPS = allGraphqlOps(); +const READ = { owner: OWNER, repo: REPO }; +const NULL_ADMIN_ENFORCED = + "data.repository.branchProtectionRules.nodes.0.isAdminEnforced: Cannot return null for non-nullable field BranchProtectionRule.isAdminEnforced."; +const NULL_ADMIN_ENFORCED_VIOLATION = `GraphQL BranchProtectionRules was answered off GitHub's schema by the mock handler "branches.rulesQuery": ${NULL_ADMIN_ENFORCED}`; + +/** A wildcard rule node as the mock's own state builds one, `overrides` on top. */ +function ruleNode(overrides: Json = {}): Json { + return ruleWireNode(completeRule({ id: "BPR_1", pattern: "release/*", ...overrides })); +} + +/** The rules read's data holding `nodes`. */ +function rulesData(...nodes: Json[]): Json { + return { + repository: { + branchProtectionRules: { nodes, pageInfo: { hasNextPage: false, endCursor: null } }, + }, + }; +} + +/** The features read's data holding `repository`. */ +function featuresData(repository: Json): Json { + return { repository: { id: "R_1", ...repository } }; +} + +/** The declared operation `key` under `variables`, as GitHub accepts it. */ +function accepted(key: SectionGraphqlKey, variables: Json): AcceptedGraphqlRequest { + const request = acceptedGraphqlRequest(OPS[key].query, OPS[key].name, variables); + if ("errors" in request) { + throw new Error(`${key} is refused by GitHub's schema: ${JSON.stringify(request.errors)}`); + } + return request; +} + +/** The pipeline's result for the declared operation `key` when its handler answers `data`. */ +function served(key: SectionGraphqlKey, variables: Json, data: Json): PipelineResult { + const op = OPS[key]; + const body = { query: op.query, operationName: op.name, variables }; + const run = scenario(); + const log: LoggedRequest = { method: "POST", pathname: "/graphql", query: "", status: 0, body }; + return handleGraphqlRequest( + { method: "POST", body }, + { + scenario: run, + working: { mode: "single", state: buildState(run.live_state, run.owner_kind) }, + checkMode: false, + ...newPipelineRunState(), + }, + log, + OPS, + { ...GRAPHQL_HANDLERS, [key]: () => ({ data }) }, + ); +} + +/** One scenario run of the real bundle; the artifact directory a failing run dumps is removed, the report carries what the assertions read. */ +async function scenarioRun(run: Scenario): Promise { + const report = await runScenario(run); + if (report.artifactDir !== undefined) { + rmSync(report.artifactDir, { recursive: true, force: true }); + } + return report; +} + +describe("data GitHub's schema could never serialize does not complete", () => { + test.each([ + { + name: "a null in a non-null field", + key: "branches.rulesQuery", + data: rulesData(ruleNode({ isAdminEnforced: null })), + line: NULL_ADMIN_ENFORCED, + }, + { + name: "a selected non-null field the data lacks", + key: "repository.featuresQuery", + data: featuresData({ issueCreationPolicy: "ALL" }), + line: "data.repository.hasSponsorshipsEnabled: Cannot return null for non-nullable field Repository.hasSponsorshipsEnabled.", + }, + { + name: "a value off its enum", + key: "repository.featuresQuery", + data: featuresData({ hasSponsorshipsEnabled: false, issueCreationPolicy: "all" }), + line: 'data.repository.issueCreationPolicy: Enum "IssueCreationPolicy" cannot represent value: "all"', + }, + { + name: "1.5 for an Int", + key: "branches.rulesQuery", + data: rulesData(ruleNode({ requiredApprovingReviewCount: 1.5 })), + line: "data.repository.branchProtectionRules.nodes.0.requiredApprovingReviewCount: Int cannot represent non-integer value: 1.5", + }, + { + // The line goes on with graphql-js's advice to its own caller. + name: "an abstract value that names no type", + key: "branches.rulesQuery", + data: rulesData({ + ...ruleNode(), + bypassForcePushAllowances: { + nodes: [{ actor: { login: "octocat" } }], + pageInfo: { hasNextPage: false }, + }, + }), + line: 'data.repository.branchProtectionRules.nodes.0.bypassForcePushAllowances.nodes.0.actor: Abstract type "BranchActorAllowanceActor" must resolve to an Object type at runtime', + }, + ] as const)("$name", ({ key, data, line }) => { + expect(completedGraphqlAnswer(accepted(key, READ), data)).toEqual({ + errors: [expect.stringContaining(line)], + }); + }); + + test("control: data on the schema completes to itself, a union member named by its __typename included", () => { + const data = rulesData(ruleNode({ bypassForcePushActors: ["octocat"] })); + expect(completedGraphqlAnswer(accepted("branches.rulesQuery", READ), data)).toEqual({ data }); + }); +}); + +describe("the product receives a handler's data through the request's selection", () => { + test.each([ + { + name: "a key the document does not select is dropped", + key: "repository.featuresQuery", + data: featuresData({ + hasSponsorshipsEnabled: true, + issueCreationPolicy: "ALL", + nameWithOwner: `${OWNER}/${REPO}`, + }), + received: featuresData({ hasSponsorshipsEnabled: true, issueCreationPolicy: "ALL" }), + }, + { + name: "a selected nullable key the handler leaves out is null", + key: "environments.pins", + data: { repository: { pinnedEnvironments: { nodes: [], pageInfo: { hasNextPage: false } } } }, + received: { + repository: { + pinnedEnvironments: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } }, + }, + }, + }, + { + name: "control: data that is the selection already is served as it is", + key: "branches.rulesQuery", + data: rulesData(ruleNode()), + received: rulesData(ruleNode()), + }, + ] as const)("$name", ({ key, data, received }) => { + const result = served(key, READ, data); + expect(result.violation).toBeUndefined(); + expect(result.response).toEqual({ status: 200, body: { data: received } }); + }); +}); + +describe("data that does not complete fails the run", () => { + test("the violation names the operation, the handler, the path, and graphql-js's reason, and the read stays a read in the log", () => { + const result = served( + "branches.rulesQuery", + READ, + rulesData(ruleNode({ isAdminEnforced: null })), + ); + expect(result.violation).toBe(NULL_ADMIN_ENFORCED_VIOLATION); + // The product is answered the violation, never the data. + expect(result.response).toEqual({ + status: 400, + body: { message: `${VIOLATION_PREFIX} ${NULL_ADMIN_ENFORCED_VIOLATION}` }, + }); + // Logged without its GraphQL kind, the POST would count as a write in the runner's check-mode and idempotence rules. + expect(result.log.graphql).toEqual({ operationName: "BranchProtectionRules", kind: "read" }); + expect(isWriteRequest(result.log)).toBe(false); + }); + + test("inside a scenario run: a handler answering a null in a Boolean! field fails the scenario with the violation, and the handlers as they are pass it (the control)", async () => { + const wildcardRule = scenario({ + name: "graphql-answer-in-run", + settings: { branches: [{ name: "release/*", protection: { enforce_admins: true } }] }, + live_state: { branch_protection_rules: [{ pattern: "release/*", isAdminEnforced: true }] }, + }); + const control = await scenarioRun(wildcardRule); + expect(control.failures).toEqual([]); + + using mutant = spyOn(GRAPHQL_HANDLERS, "branches.rulesQuery").mockImplementation(() => ({ + data: rulesData(ruleNode({ isAdminEnforced: null })), + })); + const report = await scenarioRun(wildcardRule); + expect(mutant).toHaveBeenCalled(); + // Every rules read of the run earns the line; how many the product makes is not this test's subject. + expect(report.failures[0]).toStartWith(`mock violations:\n ${NULL_ADMIN_ENFORCED_VIOLATION}`); + }, 120_000); +}); diff --git a/test/e2e/mock/graphql-answer.ts b/test/e2e/mock/graphql-answer.ts new file mode 100644 index 00000000..0b1f4ffc --- /dev/null +++ b/test/e2e/mock/graphql-answer.ts @@ -0,0 +1,46 @@ +/** + * A handler's data as GitHub's schema serves it: graphql-js completes it under the request's own document. Without it + * the mock answers what GitHub cannot, a null in a non-null field or a key the document never selected, and every + * scenario passes on it. + */ + +import { executeRootSelectionSet, type GraphQLFieldResolver } from "graphql"; +import { isPlainObject } from "../../../src/plain-data.js"; +import type { AcceptedGraphqlRequest } from "./graphql-request.js"; +import type { Json } from "./support.js"; + +/** A handler's data is keyed as the response is, so a field is read under its response key, an alias included. */ +const answeredField: GraphQLFieldResolver = (source, _args, _context, info) => + isPlainObject(source) ? source[info.path.key] : undefined; + +/** + * `data` as GitHub would serialize it for `request`: the selection and nothing beside it, a selected nullable field the + * data lacks as null. Data the schema could never serialize returns one line per error instead, the response path and + * graphql-js's message. + */ +export function completedGraphqlAnswer( + request: AcceptedGraphqlRequest, + data: Json, +): { data: Json } | { errors: readonly string[] } { + const completed = executeRootSelectionSet({ + ...request, + rootValue: data, + fieldResolver: answeredField, + }); + if (completed instanceof Promise) { + throw new Error("BUG: graphql-js completed plain data asynchronously"); + } + + const errors = completed.errors ?? []; + if (errors.length > 0) { + return { + errors: errors.map((error) => { + const path = ["data", ...(error.path ?? [])].join("."); + return `${path}: ${error.message}`; + }), + }; + } + + // graphql-js leaves `data` unset only beside errors. + return { data: completed.data as Json }; +} diff --git a/test/e2e/mock/graphql-request.ts b/test/e2e/mock/graphql-request.ts index ae2a8ef0..13c27317 100644 --- a/test/e2e/mock/graphql-request.ts +++ b/test/e2e/mock/graphql-request.ts @@ -9,11 +9,15 @@ import { GraphQLError, type GraphQLFormattedError, parse, + type ValidatedExecutionArgs, validate, validateExecutionArgs, } from "graphql"; import { sharedGraphqlSchema } from "./graphql-schema.js"; +/** A request GitHub's schema accepts, as graphql-js prepared it to run: the parsed document, its operation, the coerced variables. */ +export type AcceptedGraphqlRequest = ValidatedExecutionArgs; + /** The document, or the syntax error graphql-js reports by throwing it. */ function parsedDocument(query: string): DocumentNode | GraphQLError { try { @@ -27,24 +31,24 @@ function parsedDocument(query: string): DocumentNode | GraphQLError { } /** - * The errors GitHub answers `operationName` of `query` with under `variables`, or null when it accepts the request. - * graphql-js words them; api.github.com words the same refusals its own way. Coercion only judges: a caller keeps - * handing on the variables as sent. + * `operationName` of `query` under `variables` as GitHub accepts it, or the errors GitHub answers it with. graphql-js + * words them; api.github.com words the same refusals its own way. Coercion only judges: a caller keeps handing on the + * variables as sent. */ -export function graphqlRequestErrors( +export function acceptedGraphqlRequest( query: string, operationName: string, variables: Readonly>, -): readonly GraphQLFormattedError[] | null { +): AcceptedGraphqlRequest | { errors: readonly GraphQLFormattedError[] } { const document = parsedDocument(query); if (document instanceof GraphQLError) { - return [document.toJSON()]; + return { errors: [document.toJSON()] }; } const schema = sharedGraphqlSchema(); const documentErrors = validate(schema, document); if (documentErrors.length > 0) { - return documentErrors.map((error) => error.toJSON()); + return { errors: documentErrors.map((error) => error.toJSON()) }; } const prepared = validateExecutionArgs({ @@ -53,5 +57,15 @@ export function graphqlRequestErrors( operationName, variableValues: variables, }); - return "operation" in prepared ? null : prepared.map((error) => error.toJSON()); + return "operation" in prepared ? prepared : { errors: prepared.map((error) => error.toJSON()) }; +} + +/** The errors of acceptedGraphqlRequest(), or null when GitHub accepts the request. */ +export function graphqlRequestErrors( + query: string, + operationName: string, + variables: Readonly>, +): readonly GraphQLFormattedError[] | null { + const request = acceptedGraphqlRequest(query, operationName, variables); + return "errors" in request ? request.errors : null; } diff --git a/test/e2e/mock/routes.ts b/test/e2e/mock/routes.ts index 73ca925b..cdaf2f86 100644 --- a/test/e2e/mock/routes.ts +++ b/test/e2e/mock/routes.ts @@ -9,6 +9,7 @@ * GraphQL has no body allowlist: its schema check follows the route match, as GitHub validates before it runs anything. */ +import type { GraphQLFormattedError } from "graphql"; import { isPlainObject } from "../../../src/plain-data.js"; import type { SectionKey } from "../../../src/schema.js"; import { endpointPath, toleratedStatuses } from "../../../src/sections/contract/endpoints.js"; @@ -58,7 +59,8 @@ import { type Requirement, SECTION_BY_KEY, } from "./grading.js"; -import { graphqlRequestErrors } from "./graphql-request.js"; +import { completedGraphqlAnswer } from "./graphql-answer.js"; +import { type AcceptedGraphqlRequest, acceptedGraphqlRequest } from "./graphql-request.js"; import { GRAPHQL_HANDLERS, HANDLERS } from "./handlers.js"; import { decodeNodeId, type NodeFamily } from "./node-id.js"; import { acceptedBody } from "./request-body.js"; @@ -67,7 +69,6 @@ import { asObject, type GraphqlErrorReply, type GraphqlHandler, - type GraphqlHandlerResult, type Json, type MockResponse, } from "./support.js"; @@ -274,20 +275,15 @@ function parseGraphqlRequest( } /** - * The schema check. GitHub answers a document off its schema, or variables off the operation's declared types, with - * HTTP 200 carrying errors and no data. The action shapes every document and variable itself, so the refusal is a - * violation too: the scenario fails whatever outcome it expected. + * The schema check's refusal. GitHub answers a document off its schema, or variables off the operation's declared + * types, with HTTP 200 carrying errors and no data. The action shapes every document and variable itself, so the + * refusal is a violation too: the scenario fails whatever outcome it expected. */ function graphqlSchemaRefusal( op: TaggedGraphqlOp, - query: string, - variables: Json, + errors: readonly GraphQLFormattedError[], log: LoggedRequest, -): PipelineResult | null { - const errors = graphqlRequestErrors(query, op.name, variables); - if (errors === null) { - return null; - } +): PipelineResult { return { response: { status: 200, body: { errors } }, log: { ...log, status: 200 }, @@ -331,23 +327,43 @@ function graphqlDenial( return { response, log: denied, violation: barrierViolation }; } -/** Response guard, the status-subset analog: an undeclared error type is a mock design bug, not a scenario outcome. */ -function graphqlResponse( +/** Response guard of an `errors` answer, the status-subset analog: an undeclared error type is a mock design bug, not a scenario outcome. */ +function graphqlErrorsResponse( key: string, op: TaggedGraphqlOp, - result: GraphqlHandlerResult, -): MockResponse | { violation: string } { - if (result.errors === undefined) { - return { status: 200, body: { data: result.data } }; - } + errors: readonly GraphqlErrorReply[], + violation: (message: string) => PipelineResult, +): MockResponse | PipelineResult { const declared = toleratedGraphqlErrors(op); - const undeclared = result.errors.filter((entry) => !declared.includes(entry.type)); + const undeclared = errors.filter((entry) => !declared.includes(entry.type)); if (undeclared.length > 0) { - return { - violation: `GraphQL handler "${key}" answered undeclared error type(s) [${undeclared.map((e) => `${e.type}: "${e.message}"`).join(", ")}]; the operation declares only [${declared.join(", ")}] as tolerated outcomes`, - }; + return violation( + `GraphQL handler "${key}" answered undeclared error type(s) [${undeclared.map((e) => `${e.type}: "${e.message}"`).join(", ")}]; the operation declares only [${declared.join(", ")}] as tolerated outcomes`, + ); } - return { status: 200, body: { data: null, errors: result.errors } }; + return { status: 200, body: { data: null, errors } }; +} + +/** + * Response guard of a `data` answer. The product receives what graphql-js completes from the handler's data under the + * request's own document, as from GitHub: no key the document did not select, and null under a selected nullable key + * the handler left out. Data that does not complete is an answer GitHub's schema could never give, the mock's own + * fault: a violation, whatever outcome the scenario expected. + */ +function graphqlDataResponse( + key: string, + op: TaggedGraphqlOp, + request: AcceptedGraphqlRequest, + data: Json, + violation: (message: string) => PipelineResult, +): MockResponse | PipelineResult { + const completed = completedGraphqlAnswer(request, data); + if ("errors" in completed) { + return violation( + `GraphQL ${op.name} was answered off GitHub's schema by the mock handler "${key}": ${completed.errors.join("; ")}`, + ); + } + return { status: 200, body: { data: completed.data } }; } /** @@ -373,9 +389,9 @@ export function handleGraphqlRequest( }; // Ahead of every barrier: no fault, denial, or handler answers a request GitHub refuses before running it. - const refused = graphqlSchemaRefusal(op, query, variables, graphqlLog); - if (refused) { - return refused; + const accepted = acceptedGraphqlRequest(query, op.name, variables); + if ("errors" in accepted) { + return graphqlSchemaRefusal(op, accepted.errors, graphqlLog); } // Check-mode barrier before the fault barrier: a synthetic fault must not mask the one bug this barrier exists to catch. @@ -413,9 +429,14 @@ export function handleGraphqlRequest( // Unreachable after assertGraphqlHandlerCompleteness at construction; loud rather than a silent undefined call. return violation(`no GraphQL handler registered for dispatched operation "${key}"`); } - const response = graphqlResponse(key, op, handler({ state: target.state, op, variables })); - if ("violation" in response) { - return violation(response.violation); + const result = handler({ state: target.state, op, variables }); + // Data off the schema is refused under the operation's own log entry, so the runner still reads a read as a read. + const response = + result.errors === undefined + ? graphqlDataResponse(key, op, accepted, result.data, violationFor(graphqlLog)) + : graphqlErrorsResponse(key, op, result.errors, violation); + if ("log" in response) { + return response; } const corrupted = takeCorruption(key, options, response, graphqlLog); From 21d7ff54ba8dc604023b4529747c384888f9c971 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Thu, 12 Nov 2026 05:14:47 -0500 Subject: [PATCH 2/3] test(e2e): run the answer check's in-run control in-process The control that a mutant handler fails a run called the e2e runner's runScenario from a bun test. The runner removes the bundle it builds on the process exit event, which does not fire under bun test, so every test run left a bundle directory in the temp dir, and the unit-test job came to depend on a node binary. The control now runs the product's own code in-process through runSingle against the mock server. The handlers as they are give "clean" and no violation. A mutant branches.rulesQuery handler gives "failed" and the violation on the mock's list, the list the runner fails a scenario on. On a pipeline without the answer check that run fails too, on the product's own parse, so only the violation tells the two apart. --- test/e2e/mock/graphql-answer.test.ts | 89 +++++++++++++++++++--------- 1 file changed, 62 insertions(+), 27 deletions(-) diff --git a/test/e2e/mock/graphql-answer.test.ts b/test/e2e/mock/graphql-answer.test.ts index 043e6673..47ab0e05 100644 --- a/test/e2e/mock/graphql-answer.test.ts +++ b/test/e2e/mock/graphql-answer.test.ts @@ -5,21 +5,34 @@ */ import { describe, expect, spyOn, test } from "bun:test"; -import { rmSync } from "node:fs"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { stringify as stringifyYaml } from "yaml"; +import { parseRepoSlug } from "../../../src/discovery/targets.js"; +import { SectionSelection } from "../../../src/engine/section-selection.js"; +import { runSingle } from "../../../src/flows/single.js"; +import { GitHubApi } from "../../../src/github/api.js"; +import { silentIo } from "../../../src/io.js"; import { allGraphqlOps, type SectionGraphqlKey } from "../../../src/sections/registry.js"; -import { ADMIN_OWNER as OWNER, ADMIN_REPO as REPO, VIOLATION_PREFIX } from "../constants.js"; -import { runScenario, type ScenarioReport } from "../runner/runner.js"; -import type { Scenario } from "../scenario.js"; +import { withTempDir } from "../../temp-dir.js"; +import { + ADMIN_SLUG, + E2E_TOKEN, + ADMIN_OWNER as OWNER, + ADMIN_REPO as REPO, + VIOLATION_PREFIX, +} from "../constants.js"; import { type LoggedRequest, newPipelineRunState, type PipelineResult } from "./contract.js"; import { isWriteRequest } from "./dispatch.js"; import { completedGraphqlAnswer } from "./graphql-answer.js"; import { type AcceptedGraphqlRequest, acceptedGraphqlRequest } from "./graphql-request.js"; import { GRAPHQL_HANDLERS } from "./handlers.js"; import { handleGraphqlRequest } from "./routes.js"; -import { scenario } from "./server-test-support.js"; +import { mockServerLifecycle, scenario } from "./server-test-support.js"; import { buildState, completeRule, ruleWireNode } from "./state.js"; import type { Json } from "./support.js"; +const start = mockServerLifecycle(); const OPS = allGraphqlOps(); const READ = { owner: OWNER, repo: REPO }; const NULL_ADMIN_ENFORCED = @@ -74,13 +87,39 @@ function served(key: SectionGraphqlKey, variables: Json, data: Json): PipelineRe ); } -/** One scenario run of the real bundle; the artifact directory a failing run dumps is removed, the report carries what the assertions read. */ -async function scenarioRun(run: Scenario): Promise { - const report = await runScenario(run); - if (report.artifactDir !== undefined) { - rmSync(report.artifactDir, { recursive: true, force: true }); - } - return report; +/** + * One check run of the product's own code, in-process and over the wire, on a settings file that matches the mock's one + * wildcard rule. Returns the run's result and the violations the mock recorded, the list the runner fails a scenario on. + */ +async function checkRun(dir: string): Promise<{ result: string; violations: string[] }> { + const mock = await start( + scenario({ + live_state: { branch_protection_rules: [{ pattern: "release/*", isAdminEnforced: true }] }, + }), + ); + const settingsFile = join(dir, "settings.yml"); + const settings = { branches: [{ name: "release/*", protection: { enforce_admins: true } }] }; + writeFileSync(settingsFile, stringifyYaml(settings)); + const io = silentIo(); + const client = new GitHubApi({ token: E2E_TOKEN, io, baseUrl: mock.url, retryBaseMs: 1 }); + + const outcome = await runSingle( + client, + { + mode: "check", + repo: parseRepoSlug(ADMIN_SLUG)._unsafeUnwrap(), + settingsFile, + selfSlug: ADMIN_SLUG, + privateRepos: "redact", + onMissingPermission: "fail", + sections: SectionSelection.ALL, + privateReport: "none", + reportPublicKey: "", + runUrl: "", + }, + io, + ); + return { result: outcome._unsafeUnwrap().result, violations: mock.violations }; } describe("data GitHub's schema could never serialize does not complete", () => { @@ -187,21 +226,17 @@ describe("data that does not complete fails the run", () => { expect(isWriteRequest(result.log)).toBe(false); }); - test("inside a scenario run: a handler answering a null in a Boolean! field fails the scenario with the violation, and the handlers as they are pass it (the control)", async () => { - const wildcardRule = scenario({ - name: "graphql-answer-in-run", - settings: { branches: [{ name: "release/*", protection: { enforce_admins: true } }] }, - live_state: { branch_protection_rules: [{ pattern: "release/*", isAdminEnforced: true }] }, - }); - const control = await scenarioRun(wildcardRule); - expect(control.failures).toEqual([]); + test("inside a run of the product: a handler answering a null in a Boolean! field fails the run and records the violation, and the handlers as they are run clean (the control)", () => + withTempDir("graphql-answer-run-", async (dir) => { + expect(await checkRun(dir)).toEqual({ result: "clean", violations: [] }); - using mutant = spyOn(GRAPHQL_HANDLERS, "branches.rulesQuery").mockImplementation(() => ({ - data: rulesData(ruleNode({ isAdminEnforced: null })), + using mutant = spyOn(GRAPHQL_HANDLERS, "branches.rulesQuery").mockImplementation(() => ({ + data: rulesData(ruleNode({ isAdminEnforced: null })), + })); + const refused = await checkRun(dir); + expect(mutant).toHaveBeenCalled(); + expect(refused.result).toBe("failed"); + // Every rules read of the run earns the line; how many the product makes is not this test's subject. + expect(new Set(refused.violations)).toEqual(new Set([NULL_ADMIN_ENFORCED_VIOLATION])); })); - const report = await scenarioRun(wildcardRule); - expect(mutant).toHaveBeenCalled(); - // Every rules read of the run earns the line; how many the product makes is not this test's subject. - expect(report.failures[0]).toStartWith(`mock violations:\n ${NULL_ADMIN_ENFORCED_VIOLATION}`); - }, 120_000); }); From faa9f35121990e17ed14ad3ab06a52f38b524799 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Thu, 12 Nov 2026 08:21:16 -0500 Subject: [PATCH 3/3] test(e2e): refuse a GraphQL mock answer that completes to another value graphql-js completes some answers GitHub's schema could never give without an error. A non-object at an object position becomes an object of nulls, and a leaf is coerced: true in an Int field to 1, 5 in an ID to "5". Such an answer reached the product with no violation. After a completion with no error, the completed data must now equal the handler's data at every selected position: an object for an object, a list of the same length for a list, a leaf by ===. The one allowed difference is null under a nullable key the handler left out. Any other difference is a mock violation naming the path and both values. The file headers no longer say every scenario passed on an answer off the schema. Where the product parses the field, the run already failed, with the product's own error. --- test/e2e/mock/graphql-answer.test.ts | 77 ++++++++++++++++++++++++++-- test/e2e/mock/graphql-answer.ts | 48 ++++++++++++++--- test/e2e/mock/routes.ts | 4 +- 3 files changed, 117 insertions(+), 12 deletions(-) diff --git a/test/e2e/mock/graphql-answer.test.ts b/test/e2e/mock/graphql-answer.test.ts index 47ab0e05..3b1ce95c 100644 --- a/test/e2e/mock/graphql-answer.test.ts +++ b/test/e2e/mock/graphql-answer.test.ts @@ -1,7 +1,7 @@ /** * The pipeline serves a handler's data as graphql-js completes it under the request's own document, and fails the run - * on data GitHub's schema could never serialize. Without it a handler that stored a null hands it back in a Boolean! - * field, a key the document never selected reaches the product, and every scenario passes on both. + * on data GitHub's schema could never have produced. Without these tests the guard could stop firing and no scenario + * would turn red: the corpus holds only answers on the schema. */ import { describe, expect, spyOn, test } from "bun:test"; @@ -27,6 +27,7 @@ import { isWriteRequest } from "./dispatch.js"; import { completedGraphqlAnswer } from "./graphql-answer.js"; import { type AcceptedGraphqlRequest, acceptedGraphqlRequest } from "./graphql-request.js"; import { GRAPHQL_HANDLERS } from "./handlers.js"; +import { mintNodeId } from "./node-id.js"; import { handleGraphqlRequest } from "./routes.js"; import { mockServerLifecycle, scenario } from "./server-test-support.js"; import { buildState, completeRule, ruleWireNode } from "./state.js"; @@ -35,6 +36,11 @@ import type { Json } from "./support.js"; const start = mockServerLifecycle(); const OPS = allGraphqlOps(); const READ = { owner: OWNER, repo: REPO }; +const DELETE_RULE = { + input: { branchProtectionRuleId: mintNodeId("rule", ADMIN_SLUG, "release/*") }, +}; +const NUMBER_FOR_A_PAYLOAD = + 'data.deleteBranchProtectionRule: answered 7, the schema serves {"clientMutationId":null}'; const NULL_ADMIN_ENFORCED = "data.repository.branchProtectionRules.nodes.0.isAdminEnforced: Cannot return null for non-nullable field BranchProtectionRule.isAdminEnforced."; const NULL_ADMIN_ENFORCED_VIOLATION = `GraphQL BranchProtectionRules was answered off GitHub's schema by the mock handler "branches.rulesQuery": ${NULL_ADMIN_ENFORCED}`; @@ -173,7 +179,60 @@ describe("data GitHub's schema could never serialize does not complete", () => { }); }); -describe("the product receives a handler's data through the request's selection", () => { +// graphql-js completes each of these without an error: an object of nulls for a non-object, the coerced value for a leaf. +describe("data that completes to anything but itself is no answer of GitHub's schema", () => { + test.each([ + { + name: "a number at an object position", + key: "branches.deleteRule", + variables: DELETE_RULE, + data: { deleteBranchProtectionRule: 7 }, + line: NUMBER_FOR_A_PAYLOAD, + }, + { + name: "an array at an object position", + key: "branches.deleteRule", + variables: DELETE_RULE, + data: { deleteBranchProtectionRule: [] }, + line: 'data.deleteBranchProtectionRule: answered [], the schema serves {"clientMutationId":null}', + }, + { + name: "a string as the whole data", + key: "environments.pins", + variables: READ, + data: "not an object", + line: 'data: answered "not an object", the schema serves {"repository":null}', + }, + { + name: "a string inside a list of objects", + key: "branches.rulesQuery", + variables: READ, + data: rulesData({ + ...ruleNode(), + bypassForcePushAllowances: { nodes: ["octocat"], pageInfo: { hasNextPage: false } }, + }), + line: 'data.repository.branchProtectionRules.nodes.0.bypassForcePushAllowances.nodes.0: answered "octocat", the schema serves {"actor":null}', + }, + { + name: "true for an Int", + key: "branches.rulesQuery", + variables: READ, + data: rulesData(ruleNode({ requiredApprovingReviewCount: true })), + line: "data.repository.branchProtectionRules.nodes.0.requiredApprovingReviewCount: answered true, the schema serves 1", + }, + { + name: "5 for an ID", + key: "repository.featuresQuery", + variables: READ, + data: { repository: { id: 5, hasSponsorshipsEnabled: false, issueCreationPolicy: "ALL" } }, + line: 'data.repository.id: answered 5, the schema serves "5"', + }, + ] as const)("$name", ({ key, variables, data, line }) => { + expect(completedGraphqlAnswer(accepted(key, variables), data)).toEqual({ errors: [line] }); + }); +}); + +describe("the product receives the completed data, and its two differences from the handler's data are no violation", () => { test.each([ { name: "a key the document does not select is dropped", @@ -208,7 +267,7 @@ describe("the product receives a handler's data through the request's selection" }); }); -describe("data that does not complete fails the run", () => { +describe("data off the schema fails the run", () => { test("the violation names the operation, the handler, the path, and graphql-js's reason, and the read stays a read in the log", () => { const result = served( "branches.rulesQuery", @@ -226,6 +285,16 @@ describe("data that does not complete fails the run", () => { expect(isWriteRequest(result.log)).toBe(false); }); + test("data that completes to another value is a violation naming the path and both values", () => { + const result = served("branches.deleteRule", DELETE_RULE, { deleteBranchProtectionRule: 7 }); + const violation = `GraphQL DeleteBranchProtectionRule was answered off GitHub's schema by the mock handler "branches.deleteRule": ${NUMBER_FOR_A_PAYLOAD}`; + expect(result.violation).toBe(violation); + expect(result.response).toEqual({ + status: 400, + body: { message: `${VIOLATION_PREFIX} ${violation}` }, + }); + }); + test("inside a run of the product: a handler answering a null in a Boolean! field fails the run and records the violation, and the handlers as they are run clean (the control)", () => withTempDir("graphql-answer-run-", async (dir) => { expect(await checkRun(dir)).toEqual({ result: "clean", violations: [] }); diff --git a/test/e2e/mock/graphql-answer.ts b/test/e2e/mock/graphql-answer.ts index 0b1f4ffc..7d541ad7 100644 --- a/test/e2e/mock/graphql-answer.ts +++ b/test/e2e/mock/graphql-answer.ts @@ -1,7 +1,8 @@ /** - * A handler's data as GitHub's schema serves it: graphql-js completes it under the request's own document. Without it - * the mock answers what GitHub cannot, a null in a non-null field or a key the document never selected, and every - * scenario passes on it. + * A handler's data as GitHub's schema serves it: graphql-js completes it under the request's own document, and the + * completed data must be the handler's own at every selected position, except null under a nullable key the handler + * left out. Without it a key the document never selected reaches the product, and an answer off the schema is judged + * only where the product parses it, as the product's error. */ import { executeRootSelectionSet, type GraphQLFieldResolver } from "graphql"; @@ -13,14 +14,44 @@ import type { Json } from "./support.js"; const answeredField: GraphQLFieldResolver = (source, _args, _context, info) => isPlainObject(source) ? source[info.path.key] : undefined; +/** + * Where `completed` is not what the handler `answered`, one line per position: the path and both values. Completion + * alone lets these through: a string where an object belongs completes to an object of nulls, `true` in an Int field + * to 1. The one difference allowed is null under a nullable key the handler left out. + */ +function differences(answered: unknown, completed: unknown, path: string): string[] { + if (completed === null && answered === undefined) { + return []; + } + + if (Array.isArray(completed) && Array.isArray(answered) && answered.length === completed.length) { + return completed.flatMap((item, index) => + differences(answered[index], item, `${path}.${index}`), + ); + } + + if (isPlainObject(completed) && isPlainObject(answered)) { + return Object.entries(completed).flatMap(([key, value]) => + differences(answered[key], value, `${path}.${key}`), + ); + } + + if (answered === completed) { + return []; + } + return [ + `${path}: answered ${JSON.stringify(answered)}, the schema serves ${JSON.stringify(completed)}`, + ]; +} + /** * `data` as GitHub would serialize it for `request`: the selection and nothing beside it, a selected nullable field the - * data lacks as null. Data the schema could never serialize returns one line per error instead, the response path and - * graphql-js's message. + * data lacks as null. Data GitHub's schema could never have produced returns one line per fault instead: graphql-js's + * error at its response path, or a position where the completed value is not the handler's own. */ export function completedGraphqlAnswer( request: AcceptedGraphqlRequest, - data: Json, + data: unknown, ): { data: Json } | { errors: readonly string[] } { const completed = executeRootSelectionSet({ ...request, @@ -41,6 +72,11 @@ export function completedGraphqlAnswer( }; } + const differing = differences(data, completed.data, "data"); + if (differing.length > 0) { + return { errors: differing }; + } + // graphql-js leaves `data` unset only beside errors. return { data: completed.data as Json }; } diff --git a/test/e2e/mock/routes.ts b/test/e2e/mock/routes.ts index cdaf2f86..fbb3a34a 100644 --- a/test/e2e/mock/routes.ts +++ b/test/e2e/mock/routes.ts @@ -347,8 +347,8 @@ function graphqlErrorsResponse( /** * Response guard of a `data` answer. The product receives what graphql-js completes from the handler's data under the * request's own document, as from GitHub: no key the document did not select, and null under a selected nullable key - * the handler left out. Data that does not complete is an answer GitHub's schema could never give, the mock's own - * fault: a violation, whatever outcome the scenario expected. + * the handler left out. Data that does not complete, or differs from its completion in any other way, is the mock's + * own fault: a violation, whatever outcome the scenario expected. */ function graphqlDataResponse( key: string,