mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
fix: judge the branch rule only by the branch's own commits
Check 2 compared the Issue trailers against whatever branch the working tree happened to be on, over whatever range it was given. Those two are not the same set. After a rebase the CI range widens — `before` points at a discarded commit, the merge-base slides back, and commits that belong to dev arrive carrying other issue numbers. Every one of them then looks like a violation. Running the gate over real history from issue/89 with a dev range produced 26 false refusals out of 26 commits, which would have reddened Validate on the next force-push of any task branch. The rule now reads origin/dev..HEAD for its own verdict and leaves the event range to the other checks. A commit that genuinely carries the wrong trailer for its branch is still caught; the integration test covers both directions. Issue: #105 User-Visible: no
This commit is contained in:
@@ -189,7 +189,13 @@ export function evaluateCommit(c) {
|
|||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
// 2. имя ветки issue/NN-slug соответствует трейлерам
|
// 2. имя ветки issue/NN-slug соответствует трейлерам.
|
||||||
|
//
|
||||||
|
// Судить можно только коммиты САМОЙ ветки. Диапазон, который приходит из события
|
||||||
|
// CI, шире: после ребейза `before` указывает на снесённый коммит, merge-base
|
||||||
|
// уезжает назад, и в диапазон попадают коммиты `dev` с чужими номерами issue —
|
||||||
|
// каждый из них выглядел бы нарушением. Проверено на реальной истории: сидя на
|
||||||
|
// issue/89 с диапазоном по dev, гейт дал 26 ложных отказов из 26 коммитов.
|
||||||
export function checkBranchRule(branch, commits) {
|
export function checkBranchRule(branch, commits) {
|
||||||
const m = (branch ?? '').match(/^issue\/(\d+)-/);
|
const m = (branch ?? '').match(/^issue\/(\d+)-/);
|
||||||
if (!m) return [];
|
if (!m) return [];
|
||||||
@@ -397,7 +403,20 @@ function main(argv) {
|
|||||||
|
|
||||||
const findings = [];
|
const findings = [];
|
||||||
for (const c of commits) findings.push(...evaluateCommit(c));
|
for (const c of commits) findings.push(...evaluateCommit(c));
|
||||||
findings.push(...checkBranchRule(branch, commits));
|
|
||||||
|
// Проверке 2 отдаются только коммиты самой ветки: origin/dev..HEAD, а не
|
||||||
|
// диапазон события. См. комментарий у checkBranchRule.
|
||||||
|
const ownCommits = /^issue\/\d+-/.test(branch)
|
||||||
|
? (() => {
|
||||||
|
const hasDev = spawnSync('git', ['-C', repo, 'rev-parse', '--verify', 'origin/dev'],
|
||||||
|
{ encoding: 'utf8' }).status === 0;
|
||||||
|
if (!hasDev) return commits;
|
||||||
|
return parseRecords(
|
||||||
|
git(['log', '--reverse', `--pretty=format:${LOG_FORMAT}`, 'origin/dev..HEAD'], repo),
|
||||||
|
);
|
||||||
|
})()
|
||||||
|
: commits;
|
||||||
|
findings.push(...checkBranchRule(branch, ownCommits));
|
||||||
|
|
||||||
// Метки читаются один раз и используются дважды: проверкой 8 и escalation
|
// Метки читаются один раз и используются дважды: проверкой 8 и escalation
|
||||||
// проверки 3. Второй запрос по тому же issue — лишний сетевой вызов.
|
// проверки 3. Второй запрос по тому же issue — лишний сетевой вызов.
|
||||||
|
|||||||
@@ -287,6 +287,38 @@ test('the CLI exits 0 on a clean range and 1 on a broken one', (t) => {
|
|||||||
const parsed = JSON.parse(asJson.stdout);
|
const parsed = JSON.parse(asJson.stdout);
|
||||||
assert.equal(parsed.ok, false);
|
assert.equal(parsed.ok, false);
|
||||||
assert.equal(parsed.commits, 2);
|
assert.equal(parsed.commits, 2);
|
||||||
|
|
||||||
|
// Проверка 2 судит только коммиты самой ветки. Диапазон из события CI шире:
|
||||||
|
// после ребейза merge-base уезжает назад и втягивает коммиты dev с чужими
|
||||||
|
// номерами issue. На реальной истории это давало 26 ложных отказов из 26.
|
||||||
|
git('checkout', '-q', 'dev');
|
||||||
|
git('reset', '-q', '--hard', base);
|
||||||
|
write('src/on-dev.ts', 'export const d = 1;\n');
|
||||||
|
write('docs/CHANGELOG.md', 'ru\n');
|
||||||
|
write('docs/CHANGELOG.ru.md', 'en\n');
|
||||||
|
commitAll('Land on dev\n\nIssue: #1\nUser-Visible: yes');
|
||||||
|
const devTip = git('rev-parse', 'HEAD').trim();
|
||||||
|
git('update-ref', 'refs/remotes/origin/dev', devTip);
|
||||||
|
|
||||||
|
git('checkout', '-q', '-b', 'issue/2-own-work');
|
||||||
|
write('src/on-branch.ts', 'export const b = 1;\n');
|
||||||
|
commitAll('Work on the task branch\n\nIssue: #2\nUser-Visible: no');
|
||||||
|
|
||||||
|
// Диапазон намеренно захватывает коммит dev про #1, пока HEAD на ветке #2.
|
||||||
|
const spanning = spawnSync(process.execPath,
|
||||||
|
[gate, '--repo', dir, '--range', `${base}..HEAD`, '--json'], { encoding: 'utf8' });
|
||||||
|
const spanned = JSON.parse(spanning.stdout);
|
||||||
|
assert.equal(spanned.commits, 2);
|
||||||
|
assert.deepEqual(spanned.findings.filter((f) => f.rule === 2), [], spanning.stdout);
|
||||||
|
assert.equal(spanned.ok, true, spanning.stdout);
|
||||||
|
|
||||||
|
// А своё же нарушение ветка по-прежнему получает.
|
||||||
|
write('src/wrong-issue.ts', 'export const w = 1;\n');
|
||||||
|
commitAll('Wrong trailer for this branch\n\nIssue: #3\nUser-Visible: no');
|
||||||
|
const wrong = spawnSync(process.execPath,
|
||||||
|
[gate, '--repo', dir, '--range', `${devTip}..HEAD`, '--json'], { encoding: 'utf8' });
|
||||||
|
const wrongReport = JSON.parse(wrong.stdout);
|
||||||
|
assert.equal(wrongReport.findings.some((f) => f.rule === 2), true, wrong.stdout);
|
||||||
} finally {
|
} finally {
|
||||||
rmSync(dir, { recursive: true, force: true });
|
rmSync(dir, { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user