mirror of https://github.com/langgenius/dify.git
test(web): ignore default-branch merge commits in diff coverage (#33564)
Co-authored-by: CodingOnStar <hanxujiang@dify.com>
This commit is contained in:
parent
7e34faaf51
commit
9e2123c655
|
|
@ -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) : []
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<string, string | Error>) {
|
||||
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])
|
||||
})
|
||||
})
|
||||
|
|
@ -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))
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
|
|
|
|||
Loading…
Reference in New Issue