diff --git a/src/main.ts b/src/main.ts index 79d9086..d66629b 100644 --- a/src/main.ts +++ b/src/main.ts @@ -28,6 +28,7 @@ import { ApiClient } from "./remote/api-client.ts"; import { Remote } from "./remote/remote.ts"; import { ConnectionManager } from "./sync/connection-manager.ts"; import { PgbadgerSource } from "./sql/pgbadger.ts"; +import { baselineNotFoundMessage } from "./reporters/baseline-notice.ts"; import type { RecentQuerySource } from "./sql/recent-query.ts"; import type { FullSchema, RepoPolicyConfig } from "@query-doctor/core"; @@ -189,12 +190,7 @@ async function runInCI( if (result.kind === "found") { previousRun = result.run; } else if (result.kind === "not-found") { - log.info( - "main", - `No baseline found on branch "${comparisonBranch}". Comparison will be skipped. ` + - `To establish a baseline, run the analyzer on pushes to "${comparisonBranch}" ` + - `(add "push: branches: [${comparisonBranch}]" to your workflow trigger).`, - ); + log.info("main", baselineNotFoundMessage(comparisonBranch)); } else { // Transient fetch failure after retries — flag it so the comment says // "temporarily unavailable, re-run" rather than claiming there is no diff --git a/src/reporters/baseline-notice.test.ts b/src/reporters/baseline-notice.test.ts new file mode 100644 index 0000000..ae58fa2 --- /dev/null +++ b/src/reporters/baseline-notice.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, test } from "vitest"; +import { baselineNotFoundMessage } from "./baseline-notice.ts"; + +describe("baselineNotFoundMessage", () => { + test("names the branch and the trigger that records a baseline", () => { + const message = baselineNotFoundMessage("main"); + + expect(message).toContain("main"); + expect(message).toContain("push: branches: [main]"); + }); + + // The branch resolves from the configured comparison branch, then the pull + // request base, then the current branch. On a push run for a project with + // none set, all three can be absent, which is how this reached a real log. + test.each([ + ["empty", ""], + ["undefined", undefined], + ["null", null], + ])("never names a branch it does not have: %s", (_case, branch) => { + const message = baselineNotFoundMessage(branch); + + // What the real log read: `pushes to ""` and `push: branches: []`, which + // matches no branch and reads as an instruction the user cannot follow. + expect(message).not.toContain('""'); + expect(message).not.toContain("branches: []"); + expect(message).toContain("No comparison branch is set"); + }); +}); diff --git a/src/reporters/baseline-notice.ts b/src/reporters/baseline-notice.ts new file mode 100644 index 0000000..f2ad3ef --- /dev/null +++ b/src/reporters/baseline-notice.ts @@ -0,0 +1,24 @@ +/** + * What to log when no baseline exists for the comparison branch. + * + * The branch is resolved from the project's configured comparison branch, then + * the pull request base, then the current branch, so it can come out empty on a + * push run for a project that has not set one. Interpolating that empty string + * produced `pushes to ""` and `push: branches: []`, which matches no branch at + * all and reads as a broken instruction. + */ +export function baselineNotFoundMessage( + comparisonBranch: string | undefined | null, +): string { + if (!comparisonBranch) { + return ( + "No comparison branch is set, so nothing was compared. " + + "Set one in the project's CI settings, then push to it once so a baseline is recorded." + ); + } + return ( + `No baseline found on branch "${comparisonBranch}". Comparison will be skipped. ` + + `A baseline is recorded when the analyzer runs on a push to "${comparisonBranch}"; ` + + `if the workflow has no push trigger for it, add "push: branches: [${comparisonBranch}]".` + ); +} diff --git a/src/reporters/github/github.test.ts b/src/reporters/github/github.test.ts index 8fca354..06fca97 100644 --- a/src/reporters/github/github.test.ts +++ b/src/reporters/github/github.test.ts @@ -648,10 +648,38 @@ describe("baseline absent vs. temporarily unavailable (Site#3287)", () => { const output = renderTemplate(ctx); expect(output).toContain("No baseline on `staging`"); - expect(output).toContain("add a `push` trigger"); + // Still points at the push trigger, now as a condition rather than an + // instruction, because a correctly configured first run lands here too. + expect(output).toContain("`push` trigger"); expect(output).not.toContain("temporarily unavailable"); }); + // "" is what a first push run on a project with no configured branch actually + // produces: main.ts resolves the configured branch, then the PR base, then the + // current branch, and on that path all three can be absent. + test.each([ + ["absent", {}], + ["empty", { comparisonBranch: "" }], + ])("never names a branch it does not have: %s", (_case, overrides) => { + const output = renderTemplate(makeContext(overrides)); + + // An empty inline code span is the shape of the bug — `{{ comparisonBranch }}` + // interpolated to nothing, leaving "No baseline on ``" above instructions + // for a branch that was never named. + expect(output).not.toMatch(/``/); + // "is set", not just "No comparison branch": the unset-baseline warning + // lower in the template opens "No comparison branch configured", so the + // looser string would pass on the wrong block. + expect(output).toContain("No comparison branch is set"); + }); + + test("names the branch and keeps the push-trigger guidance when it has one", () => { + const output = renderTemplate(makeContext({ comparisonBranch: "staging" })); + + expect(output).toContain("No baseline on `staging`"); + expect(output).toContain("`push` trigger"); + }); + test("transient fetch failure renders a re-run message, not the no-baseline copy", () => { const ctx = makeContext({ comparisonBranch: "staging", @@ -663,7 +691,7 @@ describe("baseline absent vs. temporarily unavailable (Site#3287)", () => { expect(output).toContain("re-run the check"); // Must not tell the user to add a trigger that is already in place. expect(output).not.toContain("No baseline on `staging`"); - expect(output).not.toContain("add a `push` trigger"); + expect(output).not.toContain("`push` trigger"); }); }); diff --git a/src/reporters/github/success.md.j2 b/src/reporters/github/success.md.j2 index 16177f3..86507cb 100644 --- a/src/reporters/github/success.md.j2 +++ b/src/reporters/github/success.md.j2 @@ -50,7 +50,11 @@ Comparison temporarily unavailable. {% else %} No baseline found to compare against. -> **No baseline on `{{ comparisonBranch }}`** — the analyzer cannot detect regressions without a previous run. To establish a baseline, add a `push` trigger for your comparison branch so the analyzer runs on merges to `{{ comparisonBranch }}`. See the [CI integration guide](https://docs.querydoctor.com/guides/ci-integration/#workflow-trigger) for setup instructions. +{% if comparisonBranch %} +> **No baseline on `{{ comparisonBranch }}`.** The analyzer records a baseline when it runs on a push to `{{ comparisonBranch }}`, and compares later runs against it. If this run is the first, the next one has something to compare against. If the workflow has no `push` trigger for `{{ comparisonBranch }}`, add one: see the [CI integration guide](https://docs.querydoctor.com/guides/ci-integration/#workflow-trigger). +{% else %} +> **No comparison branch is set**, so nothing was compared. Set one in the project's CI settings, then push to it once so the analyzer records a baseline. See the [CI integration guide](https://docs.querydoctor.com/guides/ci-integration/#workflow-trigger). +{% endif %} {% endif %} {% if hasComparison and runMetadata and runMetadata.baseline and runMetadata.baseline.unset %}