diff --git a/web/scripts/check-components-diff-coverage-lib.mjs b/web/scripts/check-components-diff-coverage-lib.mjs index 354424d878..9436bf9453 100644 --- a/web/scripts/check-components-diff-coverage-lib.mjs +++ b/web/scripts/check-components-diff-coverage-lib.mjs @@ -2,6 +2,11 @@ import fs from 'node:fs' import path from 'node:path' const DIFF_COVERAGE_IGNORE_LINE_TOKEN = 'diff-coverage-ignore-line:' +const DEFAULT_BRANCH_REF_CANDIDATES = ['origin/main', 'main'] + +export function normalizeDiffRangeMode(mode) { + return mode === 'exact' ? 'exact' : 'merge-base' +} export function buildGitDiffRevisionArgs(base, head, mode = 'merge-base') { return mode === 'exact' @@ -9,6 +14,46 @@ export function buildGitDiffRevisionArgs(base, head, mode = 'merge-base') { : [`${base}...${head}`] } +export function resolveGitDiffContext({ + base, + head, + mode = 'merge-base', + execGit, +}) { + const requestedMode = normalizeDiffRangeMode(mode) + const context = { + base, + head, + mode: requestedMode, + requestedMode, + reason: null, + useCombinedMergeDiff: false, + } + + if (requestedMode !== 'exact' || !base || !head || !execGit) + return context + + const baseCommit = resolveCommitSha(base, execGit) ?? base + const headCommit = resolveCommitSha(head, execGit) ?? head + const parents = getCommitParents(headCommit, execGit) + if (parents.length < 2) + return context + + const [firstParent, secondParent] = parents + if (firstParent !== baseCommit) + return context + + const defaultBranchRef = resolveDefaultBranchRef(execGit) + if (!defaultBranchRef || !isAncestor(secondParent, defaultBranchRef, execGit)) + return context + + return { + ...context, + reason: `ignored merge from ${defaultBranchRef}`, + useCombinedMergeDiff: true, + } +} + export function parseChangedLineMap(diff, isTrackedComponentSourceFile) { const lineMap = new Map() let currentFile = null @@ -22,7 +67,7 @@ export function parseChangedLineMap(diff, isTrackedComponentSourceFile) { if (!currentFile || !isTrackedComponentSourceFile(currentFile)) continue - const match = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/) + const match = line.match(/^@{2,}(?: -\d+(?:,\d+)?)+ \+(\d+)(?:,(\d+))? @{2,}/) if (!match) continue @@ -220,6 +265,53 @@ function emptyIgnoreResult(changedLines = []) { } } +function getCommitParents(ref, execGit) { + const output = tryExecGit(execGit, ['rev-list', '--parents', '-n', '1', ref]) + if (!output) + return [] + + return output + .trim() + .split(/\s+/) + .slice(1) +} + +function resolveCommitSha(ref, execGit) { + return tryExecGit(execGit, ['rev-parse', '--verify', ref])?.trim() ?? null +} + +function resolveDefaultBranchRef(execGit) { + const originHeadRef = tryExecGit(execGit, ['symbolic-ref', '--quiet', '--short', 'refs/remotes/origin/HEAD'])?.trim() + if (originHeadRef) + return originHeadRef + + for (const ref of DEFAULT_BRANCH_REF_CANDIDATES) { + if (tryExecGit(execGit, ['rev-parse', '--verify', '-q', ref])) + return ref + } + + return null +} + +function isAncestor(ancestorRef, descendantRef, execGit) { + try { + execGit(['merge-base', '--is-ancestor', ancestorRef, descendantRef]) + return true + } + catch { + return false + } +} + +function tryExecGit(execGit, args) { + try { + return execGit(args) + } + catch { + return null + } +} + function getBranchLocations(branch) { return Array.isArray(branch?.locations) ? branch.locations.filter(Boolean) : [] } diff --git a/web/scripts/check-components-diff-coverage-lib.spec.ts b/web/scripts/check-components-diff-coverage-lib.spec.ts new file mode 100644 index 0000000000..4c99193e8e --- /dev/null +++ b/web/scripts/check-components-diff-coverage-lib.spec.ts @@ -0,0 +1,118 @@ +import { describe, expect, it, vi } from 'vitest' +import { parseChangedLineMap, resolveGitDiffContext } from './check-components-diff-coverage-lib.mjs' + +function createExecGitMock(responses: Record) { + return vi.fn((args: string[]) => { + const key = args.join(' ') + const response = responses[key] + + if (response instanceof Error) + throw response + + if (response === undefined) + throw new Error(`Unexpected git args: ${key}`) + + return response + }) +} + +describe('resolveGitDiffContext', () => { + it('switches exact diff to combined merge diff when head merges origin/main into the branch', () => { + const execGit = createExecGitMock({ + 'rev-parse --verify feature-parent-sha': 'feature-parent-sha\n', + 'rev-parse --verify merge-sha': 'merge-sha\n', + 'rev-list --parents -n 1 merge-sha': 'merge-sha feature-parent-sha main-parent-sha\n', + 'symbolic-ref --quiet --short refs/remotes/origin/HEAD': 'origin/main\n', + 'merge-base --is-ancestor main-parent-sha origin/main': '', + }) + + expect(resolveGitDiffContext({ + base: 'feature-parent-sha', + head: 'merge-sha', + mode: 'exact', + execGit, + })).toEqual({ + base: 'feature-parent-sha', + head: 'merge-sha', + mode: 'exact', + requestedMode: 'exact', + reason: 'ignored merge from origin/main', + useCombinedMergeDiff: true, + }) + }) + + it('falls back to origin/main when origin/HEAD is unavailable', () => { + const execGit = createExecGitMock({ + 'rev-parse --verify feature-parent-sha': 'feature-parent-sha\n', + 'rev-parse --verify merge-sha': 'merge-sha\n', + 'rev-list --parents -n 1 merge-sha': 'merge-sha feature-parent-sha main-parent-sha\n', + 'symbolic-ref --quiet --short refs/remotes/origin/HEAD': new Error('missing origin/HEAD'), + 'rev-parse --verify -q origin/main': 'main-tip-sha\n', + 'merge-base --is-ancestor main-parent-sha origin/main': '', + }) + + expect(resolveGitDiffContext({ + base: 'feature-parent-sha', + head: 'merge-sha', + mode: 'exact', + execGit, + })).toEqual({ + base: 'feature-parent-sha', + head: 'merge-sha', + mode: 'exact', + requestedMode: 'exact', + reason: 'ignored merge from origin/main', + useCombinedMergeDiff: true, + }) + }) + + it('keeps exact diff when the second parent is not the default branch', () => { + const execGit = createExecGitMock({ + 'rev-parse --verify feature-parent-sha': 'feature-parent-sha\n', + 'rev-parse --verify merge-sha': 'merge-sha\n', + 'rev-list --parents -n 1 merge-sha': 'merge-sha feature-parent-sha topic-parent-sha\n', + 'symbolic-ref --quiet --short refs/remotes/origin/HEAD': 'origin/main\n', + 'merge-base --is-ancestor topic-parent-sha origin/main': new Error('not ancestor'), + }) + + expect(resolveGitDiffContext({ + base: 'feature-parent-sha', + head: 'merge-sha', + mode: 'exact', + execGit, + })).toEqual({ + base: 'feature-parent-sha', + head: 'merge-sha', + mode: 'exact', + requestedMode: 'exact', + reason: null, + useCombinedMergeDiff: false, + }) + }) +}) + +describe('parseChangedLineMap', () => { + it('parses regular diff hunks', () => { + const diff = [ + 'diff --git a/web/app/components/example.tsx b/web/app/components/example.tsx', + '+++ b/web/app/components/example.tsx', + '@@ -10,0 +11,2 @@', + ].join('\n') + + const changedLineMap = parseChangedLineMap(diff, () => true) + + expect([...changedLineMap.get('web/app/components/example.tsx') ?? []]).toEqual([11, 12]) + }) + + it('parses combined merge diff hunks', () => { + const diff = [ + 'diff --cc web/app/components/example.tsx', + '+++ b/web/app/components/example.tsx', + '@@@ -10,0 -10,0 +11,3 @@@', + ].join('\n') + + const changedLineMap = parseChangedLineMap(diff, () => true) + + expect([...changedLineMap.get('web/app/components/example.tsx') ?? []]).toEqual([11, 12, 13]) + }) +}) diff --git a/web/scripts/check-components-diff-coverage.mjs b/web/scripts/check-components-diff-coverage.mjs index e8822b84d4..e11d21165c 100644 --- a/web/scripts/check-components-diff-coverage.mjs +++ b/web/scripts/check-components-diff-coverage.mjs @@ -6,7 +6,9 @@ import { getChangedBranchCoverage, getChangedStatementCoverage, getIgnoredChangedLinesFromFile, + normalizeDiffRangeMode, parseChangedLineMap, + resolveGitDiffContext, } from './check-components-diff-coverage-lib.mjs' import { COMPONENT_COVERAGE_EXCLUDE_LABEL } from './component-coverage-filters.mjs' import { @@ -20,7 +22,7 @@ import { } from './components-coverage-common.mjs' import { EXCLUDED_COMPONENT_MODULES } from './components-coverage-thresholds.mjs' -const DIFF_RANGE_MODE = process.env.DIFF_RANGE_MODE === 'exact' ? 'exact' : 'merge-base' +const REQUESTED_DIFF_RANGE_MODE = normalizeDiffRangeMode(process.env.DIFF_RANGE_MODE) const EXCLUDED_MODULES_LABEL = [...EXCLUDED_COMPONENT_MODULES].sort().join(', ') const repoRoot = repoRootFromCwd() @@ -43,8 +45,14 @@ if (!fs.existsSync(coverageFinalPath)) { process.exit(1) } +const diffContext = resolveGitDiffContext({ + base: baseSha, + head: headSha, + mode: REQUESTED_DIFF_RANGE_MODE, + execGit, +}) const coverage = JSON.parse(fs.readFileSync(coverageFinalPath, 'utf8')) -const changedFiles = getChangedFiles(baseSha, headSha) +const changedFiles = getChangedFiles(diffContext) const changedComponentSourceFiles = changedFiles.filter(isAnyComponentSourceFile) const changedSourceFiles = changedComponentSourceFiles.filter(filePath => isTrackedComponentSourceFile(filePath, context.excludedComponentCoverageFiles)) const changedExcludedSourceFiles = changedComponentSourceFiles.filter(filePath => isExcludedComponentSourceFile(filePath, context.excludedComponentCoverageFiles)) @@ -55,7 +63,7 @@ if (changedSourceFiles.length === 0) { } const coverageEntries = loadTrackedCoverageEntries(coverage, context) -const diffChanges = getChangedLineMap(baseSha, headSha) +const diffChanges = getChangedLineMap(diffContext) const diffRows = [] const ignoredDiffLines = [] const invalidIgnorePragmas = [] @@ -109,6 +117,7 @@ const diffBranchFailures = diffRows.filter(row => row.branches.uncoveredBranches appendSummary(buildSummary({ changedSourceFiles, + diffContext, diffBranchFailures, diffRows, diffStatementFailures, @@ -144,6 +153,7 @@ if ( function buildSummary({ changedSourceFiles, + diffContext, diffBranchFailures, diffRows, diffStatementFailures, @@ -154,8 +164,7 @@ function buildSummary({ const lines = [ '### app/components Pure Diff Coverage', '', - `Compared \`${baseSha.slice(0, 12)}\` -> \`${headSha.slice(0, 12)}\``, - `Diff range mode: \`${DIFF_RANGE_MODE}\``, + ...buildDiffContextSummary(diffContext), '', `Excluded modules: \`${EXCLUDED_MODULES_LABEL}\``, `Excluded file kinds: \`${COMPONENT_COVERAGE_EXCLUDE_LABEL}\``, @@ -223,6 +232,8 @@ function buildSkipSummary(changedExcludedSourceFiles) { const lines = [ '### app/components Pure Diff Coverage', '', + ...buildDiffContextSummary(diffContext), + '', `Excluded modules: \`${EXCLUDED_MODULES_LABEL}\``, `Excluded file kinds: \`${COMPONENT_COVERAGE_EXCLUDE_LABEL}\``, '', @@ -239,16 +250,49 @@ function buildSkipSummary(changedExcludedSourceFiles) { return lines } -function getChangedFiles(base, head) { - const output = execGit(['diff', '--name-only', '--diff-filter=ACMR', ...buildGitDiffRevisionArgs(base, head, DIFF_RANGE_MODE), '--', APP_COMPONENTS_PREFIX]) +function buildDiffContextSummary(diffContext) { + const lines = [ + `Compared \`${diffContext.base.slice(0, 12)}\` -> \`${diffContext.head.slice(0, 12)}\``, + ] + + if (diffContext.useCombinedMergeDiff) { + lines.push(`Requested diff range mode: \`${diffContext.requestedMode}\``) + lines.push(`Effective diff strategy: \`combined-merge\` (${diffContext.reason})`) + } + else if (diffContext.reason) { + lines.push(`Requested diff range mode: \`${diffContext.requestedMode}\``) + lines.push(`Effective diff range mode: \`${diffContext.mode}\` (${diffContext.reason})`) + } + else { + lines.push(`Diff range mode: \`${diffContext.mode}\``) + } + + return lines +} + +function getChangedFiles(diffContext) { + if (diffContext.useCombinedMergeDiff) { + const output = execGit(['diff-tree', '--cc', '--no-commit-id', '--name-only', '-r', diffContext.head, '--', APP_COMPONENTS_PREFIX]) + return output + .split('\n') + .map(line => line.trim()) + .filter(Boolean) + } + + const output = execGit(['diff', '--name-only', '--diff-filter=ACMR', ...buildGitDiffRevisionArgs(diffContext.base, diffContext.head, diffContext.mode), '--', APP_COMPONENTS_PREFIX]) return output .split('\n') .map(line => line.trim()) .filter(Boolean) } -function getChangedLineMap(base, head) { - const diff = execGit(['diff', '--unified=0', '--no-color', '--diff-filter=ACMR', ...buildGitDiffRevisionArgs(base, head, DIFF_RANGE_MODE), '--', APP_COMPONENTS_PREFIX]) +function getChangedLineMap(diffContext) { + if (diffContext.useCombinedMergeDiff) { + const diff = execGit(['diff-tree', '--cc', '--no-commit-id', '-r', '--unified=0', diffContext.head, '--', APP_COMPONENTS_PREFIX]) + return parseChangedLineMap(diff, filePath => isTrackedComponentSourceFile(filePath, context.excludedComponentCoverageFiles)) + } + + const diff = execGit(['diff', '--unified=0', '--no-color', '--diff-filter=ACMR', ...buildGitDiffRevisionArgs(diffContext.base, diffContext.head, diffContext.mode), '--', APP_COMPONENTS_PREFIX]) return parseChangedLineMap(diff, filePath => isTrackedComponentSourceFile(filePath, context.excludedComponentCoverageFiles)) } diff --git a/web/scripts/report-components-test-touch.mjs b/web/scripts/report-components-test-touch.mjs index 0d384c2f4a..43f316e39a 100644 --- a/web/scripts/report-components-test-touch.mjs +++ b/web/scripts/report-components-test-touch.mjs @@ -2,6 +2,8 @@ import { execFileSync } from 'node:child_process' import fs from 'node:fs' import { buildGitDiffRevisionArgs, + normalizeDiffRangeMode, + resolveGitDiffContext, } from './check-components-diff-coverage-lib.mjs' import { createComponentCoverageContext, @@ -10,7 +12,7 @@ import { isTrackedComponentSourceFile, } from './components-coverage-common.mjs' -const DIFF_RANGE_MODE = process.env.DIFF_RANGE_MODE === 'exact' ? 'exact' : 'merge-base' +const REQUESTED_DIFF_RANGE_MODE = normalizeDiffRangeMode(process.env.DIFF_RANGE_MODE) const repoRoot = repoRootFromCwd() const context = createComponentCoverageContext(repoRoot) @@ -26,13 +28,21 @@ if (!baseSha || /^0+$/.test(baseSha)) { process.exit(0) } -const changedFiles = getChangedFiles(baseSha, headSha) +const diffContext = resolveGitDiffContext({ + base: baseSha, + head: headSha, + mode: REQUESTED_DIFF_RANGE_MODE, + execGit, +}) +const changedFiles = getChangedFiles(diffContext) const changedSourceFiles = changedFiles.filter(filePath => isTrackedComponentSourceFile(filePath, context.excludedComponentCoverageFiles)) if (changedSourceFiles.length === 0) { appendSummary([ '### app/components Test Touch', '', + ...buildDiffContextSummary(diffContext), + '', 'No tracked source changes under `web/app/components/`. Test-touch report skipped.', ]) process.exit(0) @@ -45,6 +55,7 @@ const totalChangedWebTests = [...new Set([...changedRelevantTestFiles, ...change appendSummary(buildSummary({ changedOtherWebTestFiles, changedRelevantTestFiles, + diffContext, changedSourceFiles, totalChangedWebTests, })) @@ -52,14 +63,14 @@ appendSummary(buildSummary({ function buildSummary({ changedOtherWebTestFiles, changedRelevantTestFiles, + diffContext, changedSourceFiles, totalChangedWebTests, }) { const lines = [ '### app/components Test Touch', '', - `Compared \`${baseSha.slice(0, 12)}\` -> \`${headSha.slice(0, 12)}\``, - `Diff range mode: \`${DIFF_RANGE_MODE}\``, + ...buildDiffContextSummary(diffContext), '', `Tracked source files changed: ${changedSourceFiles.length}`, `Component-local or shared integration tests changed: ${changedRelevantTestFiles.length}`, @@ -99,8 +110,36 @@ function buildSummary({ return lines } -function getChangedFiles(base, head) { - const output = execGit(['diff', '--name-only', '--diff-filter=ACMR', ...buildGitDiffRevisionArgs(base, head, DIFF_RANGE_MODE), '--', 'web']) +function buildDiffContextSummary(diffContext) { + const lines = [ + `Compared \`${diffContext.base.slice(0, 12)}\` -> \`${diffContext.head.slice(0, 12)}\``, + ] + + if (diffContext.useCombinedMergeDiff) { + lines.push(`Requested diff range mode: \`${diffContext.requestedMode}\``) + lines.push(`Effective diff strategy: \`combined-merge\` (${diffContext.reason})`) + } + else if (diffContext.reason) { + lines.push(`Requested diff range mode: \`${diffContext.requestedMode}\``) + lines.push(`Effective diff range mode: \`${diffContext.mode}\` (${diffContext.reason})`) + } + else { + lines.push(`Diff range mode: \`${diffContext.mode}\``) + } + + return lines +} + +function getChangedFiles(diffContext) { + if (diffContext.useCombinedMergeDiff) { + const output = execGit(['diff-tree', '--cc', '--no-commit-id', '--name-only', '-r', diffContext.head, '--', 'web']) + return output + .split('\n') + .map(line => line.trim()) + .filter(Boolean) + } + + const output = execGit(['diff', '--name-only', '--diff-filter=ACMR', ...buildGitDiffRevisionArgs(diffContext.base, diffContext.head, diffContext.mode), '--', 'web']) return output .split('\n') .map(line => line.trim())