Skip to content

Commit 801127a

Browse files
Copilotalexr00
andauthored
fix: include event and workflowName in status check deduplication key
When the same workflow runs for both push and pull_request events, checks with the same name but different events were incorrectly merged during deduplication. Now the deduplication key includes the event and workflowName fields so distinct checks are preserved. Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/588c7904-b943-44bc-842d-b78baf609a0a Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 15e8a90 commit 801127a

File tree

2 files changed

+87
-4
lines changed

2 files changed

+87
-4
lines changed

src/github/githubRepository.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,9 +1916,13 @@ export class GitHubRepository extends Disposable {
19161916
const statusByContext = new Map<string, PullRequestCheckStatus>();
19171917

19181918
for (const status of statuses) {
1919-
const existing = statusByContext.get(status.context);
1919+
// Include event and workflowName in the key so that checks from different
1920+
// workflow events (e.g. "push" vs "pull_request") or different workflows
1921+
// are not incorrectly merged during deduplication.
1922+
const key = `${status.context}\0${status.event ?? ''}\0${status.workflowName ?? ''}`;
1923+
const existing = statusByContext.get(key);
19201924
if (!existing) {
1921-
statusByContext.set(status.context, status);
1925+
statusByContext.set(key, status);
19221926
continue;
19231927
}
19241928

@@ -1928,15 +1932,15 @@ export class GitHubRepository extends Disposable {
19281932

19291933
if (currentIsPending && !existingIsPending) {
19301934
// Current is pending, existing is completed - prefer current
1931-
statusByContext.set(status.context, status);
1935+
statusByContext.set(key, status);
19321936
} else if (!currentIsPending && existingIsPending) {
19331937
// Current is completed, existing is pending - keep existing
19341938
continue;
19351939
} else {
19361940
// Both are same type (both pending or both completed)
19371941
// Prefer the one with a higher ID (more recent), as GitHub IDs are monotonically increasing
19381942
if (status.id > existing.id) {
1939-
statusByContext.set(status.context, status);
1943+
statusByContext.set(key, status);
19401944
}
19411945
}
19421946
}

src/test/github/githubRepository.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { Uri } from 'vscode';
1515
import { MockExtensionContext } from '../mocks/mockExtensionContext';
1616
import { GitHubManager } from '../../authentication/githubServer';
1717
import { GitHubServerType } from '../../common/authentication';
18+
import { CheckState, PullRequestCheckStatus } from '../../github/interface';
1819

1920
describe('GitHubRepository', function () {
2021
let sinon: SinonSandbox;
@@ -52,4 +53,82 @@ describe('GitHubRepository', function () {
5253
// assert(! dotcomRepository.isGitHubDotCom);
5354
});
5455
});
56+
57+
describe('deduplicateStatusChecks', function () {
58+
function createStatus(overrides: Partial<PullRequestCheckStatus> & { id: string; context: string }): PullRequestCheckStatus {
59+
return {
60+
databaseId: undefined,
61+
url: undefined,
62+
avatarUrl: undefined,
63+
state: CheckState.Success,
64+
description: null,
65+
targetUrl: null,
66+
workflowName: undefined,
67+
event: undefined,
68+
isRequired: false,
69+
isCheckRun: true,
70+
...overrides,
71+
};
72+
}
73+
74+
function callDeduplicateStatusChecks(repo: GitHubRepository, statuses: PullRequestCheckStatus[]): PullRequestCheckStatus[] {
75+
return (repo as any).deduplicateStatusChecks(statuses);
76+
}
77+
78+
let repo: GitHubRepository;
79+
80+
beforeEach(function () {
81+
const url = 'https://github.com/some/repo';
82+
const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom);
83+
const rootUri = Uri.file('C:\\users\\test\\repo');
84+
repo = new GitHubRepository(1, remote, rootUri, credentialStore, telemetry);
85+
});
86+
87+
it('keeps checks with different events as separate entries', function () {
88+
const statuses = [
89+
createStatus({ id: '1', context: 'Build Linux / x86-64', event: 'push', workflowName: 'Build Linux' }),
90+
createStatus({ id: '2', context: 'Build Linux / x86-64', event: 'pull_request', workflowName: 'Build Linux' }),
91+
];
92+
const result = callDeduplicateStatusChecks(repo, statuses);
93+
assert.strictEqual(result.length, 2);
94+
});
95+
96+
it('deduplicates checks with the same name, event, and workflow', function () {
97+
const statuses = [
98+
createStatus({ id: '1', context: 'Build Linux / x86-64', event: 'push', workflowName: 'Build Linux', state: CheckState.Success }),
99+
createStatus({ id: '2', context: 'Build Linux / x86-64', event: 'push', workflowName: 'Build Linux', state: CheckState.Success }),
100+
];
101+
const result = callDeduplicateStatusChecks(repo, statuses);
102+
assert.strictEqual(result.length, 1);
103+
assert.strictEqual(result[0].id, '2'); // higher ID preferred
104+
});
105+
106+
it('keeps checks from different workflows as separate entries', function () {
107+
const statuses = [
108+
createStatus({ id: '1', context: 'build', event: 'push', workflowName: 'CI' }),
109+
createStatus({ id: '2', context: 'build', event: 'push', workflowName: 'Nightly' }),
110+
];
111+
const result = callDeduplicateStatusChecks(repo, statuses);
112+
assert.strictEqual(result.length, 2);
113+
});
114+
115+
it('prefers pending checks over completed ones during deduplication', function () {
116+
const statuses = [
117+
createStatus({ id: '1', context: 'test', event: 'push', workflowName: 'CI', state: CheckState.Success }),
118+
createStatus({ id: '2', context: 'test', event: 'push', workflowName: 'CI', state: CheckState.Pending }),
119+
];
120+
const result = callDeduplicateStatusChecks(repo, statuses);
121+
assert.strictEqual(result.length, 1);
122+
assert.strictEqual(result[0].state, CheckState.Pending);
123+
});
124+
125+
it('handles status contexts without event or workflowName', function () {
126+
const statuses = [
127+
createStatus({ id: '1', context: 'ci/jenkins', isCheckRun: false }),
128+
createStatus({ id: '2', context: 'ci/travis', isCheckRun: false }),
129+
];
130+
const result = callDeduplicateStatusChecks(repo, statuses);
131+
assert.strictEqual(result.length, 2);
132+
});
133+
});
55134
});

0 commit comments

Comments
 (0)