[gold] Update most frontend links to details page to include grouping info.
Bug: skia:13712
Change-Id: I0ffe33687218bb814692a6b35882402cf16ddfe5
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/578680
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Leandro Lovisolo <lovisolo@google.com>
diff --git a/golden/modules/common.ts b/golden/modules/common.ts
index 968215f..5fd5ad4 100644
--- a/golden/modules/common.ts
+++ b/golden/modules/common.ts
@@ -43,13 +43,13 @@
/**
* Returns a link to the details page for a given test-digest pair.
- * @param test Test name.
+ * @param grouping Grouping.
* @param digest Digest.
* @param clID CL ID. Optional, omit or use empty string for master branch.
* @param crs Code review system. Optional, omit or use empty string for master branch.
*/
-export function detailHref(test: string, digest: string, clID = '', crs = ''): string {
- const u = `/detail?test=${test}&digest=${digest}`;
+export function detailHref(grouping: Params, digest: string, clID = '', crs = ''): string {
+ const u = `/detail?grouping=${encodeURIComponent(fromObject(grouping))}&digest=${digest}`;
if (clID) {
return `${u}&changelist_id=${clID}&crs=${crs}`;
}
diff --git a/golden/modules/common_test.ts b/golden/modules/common_test.ts
index ef27f81..3d3dbad 100644
--- a/golden/modules/common_test.ts
+++ b/golden/modules/common_test.ts
@@ -48,11 +48,13 @@
describe('detailHref', () => {
it('returns a path with and without an changelist id', () => {
- expect(detailHref('my-test', aDigest)).to.equal(
- '/detail?test=my-test&digest=aaab78c9711cb79197d47f448ba51338',
+ expect(detailHref({ source_type: 'my-corpus', name: 'my-test' }, aDigest)).to.equal(
+ '/detail?grouping=name%3Dmy-test%26source_type%3Dmy-corpus'
+ + '&digest=aaab78c9711cb79197d47f448ba51338',
);
- expect(detailHref('my-test', aDigest, '12345', 'gerrit')).to.equal(
- '/detail?test=my-test&digest=aaab78c9711cb79197d47f448ba51338&changelist_id=12345&crs=gerrit',
+ expect(detailHref({ source_type: 'my-corpus', name: 'my-test' }, aDigest, '12345', 'gerrit')).to.equal(
+ '/detail?grouping=name%3Dmy-test%26source_type%3Dmy-corpus'
+ + '&digest=aaab78c9711cb79197d47f448ba51338&changelist_id=12345&crs=gerrit',
);
});
});
diff --git a/golden/modules/digest-details-sk/digest-details-sk.ts b/golden/modules/digest-details-sk/digest-details-sk.ts
index 736c4f5..2075a71 100644
--- a/golden/modules/digest-details-sk/digest-details-sk.ts
+++ b/golden/modules/digest-details-sk/digest-details-sk.ts
@@ -186,10 +186,22 @@
}
private static imageComparisonTemplate = (ele: DigestDetailsSk) => {
+ // We need the digest's grouping to compute a link to the details page. If the digest has no
+ // params, we'll get an exception when computing the grouping. If we don't catch the exception,
+ // this whole element will fail to render.
+ let maybeGrouping: Params | null = null;
+ try {
+ maybeGrouping = ele.getGrouping();
+ } catch {
+ // Nothing to do.
+ }
+
const left: ImageComparisonData = {
digest: ele._details.digest,
title: truncate(ele._details.digest, 15),
- detail: detailHref(ele._details.test, ele._details.digest, ele._changeListID, ele._crs),
+ detail: maybeGrouping
+ ? detailHref(maybeGrouping, ele._details.digest, ele._changeListID, ele._crs)
+ : '',
};
if (!ele.right) {
const hasOtherDigests = (ele._details.traces?.digests?.length || 0) > 1;
@@ -204,7 +216,9 @@
const right: ImageComparisonData = {
digest: ele.right.digest,
title: ele.right.status === 'positive' ? 'Closest Positive' : 'Closest Negative',
- detail: detailHref(ele._details.test, ele.right.digest, ele._changeListID, ele._crs),
+ detail: maybeGrouping
+ ? detailHref(maybeGrouping, ele.right.digest, ele._changeListID, ele._crs)
+ : '',
};
if (ele.overrideRight) {
right.title = truncate(ele.right.digest, 15);
diff --git a/golden/modules/digest-details-sk/digest-details-sk_test.ts b/golden/modules/digest-details-sk/digest-details-sk_test.ts
index 62d25f6..6b254c2 100644
--- a/golden/modules/digest-details-sk/digest-details-sk_test.ts
+++ b/golden/modules/digest-details-sk/digest-details-sk_test.ts
@@ -83,8 +83,10 @@
'Closest Positive',
]);
expect(await digestDetailsSkPO.imageCompareSkPO.getImageCaptionHrefs()).to.deep.equal([
- '/detail?test=dots-legend-sk_too-many-digests&digest=6246b773851984c726cb2e1cb13510c2',
- '/detail?test=dots-legend-sk_too-many-digests&digest=99c58c7002073346ff55f446d47d6311',
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ + 'digest=6246b773851984c726cb2e1cb13510c2',
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ + 'digest=99c58c7002073346ff55f446d47d6311',
]);
expect(await digestDetailsSkPO.isClosestImageIsNegativeWarningVisible()).to.be.false;
});
@@ -97,8 +99,10 @@
'Closest Negative',
]);
expect(await digestDetailsSkPO.imageCompareSkPO.getImageCaptionHrefs()).to.deep.equal([
- '/detail?test=dots-legend-sk_too-many-digests&digest=6246b773851984c726cb2e1cb13510c2',
- '/detail?test=dots-legend-sk_too-many-digests&digest=ec3b8f27397d99581e06eaa46d6d5837',
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ + 'digest=6246b773851984c726cb2e1cb13510c2',
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ + 'digest=ec3b8f27397d99581e06eaa46d6d5837',
]);
expect(await digestDetailsSkPO.isClosestImageIsNegativeWarningVisible()).to.be.true;
});
@@ -193,9 +197,9 @@
it('includes changelist id on the appropriate links', async () => {
expect(await digestDetailsSkPO.imageCompareSkPO.getImageCaptionHrefs()).to.deep.equal([
- '/detail?test=dots-legend-sk_too-many-digests'
- + '&digest=6246b773851984c726cb2e1cb13510c2&changelist_id=12345&crs=github',
- '/detail?test=dots-legend-sk_too-many-digests&'
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ + 'digest=6246b773851984c726cb2e1cb13510c2&changelist_id=12345&crs=github',
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ 'digest=99c58c7002073346ff55f446d47d6311&changelist_id=12345&crs=github',
]);
@@ -208,11 +212,11 @@
it('passes changeListID and crs to appropriate subelements', async () => {
expect(await digestDetailsSkPO.dotsLegendSkPO.getDigestHrefs()).to.deep.equal([
- '/detail?test=dots-legend-sk_too-many-digests&'
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ 'digest=6246b773851984c726cb2e1cb13510c2&changelist_id=12345&crs=github',
- '/detail?test=dots-legend-sk_too-many-digests&'
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ 'digest=99c58c7002073346ff55f446d47d6311&changelist_id=12345&crs=github',
- '/detail?test=dots-legend-sk_too-many-digests&'
+ '/detail?grouping=name%3Ddots-legend-sk_too-many-digests%26source_type%3Dinfra&'
+ 'digest=ec3b8f27397d99581e06eaa46d6d5837&changelist_id=12345&crs=github',
]);
});
diff --git a/golden/modules/dots-legend-sk/dots-legend-sk.ts b/golden/modules/dots-legend-sk/dots-legend-sk.ts
index d57afcf..e0b9e99 100644
--- a/golden/modules/dots-legend-sk/dots-legend-sk.ts
+++ b/golden/modules/dots-legend-sk/dots-legend-sk.ts
@@ -147,7 +147,7 @@
}
private digestDetailHref(index: number): string {
- return detailHref(this.grouping.name, this._digests[index].digest, this.changeListID, this.crs);
+ return detailHref(this.grouping, this._digests[index].digest, this.changeListID, this.crs);
}
private digestDiffHref(index: number): string {
diff --git a/golden/modules/dots-legend-sk/dots-legend-sk_test.ts b/golden/modules/dots-legend-sk/dots-legend-sk_test.ts
index 49be595..9bb3475 100644
--- a/golden/modules/dots-legend-sk/dots-legend-sk_test.ts
+++ b/golden/modules/dots-legend-sk/dots-legend-sk_test.ts
@@ -57,7 +57,9 @@
});
it('renders digest links correctly', async () => {
- const digestHrefFor = (d: string) => `/detail?test=My Test&digest=${d}`;
+ const digestHrefFor = (d: string) => '/detail?'
+ + 'grouping=name%3DMy%2520Test%26source_type%3Dmy-corpus&'
+ + `digest=${d}`;
expect(await dotsLegendSkPO.getDigestHrefs()).to.deep.equal([
digestHrefFor('00000000000000000000000000000000'),
digestHrefFor('11111111111111111111111111111111'),
@@ -97,7 +99,9 @@
});
it('renders digest links correctly', async () => {
- const digestHrefFor = (d: string) => `/detail?test=My Test&digest=${d}&changelist_id=123456&crs=gerrit`;
+ const digestHrefFor = (d: string) => '/detail?'
+ + 'grouping=name%3DMy%2520Test%26source_type%3Dmy-corpus&'
+ + `digest=${d}&changelist_id=123456&crs=gerrit`;
expect(await dotsLegendSkPO.getDigestHrefs()).to.deep.equal([
digestHrefFor('00000000000000000000000000000000'),
digestHrefFor('11111111111111111111111111111111'),
diff --git a/golden/modules/triagelog-page-sk/triagelog-page-sk.ts b/golden/modules/triagelog-page-sk/triagelog-page-sk.ts
index a25acda..ff0f8f4 100644
--- a/golden/modules/triagelog-page-sk/triagelog-page-sk.ts
+++ b/golden/modules/triagelog-page-sk/triagelog-page-sk.ts
@@ -13,7 +13,9 @@
import { stateReflector } from 'common-sk/modules/stateReflector';
import { ElementSk } from '../../../infra-sk/modules/ElementSk';
import '../pagination-sk';
-import { sendBeginTask, sendEndTask, sendFetchError } from '../common';
+import {
+ detailHref, sendBeginTask, sendEndTask, sendFetchError,
+} from '../common';
import {
TriageDelta, TriageLogEntry, TriageLogResponse,
} from '../rpc_types';
@@ -71,17 +73,14 @@
<tr class="details details-separator"><td colspan="4"></td></tr>
`;
- private static detailsEntryTemplate = (el: TriagelogPageSk, delta: TriageDelta) => {
- let detailHref = `/detail?test=${delta.grouping.name}&digest=${delta.digest}`;
- if (el.changelistID) {
- detailHref += `&changelist_id=${el.changelistID}&crs=${el.crs}`;
- }
- return html`
+ private static detailsEntryTemplate = (el: TriagelogPageSk, delta: TriageDelta) => html`
<tr class=details>
<td></td>
<td class=test-name title="Grouping ${JSON.stringify(delta.grouping)}">${delta.grouping.name}</td>
<td class=digest>
- <a href=${detailHref} target=_blank rel=noopener>
+ <a href=${detailHref(delta.grouping, delta.digest, el.changelistID, el.crs)}
+ target=_blank
+ rel=noopener>
${delta.digest}
</a>
</td>
@@ -90,7 +89,6 @@
</td>
</tr>
`;
- };
private entries: TriageLogEntry[] = []; // Log entries fetched from the server.
