Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -68,32 +68,32 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y
# i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep,revertRecurrence
# coverageDelta,callerImpact
# conflictMarker,debugLeftover,focusedTest,sizeSmell,floatingPromise,deepNesting,errorSwallow
# unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep
# revertRecurrence,coverageDelta,callerImpact
#
# Profile defaults:
# fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
# hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild
# testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker
# debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y,i18n,apiBreak
# deprecatedDep
# debugLeftover,focusedTest,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y
# i18n,apiBreak,deprecatedDep
# balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript
# heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight
# typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication
# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting
# errorSwallow,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak
# deprecatedDep,revertRecurrence,coverageDelta,callerImpact
# todoMarker,magicNumber,conflictMarker,debugLeftover,focusedTest,sizeSmell,floatingPromise
# deepNesting,errorSwallow,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint
# apiBreak,deprecatedDep,revertRecurrence,coverageDelta,callerImpact
# deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
# hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y
# i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep,revertRecurrence
# coverageDelta,callerImpact
# conflictMarker,debugLeftover,focusedTest,sizeSmell,floatingPromise,deepNesting,errorSwallow
# unsafeAny,a11y,i18n,unusedExport,exhaustiveness,flakyTest,commitLint,apiBreak,deprecatedDep
# revertRecurrence,coverageDelta,callerImpact
# END GENERATED REES ANALYZERS

# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep
Expand Down
22 changes: 22 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,28 @@ export const REES_ANALYZERS = [
"Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.",
},
},
{
name: "focusedTest",
title: "Focused tests",
category: "quality",
cost: "local",
defaultEnabled: true,
profiles: ["fast", "balanced", "deep"],
requires: ["files"],
limits: {
maxFindings: 25,
maxLineChars: 2000,
},
docs: {
summary:
"Flags a focused test a PR adds in a test file — `describe.only` / `it.only` / `test.only` — which silently skips every other test in that file.",
looksAt: "Added lines in changed test files.",
reports: "File and line of each focused-test call.",
network: "Pure local analyzer. No external network call.",
notes:
"The test-file counterpart of the debug-leftover analyzer. String literals are stripped before matching, so a `.only` inside a string is not flagged.",
},
},
{
name: "sizeSmell",
title: "Size smells",
Expand Down
26 changes: 26 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,32 @@
"notes": "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching."
}
},
{
"name": "focusedTest",
"title": "Focused tests",
"category": "quality",
"cost": "local",
"defaultEnabled": true,
"profiles": [
"fast",
"balanced",
"deep"
],
"requires": [
"files"
],
"limits": {
"maxFindings": 25,
"maxLineChars": 2000
},
"docs": {
"summary": "Flags a focused test a PR adds in a test file — `describe.only` / `it.only` / `test.only` — which silently skips every other test in that file.",
"looksAt": "Added lines in changed test files.",
"reports": "File and line of each focused-test call.",
"network": "Pure local analyzer. No external network call.",
"notes": "The test-file counterpart of the debug-leftover analyzer. String literals are stripped before matching, so a `.only` inside a string is not flagged."
}
},
{
"name": "sizeSmell",
"title": "Size smells",
Expand Down
86 changes: 86 additions & 0 deletions review-enrichment/src/analyzers/focused-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Focused-test analyzer (part of #1499 quality-signal family). Flags a focused test a PR adds in a test file —
// `describe.only` / `it.only` / `test.only` (and `context`/`suite`/`specify`) — which silently skips every OTHER
// test in that file, so CI can stay green while most of the suite no longer runs. It is the test-file counterpart
// of the debug-leftover analyzer: that one scans NON-test source, this one scans test files only. Pure compute,
// no network. String-literal content is stripped before matching so a `"it.only"` inside a string is not flagged.
// Line-cited via hunk headers, mirroring the sibling local analyzers.
import type { FocusedTestFinding, EnrichRequest } from "../types.js";
import { codeOnly } from "./secret-log.js";
import { isTestPath } from "./test-ratio.js";

const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;

// A `.only` chained onto a known test-block function — the cross-framework (Jest/Vitest/Mocha/Jasmine) way to
// focus one test/suite and skip the rest of the file. Restricted to those function names so an unrelated
// `stream.only(` never matches.
const FOCUSED_TEST_RE =
/\b(?:describe|context|suite|it|test|specify)\s*\.\s*only\s*\(/;

/** Detect a focused-test (`.only`) call in one added line, or null. Pure. */
export function detectFocusedTest(line: string): FocusedTestFinding["kind"] | null {
return FOCUSED_TEST_RE.test(codeOnly(line)) ? "only" : null;
}

type ScanLimits = {
maxFindings?: number;
signal?: AbortSignal;
};

/** Scan one TEST file patch's added lines for focused-test calls, line-cited via hunk headers. Non-test files are
* skipped (a `.only` there is not a focused test). Pure. */
export function scanPatchForFocusedTest(
path: string,
patch: string,
limits: ScanLimits = {},
): FocusedTestFinding[] {
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
if (maxFindings <= 0 || !isTestPath(path)) return [];
const findings: FocusedTestFinding[] = [];
let newLine = 0;
let inHunk = false;
for (const line of patch.split("\n")) {
if (limits.signal?.aborted) throw new Error("analyzer_aborted");
const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
if (hunk) {
newLine = Number(hunk[1]);
inHunk = true;
continue;
}
if (!inHunk) continue;
if (line.startsWith("+")) {
const body = line.slice(1);
if (body.length <= MAX_LINE_CHARS) {
const kind = detectFocusedTest(body);
if (kind) {
findings.push({ file: path, line: newLine, kind });
if (findings.length >= maxFindings) return findings;
}
}
newLine++;
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
newLine++;
}
}
return findings;
}

/** Analyzer entrypoint: scan every changed test file's added lines for focused-test calls. */
export async function scanFocusedTest(
req: EnrichRequest,
signal?: AbortSignal,
): Promise<FocusedTestFinding[]> {
const findings: FocusedTestFinding[] = [];
for (const file of req.files ?? []) {
if (signal?.aborted) throw new Error("analyzer_aborted");
if (!file.patch) continue;
for (const finding of scanPatchForFocusedTest(file.path, file.patch, {
maxFindings: MAX_FINDINGS - findings.length,
signal,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
30 changes: 30 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { scanLooseRanges } from "./loose-range.js";
import { scanMagicNumbers } from "./magic-number.js";
import { scanConflictMarkers } from "./conflict-marker.js";
import { scanDebugLeftover } from "./debug-leftover.js";
import { scanFocusedTest } from "./focused-test.js";
import { scanDeepNesting } from "./deep-nesting.js";
import { scanI18nRegression } from "./i18n-regression.js";
import { scanErrorSwallow } from "./error-swallow.js";
Expand Down Expand Up @@ -1028,6 +1029,35 @@ export const ANALYZER_DESCRIPTORS = [
},
run: (req, { signal }) => scanDebugLeftover(req, signal),
}),
descriptor({
name: "focusedTest",
title: "Focused tests",
category: "quality",
cost: "local",
defaultEnabled: true,
requires: ["files"],
limits: { maxFindings: 25, maxLineChars: 2000 },
docs: {
summary:
"Flags a focused test a PR adds in a test file — `describe.only` / `it.only` / `test.only` — which silently skips every other test in that file.",
looksAt: "Added lines in changed test files.",
reports: "File and line of each focused-test call.",
network: "Pure local analyzer. No external network call.",
notes:
"The test-file counterpart of the debug-leftover analyzer. String literals are stripped before matching, so a `.only` inside a string is not flagged.",
},
render: (findings, helpers) => {
if (!findings.length) return [];
const lines = ["### Focused tests (`.only` added by this PR — skips the rest of the file)"];
for (const item of findings) {
lines.push(
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.kind)}`,
);
}
return lines;
},
run: (req, { signal }) => scanFocusedTest(req, signal),
}),
descriptor({
name: "sizeSmell",
title: "Size smells",
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ export function renderBrief(
lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber));
lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker));
lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover));
lines.push(...renderDescriptorSection("focusedTest", findings.focusedTest));
lines.push(...renderDescriptorSection("sizeSmell", findings.sizeSmell));
lines.push(...renderDescriptorSection("floatingPromise", findings.floatingPromise));
lines.push(...renderDescriptorSection("deepNesting", findings.deepNesting));
Expand Down
7 changes: 7 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,12 @@ export interface DebugLeftoverFinding {
kind: "debugger" | "console" | "print";
}

export interface FocusedTestFinding {
file: string;
line: number;
kind: "only";
}

/** Maintainability size smell from patch structure (#2019, part of #1499).
* Reports estimated file length or added function body span — never source content. */
export interface SizeSmellFinding {
Expand Down Expand Up @@ -693,6 +699,7 @@ export interface BriefFindings {
magicNumber?: MagicNumberFinding[];
conflictMarker?: ConflictMarkerFinding[];
debugLeftover?: DebugLeftoverFinding[];
focusedTest?: FocusedTestFinding[];
sizeSmell?: SizeSmellFinding[];
floatingPromise?: FloatingPromiseFinding[];
deepNesting?: DeepNestingFinding[];
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/test/analyzer-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const EXPECTED_ANALYZERS = [
"magicNumber",
"conflictMarker",
"debugLeftover",
"focusedTest",
"sizeSmell",
"floatingPromise",
"deepNesting",
Expand Down
100 changes: 100 additions & 0 deletions review-enrichment/test/focused-test.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// Units for the focused-test analyzer (part of #1499). Own file (not enrichment.test.ts) so concurrent analyzer
// PRs don't collide. No network — pure, stateless per-line detection. Runs against the compiled dist/.
import { test } from "node:test";
import assert from "node:assert/strict";
import {
detectFocusedTest,
scanFocusedTest,
scanPatchForFocusedTest,
} from "../dist/analyzers/focused-test.js";
import { renderBrief } from "../dist/render.js";

const patchOf = (lines: string[]) =>
`@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`;

test("detectFocusedTest: recognizes .only on every test-block function", () => {
assert.equal(detectFocusedTest("describe.only('a', () => {"), "only");
assert.equal(detectFocusedTest(" it.only('does x', () => {"), "only");
assert.equal(detectFocusedTest("test.only('y', () => {"), "only");
assert.equal(detectFocusedTest("context.only('z', () => {"), "only");
assert.equal(detectFocusedTest("suite.only('s', () => {"), "only");
assert.equal(detectFocusedTest("specify.only('w', () => {"), "only");
// whitespace tolerance around the dot and call paren
assert.equal(detectFocusedTest("it . only ( 'spaced', () => {"), "only");
});

test("detectFocusedTest: ordinary tests and non-test .only calls are not flagged", () => {
assert.equal(detectFocusedTest("it('does x', () => {"), null);
assert.equal(detectFocusedTest("describe('suite', () => {"), null);
// `.only(` on something that is not a test-block function
assert.equal(detectFocusedTest("stream.only(handler)"), null);
// a test-fn name embedded in a longer identifier must not match
assert.equal(detectFocusedTest("submit.only(form)"), null);
// `.only` with no call paren is not a focused-test call
assert.equal(detectFocusedTest("const flag = it.only"), null);
});

test("detectFocusedTest: a .only inside a string literal is not flagged", () => {
assert.equal(detectFocusedTest('const s = "it.only(\'x\')";'), null);
});

test("scanPatchForFocusedTest: only scans test files", () => {
const patch = patchOf(["it.only('x', () => {});"]);
assert.deepEqual(scanPatchForFocusedTest("src/widget.test.ts", patch), [
{ file: "src/widget.test.ts", line: 1, kind: "only" },
]);
// a .only in a non-test source file is not a focused test → skipped
assert.deepEqual(scanPatchForFocusedTest("src/widget.ts", patch), []);
});

test("scanPatchForFocusedTest: cites the correct new-file line across context and removed lines", () => {
const patch = [
"@@ -1,2 +1,4 @@",
" import { test } from 'node:test';", // context → line 1
"+// setup", // added → line 2
"-const old = 1;", // removed → no new line
"+it.only('focused', () => {});", // added → line 3
"+test('other', () => {});", // added → line 4
].join("\n");
assert.deepEqual(scanPatchForFocusedTest("test/foo.spec.ts", patch), [
{ file: "test/foo.spec.ts", line: 3, kind: "only" },
]);
});

test("scanPatchForFocusedTest: respects the maxFindings cap", () => {
const patch = patchOf([
"it.only('a', () => {});",
"test.only('b', () => {});",
"describe.only('c', () => {});",
]);
assert.equal(scanPatchForFocusedTest("a.test.ts", patch, { maxFindings: 2 }).length, 2);
assert.deepEqual(scanPatchForFocusedTest("a.test.ts", patch, { maxFindings: 0 }), []);
});

test("scanFocusedTest: scans every changed test file, skipping non-test files and files without a patch", async () => {
const findings = await scanFocusedTest({
repoFullName: "o/r",
prNumber: 1,
files: [
{ path: "a.test.ts", patch: patchOf(["it.only('x', () => {});"]) },
{ path: "b.ts", patch: patchOf(["it.only('y', () => {});"]) }, // non-test → skipped
{ path: "c.test.ts", patch: null }, // no patch → skipped
{ path: "d.spec.ts", patch: patchOf(["describe.only('z', () => {});"]) },
],
});
assert.deepEqual(findings, [
{ file: "a.test.ts", line: 1, kind: "only" },
{ file: "d.spec.ts", line: 1, kind: "only" },
]);
});

test("focusedTest renders a section only when there are findings", () => {
const { promptSection } = renderBrief({
focusedTest: [{ file: "a.test.ts", line: 3, kind: "only" }],
});
assert.match(promptSection, /Focused tests/);
assert.match(promptSection, /a\.test\.ts:3/);

const { promptSection: empty } = renderBrief({ focusedTest: [] });
assert.doesNotMatch(empty, /Focused tests/);
});
Loading