Skip to content
Draft
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
14 changes: 14 additions & 0 deletions src/github/folderRepositoryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2099,6 +2099,20 @@ export class FolderRepositoryManager extends Disposable {
const deleteConfig = async (branch: string) => {
await PullRequestGitHelper.associateBaseBranchWithBranch(this.repository, branch, undefined);
await PullRequestGitHelper.associateBranchWithPullRequest(this.repository, undefined, branch);
// Git normally removes the entire [branch "<name>"] section when a branch is deleted, but if the
// branch no longer exists the leftover section keeps the branch showing up in this list. Remove any
// remaining branch.<name>.* config entries so already-deleted branches don't reappear.
if (this.repository.unsetConfig) {
const prefix = `branch.${branch}.`;
const remaining = (await this.repository.getConfigs()).filter(config => config.key.startsWith(prefix));
for (const config of remaining) {
try {
await this.repository.unsetConfig(config.key);
} catch (e) {
Logger.error(`Failed to remove leftover git config ${config.key}: ${e}`, this.id);
}
}
}
};

// delete configs first since that can't be parallelized
Expand Down
35 changes: 34 additions & 1 deletion src/test/github/folderRepositoryManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ describe('PullRequestManager', function () {
let manager: FolderRepositoryManager;
let telemetry: MockTelemetry;
let mockThemeWatcher: MockThemeWatcher;
let repository: MockRepository;

beforeEach(function () {
sinon = createSandbox();
MockCommandRegistry.install(sinon);

telemetry = new MockTelemetry();
mockThemeWatcher = new MockThemeWatcher();
const repository = new MockRepository();
repository = new MockRepository();
const context = new MockExtensionContext();
const credentialStore = new CredentialStore(telemetry, context);
const repositoriesManager = new RepositoriesManager(credentialStore, telemetry);
Expand Down Expand Up @@ -68,6 +69,38 @@ describe('PullRequestManager', function () {
assert.deepStrictEqual(manager.activePullRequest, pr);
});
});

describe('deleteBranches', function () {
const noopProgress = { report: () => { } };

it('removes leftover branch config after deleting a branch', async function () {
await repository.createBranch('feature', false, 'commit-hash');
await repository.setConfig('branch.feature.remote', 'origin');
await repository.setConfig('branch.feature.merge', 'refs/heads/feature');
await repository.setConfig('branch.feature.github-pr-owner-number', 'owner#repo#1');

const nonExistant = new Set<string>();
await (manager as any).deleteBranches([{ label: 'feature' }], nonExistant, noopProgress, 1, 0, []);

const configs = await repository.getConfigs();
assert.strictEqual(configs.filter(c => c.key.startsWith('branch.feature.')).length, 0);
assert.strictEqual(nonExistant.has('feature'), false);
});

it('removes leftover branch config for a branch that no longer exists', async function () {
// The branch ref is already gone, but stale [branch "gone"] config remains.
await repository.setConfig('branch.gone.remote', 'origin');
await repository.setConfig('branch.gone.merge', 'refs/heads/gone');
await repository.setConfig('branch.gone.github-pr-owner-number', 'owner#repo#2');

const nonExistant = new Set<string>();
await (manager as any).deleteBranches([{ label: 'gone' }], nonExistant, noopProgress, 1, 0, []);

const configs = await repository.getConfigs();
assert.strictEqual(configs.filter(c => c.key.startsWith('branch.gone.')).length, 0);
assert.strictEqual(nonExistant.has('gone'), true);
});
});
});

describe('titleAndBodyFrom', function () {
Expand Down
10 changes: 9 additions & 1 deletion src/test/mocks/mockRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ export class MockRepository implements Repository {
return oldValue;
}

async unsetConfig(key: string): Promise<string> {
const oldValue = this._config.get(key) || '';
this._config.delete(key);
return oldValue;
}

getObjectDetails(treeish: string, treePath: string): Promise<{ mode: string; object: string; size: number }> {
return Promise.reject(new Error(`Unexpected getObjectDetails(${treeish}, ${treePath})`));
}
Expand Down Expand Up @@ -181,7 +187,9 @@ export class MockRepository implements Repository {
async deleteBranch(name: string, force?: boolean | undefined): Promise<void> {
const index = this._branches.findIndex(b => b.name === name);
if (index === -1) {
throw new Error(`Attempt to delete nonexistent branch ${name}`);
const error: Error & { stderr?: string } = new Error(`Attempt to delete nonexistent branch ${name}`);
error.stderr = `error: branch '${name}' not found.`;
throw error;
}
this._branches.splice(index, 1);
}
Expand Down