Disambiguate builders with the same name using project/bucket
The plugin incorrectly assumes that builder name is unique across
projects and buckets. Use project and bucket name for builders that have
the same name so users can tell them apart.
Bug: 341807205
Change-Id: I11f3632459de7f19023a0dabc144ac13601acece
Reviewed-on: https://chromium-review.googlesource.com/c/infra/gerrit-plugins/buildbucket/+/6997246
Reviewed-by: Scott Lee <ddoman@chromium.org>
diff --git a/web/checks-fetcher.ts b/web/checks-fetcher.ts
index c018b05..005596a 100644
--- a/web/checks-fetcher.ts
+++ b/web/checks-fetcher.ts
@@ -794,7 +794,7 @@
status: this.getRunStatusFromBuild(attempt),
statusDescription: this.getRunStatusDescFromBuild(attempt),
statusLink: createBuildUrl(this.buildbucketHost, attempt.id),
- actions: [this.copyNameAction(attempt.builder.builder!)],
+ actions: [this.copyNameAction(builder)],
scheduledTimestamp: getDateFromTimestamp(attempt.createTime),
startedTimestamp: getDateFromTimestamp(attempt.startTime),
finishedTimestamp: getDateFromTimestamp(attempt.endTime),
@@ -815,13 +815,12 @@
links: this.createBuildResultLinks(attempt),
});
- const builderName = attempt.builder.builder;
if (
currAttemptNum === finalBuilderAttempts.length &&
- hiddenRootBuilders[builderName!]
+ hiddenRootBuilders[builder]
) {
const links: Link[] = [];
- hiddenRootBuilders[builderName!].forEach(hiddenAttempt => {
+ hiddenRootBuilders[builder].forEach(hiddenAttempt => {
const link = this.createBuildResultLinks(hiddenAttempt);
link[0].primary = false;
links.push(link[0]);
@@ -1279,12 +1278,38 @@
*/
createAttemptList(builds: Build[]): {[key: string]: Build[]} {
const dct: {[key: string]: Build[]} = {};
+
+ // Identify ambiguous builder names.
+ const builderNameMap = new Map<string, Set<string>>();
+ const ambiguousNames = new Set<string>();
+
builds.forEach(build => {
- const name = build.builder.builder!;
- if (!(name in dct)) {
- dct[name] = [build];
+ const builderName = build.builder.builder!;
+ const uniqueLocation = `${build.builder.project}/${build.builder.bucket}`;
+
+ if (!builderNameMap.has(builderName)) {
+ builderNameMap.set(builderName, new Set());
+ }
+ builderNameMap.get(builderName)!.add(uniqueLocation);
+ });
+
+ for (const [builderName, locations] of builderNameMap.entries()) {
+ if (locations.size > 1) {
+ ambiguousNames.add(builderName);
+ }
+ }
+
+ builds.forEach(build => {
+ const builderName = build.builder.builder!;
+ let key = builderName;
+ if (ambiguousNames.has(builderName)) {
+ key = `${build.builder.project}/${build.builder.bucket}/${builderName}`;
+ }
+
+ if (!(key in dct)) {
+ dct[key] = [build];
} else {
- dct[name].push(build);
+ dct[key].push(build);
}
});
Object.values(dct).forEach(arr => {
diff --git a/web/test/checks-fetcher_test.ts b/web/test/checks-fetcher_test.ts
index 2fd3f15..d7699b9 100644
--- a/web/test/checks-fetcher_test.ts
+++ b/web/test/checks-fetcher_test.ts
@@ -647,6 +647,88 @@
);
});
+ test('createAttemptList handles ambiguous builder names', () => {
+ const buildUnique: Build = {
+ id: '1',
+ builder: {project: 'p1', bucket: 'b1', builder: 'unique-builder'},
+ status: BuildStatus.SUCCESS,
+ createTime: '',
+ startTime: '',
+ endTime: '',
+ };
+ const buildAmbiguous1: Build = {
+ id: '4',
+ builder: {project: 'p1', bucket: 'b1', builder: 'ambiguous-builder'},
+ status: BuildStatus.SUCCESS,
+ createTime: '',
+ startTime: '',
+ endTime: '',
+ };
+ const buildAmbiguous2: Build = {
+ id: '3',
+ builder: {project: 'p2', bucket: 'b2', builder: 'ambiguous-builder'},
+ status: BuildStatus.SUCCESS,
+ createTime: '',
+ startTime: '',
+ endTime: '',
+ };
+ const buildAmbiguous1Attempt2: Build = {
+ id: '2',
+ builder: {project: 'p1', bucket: 'b1', builder: 'ambiguous-builder'},
+ status: BuildStatus.SUCCESS,
+ createTime: '',
+ startTime: '',
+ endTime: '',
+ };
+
+ const builds = [
+ buildUnique,
+ buildAmbiguous1,
+ buildAmbiguous2,
+ buildAmbiguous1Attempt2,
+ ];
+ const result = fetcher.createAttemptList(builds);
+
+ assert.deepEqual(Object.keys(result).sort(), [
+ 'p1/b1/ambiguous-builder',
+ 'p2/b2/ambiguous-builder',
+ 'unique-builder',
+ ]);
+
+ assert.deepEqual(result['unique-builder'], [buildUnique]);
+ assert.deepEqual(result['p1/b1/ambiguous-builder'], [
+ buildAmbiguous1,
+ buildAmbiguous1Attempt2,
+ ]);
+ assert.deepEqual(result['p2/b2/ambiguous-builder'], [buildAmbiguous2]);
+ });
+
+ test('convertAttemptsToRuns uses qualified name for ambiguous builders', async () => {
+ const ambiguousBuild: Build = {
+ id: '1',
+ builder: {project: 'p1', bucket: 'b1', builder: 'ambiguous-builder'},
+ status: BuildStatus.FAILURE,
+ createTime: '',
+ startTime: '',
+ endTime: '',
+ };
+ const qualifiedName = 'p1/b1/ambiguous-builder';
+ const attemptsByBuilder = {
+ [qualifiedName]: [ambiguousBuild],
+ };
+
+ const runs = await fetcher.convertAttemptsToRuns(
+ 1,
+ 123,
+ 'repo',
+ 1,
+ attemptsByBuilder
+ );
+
+ assert.strictEqual(runs.length, 1);
+ assert.strictEqual(runs[0].checkName, qualifiedName);
+ });
+
test('convertAttemptsToRuns creates all runs for all attempts', async () => {
const attempts: Build[] = [
{
