From ace61869d0688b1fa833e1239d0b2a67d247d824 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 30 Apr 2026 21:57:27 -0400 Subject: [PATCH] test(#2916): parameterize fixtures so both main and trunk are exercised Two follow-ups on commit 80f14cac (which hardened quick-branching with a trunk fixture): 1. quick-branching.test.cjs: add a `defaultBranch` parameter to setupFixture and run the "branches off origin/HEAD" assertion against both `main` and `trunk`. The wholesale switch to trunk in 80f14cac removed coverage of the conventional `main` path; parameterizing restores it without giving up the symbolic-ref guarantee. 2. bug-2916-handle-branching-default-base.test.cjs: apply the same parameterization here. handle_branching has the same default-branch detection logic as Step 2.5, so it deserves the same trunk regression guard. Previously this file only exercised `main`. A regression that silently defaults to `main` instead of consulting `git symbolic-ref refs/remotes/origin/HEAD` now fails the `trunk` variant in both files. Tests: 10/10 in the touched suites. --- ...916-handle-branching-default-base.test.cjs | 99 ++++++++++--------- tests/quick-branching.test.cjs | 89 +++++++++-------- 2 files changed, 103 insertions(+), 85 deletions(-) diff --git a/tests/bug-2916-handle-branching-default-base.test.cjs b/tests/bug-2916-handle-branching-default-base.test.cjs index bfaca64f6..a51cbf2c0 100644 --- a/tests/bug-2916-handle-branching-default-base.test.cjs +++ b/tests/bug-2916-handle-branching-default-base.test.cjs @@ -108,39 +108,44 @@ function extractHandleBranchingBash() { } /** - * Build a fixture: a bare "origin" repo with `main` (one commit), a clone with - * `origin/HEAD` pointed at `main`, and a checked-out previous-phase branch - * carrying its own unmerged commit. Returns the clone path. + * Build a fixture: a bare "origin" repo with the named default branch (one + * commit), a clone with `origin/HEAD` pointed at it, and a checked-out + * previous-phase branch carrying its own unmerged commit. + * + * `defaultBranch` is parameterized so callers can lock in that the workflow + * honors `git symbolic-ref refs/remotes/origin/HEAD` rather than silently + * defaulting to `main` (#2921 CR feedback — quick-branching.test.cjs got the + * same treatment in 80f14cac; this test deserves the same coverage). */ -function setupFixture() { +function setupFixture(defaultBranch = 'main') { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2916-')); const seedPath = path.join(root, 'seed'); const originPath = path.join(root, 'origin.git'); const clonePath = path.join(root, 'clone'); fs.mkdirSync(seedPath); - git(seedPath, 'init', '-b', 'main'); + git(seedPath, 'init', '-b', defaultBranch); git(seedPath, 'config', 'commit.gpgsign', 'false'); fs.writeFileSync(path.join(seedPath, 'README.md'), '# seed\n'); git(seedPath, 'add', 'README.md'); git(seedPath, 'commit', '-m', 'initial'); git(root, 'clone', '--bare', seedPath, originPath); - git(originPath, 'symbolic-ref', 'HEAD', 'refs/heads/main'); + git(originPath, 'symbolic-ref', 'HEAD', `refs/heads/${defaultBranch}`); git(root, 'clone', originPath, clonePath); git(clonePath, 'config', 'commit.gpgsign', 'false'); git(clonePath, 'config', 'user.email', 'test@test.com'); git(clonePath, 'config', 'user.name', 'Test'); - // Simulate finishing a previous phase: branch off main, add a commit, and - // *stay* on it (this is the failure scenario described in the bug). + // Simulate finishing a previous phase: branch off the default branch, add + // a commit, and *stay* on it (the failure scenario described in the bug). git(clonePath, 'checkout', '-b', 'feature/phase-01-foundation'); fs.writeFileSync(path.join(clonePath, 'phase01.txt'), 'phase 1 work\n'); git(clonePath, 'add', 'phase01.txt'); git(clonePath, 'commit', '-m', 'phase 01 work'); - return { root, clonePath }; + return { root, clonePath, defaultBranch }; } function runHandleBranchingStep(bash, cwd, branchName) { @@ -163,45 +168,51 @@ function runHandleBranchingStep(bash, cwd, branchName) { } describe('handle_branching branches off origin/HEAD, not current HEAD (#2916)', () => { - test('new phase branch contains 0 commits inherited from previous-phase HEAD', () => { - const bash = extractHandleBranchingBash(); - const { root, clonePath } = setupFixture(); + // Run against `main` (conventional default) and `trunk` (non-main default + // exercising the symbolic-ref code path) so a regression that hard-codes + // `main` instead of consulting origin/HEAD will fail the trunk variant. + for (const defaultBranch of ['main', 'trunk']) { + test(`new phase branch branches off origin/${defaultBranch} with 0 inherited commits`, () => { + const bash = extractHandleBranchingBash(); + const { root, clonePath } = setupFixture(defaultBranch); - try { - // Sanity: we begin sitting on the previous phase branch, 1 ahead of origin/main. - assert.equal( - git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), - 'feature/phase-01-foundation' - ); - assert.equal( - git(clonePath, 'rev-list', '--count', 'origin/main..HEAD'), - '1', - 'fixture should be 1 commit ahead of origin/main' - ); + try { + const upstream = `origin/${defaultBranch}`; - runHandleBranchingStep(bash, clonePath, 'feature/phase-02-content-sync'); + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'feature/phase-01-foundation' + ); + assert.equal( + git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`), + '1', + `fixture should be 1 commit ahead of ${upstream}` + ); - assert.equal( - git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), - 'feature/phase-02-content-sync', - 'handle_branching should switch to the new phase branch' - ); + runHandleBranchingStep(bash, clonePath, 'feature/phase-02-content-sync'); - const inherited = git(clonePath, 'rev-list', '--count', 'origin/main..HEAD'); - assert.equal( - inherited, - '0', - `new phase branch must branch off origin/main, but inherited ${inherited} commit(s) from previous-phase HEAD` - ); - assert.equal( - git(clonePath, 'rev-parse', 'HEAD'), - git(clonePath, 'rev-parse', 'origin/main'), - 'new phase branch tip must equal origin/main tip' - ); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } - }); + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'feature/phase-02-content-sync', + 'handle_branching should switch to the new phase branch' + ); + + const inherited = git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`); + assert.equal( + inherited, + '0', + `new phase branch must branch off ${upstream}, but inherited ${inherited} commit(s) from previous-phase HEAD` + ); + assert.equal( + git(clonePath, 'rev-parse', 'HEAD'), + git(clonePath, 'rev-parse', upstream), + `new phase branch tip must equal ${upstream} tip` + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + } test('handle_branching reuses an existing branch instead of forking again', () => { const bash = extractHandleBranchingBash(); diff --git a/tests/quick-branching.test.cjs b/tests/quick-branching.test.cjs index 9c4fed373..d0ac0dacb 100644 --- a/tests/quick-branching.test.cjs +++ b/tests/quick-branching.test.cjs @@ -103,35 +103,35 @@ function extractStep25Bash() { * implementation skips `git symbolic-ref refs/remotes/origin/HEAD` and just * defaults to `main`, every assertion below collapses (#2921 CR nitpick). */ -function setupFixture() { +function setupFixture(defaultBranch = 'trunk') { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-quick-branching-')); const seedPath = path.join(root, 'seed'); const originPath = path.join(root, 'origin.git'); const clonePath = path.join(root, 'clone'); fs.mkdirSync(seedPath); - git(seedPath, 'init', '-b', 'trunk'); + git(seedPath, 'init', '-b', defaultBranch); git(seedPath, 'config', 'commit.gpgsign', 'false'); fs.writeFileSync(path.join(seedPath, 'README.md'), '# seed\n'); git(seedPath, 'add', 'README.md'); git(seedPath, 'commit', '-m', 'initial'); git(root, 'clone', '--bare', seedPath, originPath); - git(originPath, 'symbolic-ref', 'HEAD', 'refs/heads/trunk'); + git(originPath, 'symbolic-ref', 'HEAD', `refs/heads/${defaultBranch}`); git(root, 'clone', originPath, clonePath); git(clonePath, 'config', 'commit.gpgsign', 'false'); git(clonePath, 'config', 'user.email', 'test@test.com'); git(clonePath, 'config', 'user.name', 'Test'); - // Simulate finishing a previous quick task: branch off trunk, add a commit, - // and stay on it (this is the failure scenario from #2916). + // Simulate finishing a previous quick task: branch off the default branch, + // add a commit, and stay on it (this is the failure scenario from #2916). git(clonePath, 'checkout', '-b', 'quick/01-prev-task'); fs.writeFileSync(path.join(clonePath, 'prev.txt'), 'prev work\n'); git(clonePath, 'add', 'prev.txt'); git(clonePath, 'commit', '-m', 'prev quick task work'); - return { root, clonePath }; + return { root, clonePath, defaultBranch }; } function runStep(bash, cwd, branchName) { @@ -214,45 +214,52 @@ describe('quick workflow: branching support', () => { ); }); - test('new quick-task branch contains 0 commits inherited from previous-task HEAD (#2916)', () => { - const bash = extractStep25Bash(); - const { root, clonePath } = setupFixture(); + // Run against both `main` (the conventional default) and `trunk` (a non- + // main default that exercises the symbolic-ref code path). Keeping both + // restores main coverage that was removed when the fixture switched + // wholesale to trunk in 80f14cac. + for (const defaultBranch of ['main', 'trunk']) { + test(`new quick-task branch branches off origin/${defaultBranch} (#2916)`, () => { + const bash = extractStep25Bash(); + const { root, clonePath } = setupFixture(defaultBranch); - try { - // Sanity: we begin sitting on the previous quick-task branch, 1 ahead of origin/trunk. - assert.equal( - git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), - 'quick/01-prev-task' - ); - assert.equal( - git(clonePath, 'rev-list', '--count', 'origin/trunk..HEAD'), - '1', - 'fixture should be 1 commit ahead of origin/trunk' - ); + try { + const upstream = `origin/${defaultBranch}`; - runStep(bash, clonePath, 'quick/02-new-task'); + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'quick/01-prev-task' + ); + assert.equal( + git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`), + '1', + `fixture should be 1 commit ahead of ${upstream}` + ); - assert.equal( - git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), - 'quick/02-new-task', - 'Step 2.5 should switch to the new quick-task branch' - ); + runStep(bash, clonePath, 'quick/02-new-task'); - const inherited = git(clonePath, 'rev-list', '--count', 'origin/trunk..HEAD'); - assert.equal( - inherited, - '0', - `new quick-task branch must branch off origin/trunk, but inherited ${inherited} commit(s) from previous-task HEAD` - ); - assert.equal( - git(clonePath, 'rev-parse', 'HEAD'), - git(clonePath, 'rev-parse', 'origin/trunk'), - 'new quick-task branch tip must equal origin/trunk tip' - ); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } - }); + assert.equal( + git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'), + 'quick/02-new-task', + 'Step 2.5 should switch to the new quick-task branch' + ); + + const inherited = git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`); + assert.equal( + inherited, + '0', + `new quick-task branch must branch off ${upstream}, but inherited ${inherited} commit(s) from previous-task HEAD` + ); + assert.equal( + git(clonePath, 'rev-parse', 'HEAD'), + git(clonePath, 'rev-parse', upstream), + `new quick-task branch tip must equal ${upstream} tip` + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + } test('Step 2.5 reuses an existing quick-task branch instead of forking again', () => { const bash = extractStep25Bash();