[resultui] use computed invocation ID to query resultdb
1. Use computed invocation ID to query resultdb.
2. Change how uninitialised invocation ID is represented to make it
easier to update.
3. Add test cases.
After this CL:
1. old builds that don't support the new invocation ID format will not
work correctly. This will be fixed in the next CL.
2. result tab's rendering time will be reduced by 200-1000ms when
accessed directly.
R=nodir@chromium.org, nqmtuan@google.com
Bug: 1114935
Change-Id: Ie82d61895b87ddb9220dbbc22c4c462a38b22239
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/2712050
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Weiwei Lin <weiweilin@google.com>
diff --git a/milo/frontend/resultui/src/context/build_state.ts b/milo/frontend/resultui/src/context/build_state.ts
index f53046d..44424e5 100644
--- a/milo/frontend/resultui/src/context/build_state.ts
+++ b/milo/frontend/resultui/src/context/build_state.ts
@@ -14,13 +14,14 @@
import { computed, observable } from 'mobx';
import { fromPromise, FULFILLED, IPromiseBasedObservable } from 'mobx-utils';
-import { getGitilesRepoURL } from '../libs/build_utils';
+import { getGitilesRepoURL } from '../libs/build_utils';
import { consumeContext, provideContext } from '../libs/context';
import * as iter from '../libs/iter_utils';
import { BuildExt } from '../models/build_ext';
import { Build, BuilderID, GetBuildRequest, GitilesCommit } from '../services/buildbucket';
import { QueryBlamelistRequest, QueryBlamelistResponse } from '../services/milo_internal';
+import { getInvIdFromBuildId, getInvIdFromBuildNum } from '../services/resultdb';
import { AppState } from './app_state';
/**
@@ -30,6 +31,35 @@
@observable.ref builder?: BuilderID;
@observable.ref buildNumOrId?: string;
+ /**
+ * buildNum is defined when this.buildNumOrId is defined and doesn't start
+ * with 'b'.
+ */
+ @computed get buildNum() {
+ return this.buildNumOrId?.startsWith('b') === false ? Number(this.buildNumOrId) : null;
+ }
+
+ /**
+ * buildId is defined when this.buildNumOrId is defined and starts with 'b'.
+ */
+ @computed get buildId() {
+ return this.buildNumOrId?.startsWith('b') ? this.buildNumOrId.slice(1) : null;
+ }
+
+ @computed private get invocationId$(): IPromiseBasedObservable<string> {
+ if (this.builder && this.buildNum) {
+ return fromPromise(getInvIdFromBuildNum(this.builder, this.buildNum));
+ } else if (this.buildId) {
+ return fromPromise(Promise.resolve(getInvIdFromBuildId(this.buildId)));
+ } else {
+ return fromPromise(Promise.race([]));
+ }
+ }
+
+ @computed get invocationId() {
+ return this.invocationId$.state === FULFILLED ? this.invocationId$.value : null;
+ }
+
@observable.ref private timestamp = Date.now();
constructor(private appState: AppState) {}
@@ -62,9 +92,9 @@
// establish a dependency on timestamp.
this.timestamp; // tslint:disable-line: no-unused-expression
- const req: GetBuildRequest = this.buildNumOrId.startsWith('b')
- ? {id: this.buildNumOrId.slice(1), fields: '*'}
- : {builder: this.builder, buildNumber: Number(this.buildNumOrId), fields: '*'};
+ const req: GetBuildRequest = this.buildId
+ ? {id: this.buildId, fields: '*'}
+ : {builder: this.builder, buildNumber: this.buildNum!, fields: '*'};
return fromPromise(this.appState.buildsService.getBuild(req));
}
diff --git a/milo/frontend/resultui/src/context/invocation_state.ts b/milo/frontend/resultui/src/context/invocation_state.ts
index 62ecd54..7b9c8e0 100644
--- a/milo/frontend/resultui/src/context/invocation_state.ts
+++ b/milo/frontend/resultui/src/context/invocation_state.ts
@@ -26,8 +26,9 @@
* Records state of an invocation.
*/
export class InvocationState {
- @observable.ref invocationId = '';
- @observable.ref initialized = false;
+ // '' means no associated invocation ID.
+ // null means uninitialized.
+ @observable.ref invocationId: string | null = null;
@observable.ref showUnexpectedVariants = true;
@observable.ref showUnexpectedlySkippedVariants = true;
diff --git a/milo/frontend/resultui/src/pages/build_page/index.ts b/milo/frontend/resultui/src/pages/build_page/index.ts
index fcf837b..c6ca024 100644
--- a/milo/frontend/resultui/src/pages/build_page/index.ts
+++ b/milo/frontend/resultui/src/pages/build_page/index.ts
@@ -69,6 +69,11 @@
@observable.ref buildState!: BuildState;
@observable.ref invocationState!: InvocationState;
+ // Set to true when testing.
+ // Otherwise this.render() will throw due to missing initialization steps in
+ // this.onBeforeEnter.
+ @observable.ref prerender = false;
+
@observable private readonly uncommittedConfigs: UserConfigs = merge({}, DEFAULT_USER_CONFIGS);
@observable.ref private showFeedbackDialog = false;
@@ -148,15 +153,11 @@
));
this.disposers.push(reaction(
- () => [
- this.appState,
- this.buildState.build?.infra?.resultdb?.invocation?.slice('invocations/'.length) || '',
- ] as [AppState, string],
- ([appState, invId]) => {
+ () => this.appState,
+ (appState) => {
this.invocationState?.dispose();
this.invocationState = new InvocationState(appState);
- this.invocationState.invocationId = invId;
- this.invocationState.initialized = true;
+ this.invocationState.invocationId = this.buildState.invocationId;
// Emulate @property() update.
this.updated(new Map([['invocationState', this.invocationState]]));
@@ -165,7 +166,14 @@
));
this.disposers.push(() => this.invocationState.dispose());
- this.disposers.push(autorun(
+ this.disposers.push(reaction(
+ () => this.buildState.invocationId,
+ (invId) => this.invocationState.invocationId = invId,
+ {fireImmediately: true},
+ ));
+
+ this.disposers.push(reaction(
+ () => this.invocationState.invocation$.state,
() => {
if (this.invocationState.invocation$.state !== REJECTED) {
return;
@@ -305,6 +313,10 @@
}
protected render() {
+ if (this.prerender) {
+ return html``;
+ }
+
return html`
<mwc-dialog
id="settings-dialog"
diff --git a/milo/frontend/resultui/src/pages/build_page/index_test.ts b/milo/frontend/resultui/src/pages/build_page/index_test.ts
new file mode 100644
index 0000000..5566169
--- /dev/null
+++ b/milo/frontend/resultui/src/pages/build_page/index_test.ts
@@ -0,0 +1,80 @@
+// Copyright 2021 The LUCI Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { aTimeout, fixture, fixtureCleanup } from '@open-wc/testing/index-no-side-effects';
+import { Commands, RouterLocation } from '@vaadin/router';
+import { assert } from 'chai';
+import { html } from 'lit-element';
+
+import '.';
+import { BuildPageElement } from '.';
+import { AppState } from '../../context/app_state';
+import { UserConfigsStore } from '../../context/user_configs';
+import { getInvIdFromBuildId, getInvIdFromBuildNum } from '../../services/resultdb';
+
+const builder = {
+ project: 'project',
+ bucket: 'bucket',
+ builder: 'builder',
+};
+
+describe('Invocation Page', () => {
+ it('should compute invocation ID from buildNum in URL', async () => {
+ after(fixtureCleanup);
+ const page = await fixture<BuildPageElement>(html`
+ <milo-build-page
+ .prerender=${true}
+ .appState=${new AppState()}
+ .configsStore=${new UserConfigsStore()}
+ ></milo-build-page>
+ `);
+
+ const location = {
+ params: {
+ ...builder,
+ 'build_num_or_id': '1234',
+ },
+ } as Partial<RouterLocation> as RouterLocation;
+ const cmd = {} as Partial<Commands> as Commands;
+ await page.onBeforeEnter(location, cmd);
+ page.connectedCallback();
+ await aTimeout(10);
+ assert.strictEqual(page.buildState.invocationId, await getInvIdFromBuildNum(builder, 1234));
+ page.disconnectedCallback();
+ });
+
+ it('should compute invocation ID from build ID in URL', async () => {
+ after(fixtureCleanup);
+ const page = await fixture<BuildPageElement>(html`
+ <milo-build-page
+ .prerender=${true}
+ .appState=${new AppState()}
+ .configsStore=${new UserConfigsStore()}
+ ></milo-build-page>
+ `);
+
+ const location = {
+ params: {
+ ...builder,
+ 'build_num_or_id': 'b1234',
+ },
+ } as Partial<RouterLocation> as RouterLocation;
+ const cmd = {} as Partial<Commands> as Commands;
+ await page.onBeforeEnter(location, cmd);
+ page.connectedCallback();
+ await aTimeout(10);
+ assert.strictEqual(page.buildState.invocationId, getInvIdFromBuildId('1234'));
+ page.disconnectedCallback();
+ });
+});
diff --git a/milo/frontend/resultui/src/pages/invocation_page/index.ts b/milo/frontend/resultui/src/pages/invocation_page/index.ts
index b484a9a..75977f7 100644
--- a/milo/frontend/resultui/src/pages/invocation_page/index.ts
+++ b/milo/frontend/resultui/src/pages/invocation_page/index.ts
@@ -60,7 +60,6 @@
this.invocationState?.dispose();
this.invocationState = new InvocationState(appState);
this.invocationState.invocationId = this.invocationId;
- this.invocationState.initialized = true;
// Emulate @property() update.
this.updated(new Map([['invocationState', this.invocationState]]));
@@ -116,7 +115,7 @@
label: 'Test Results',
href: router.urlForName(
'invocation-test-results',
- {'invocation_id': this.invocationState.invocationId},
+ {'invocation_id': this.invocationState.invocationId!},
),
},
{
@@ -124,7 +123,7 @@
label: 'Invocation Details',
href: router.urlForName(
'invocation-details',
- {'invocation_id': this.invocationState.invocationId},
+ {'invocation_id': this.invocationState.invocationId!},
),
},
];
diff --git a/milo/frontend/resultui/src/pages/invocation_page/index_test.ts b/milo/frontend/resultui/src/pages/invocation_page/index_test.ts
index 7eeac9e..38c9627 100644
--- a/milo/frontend/resultui/src/pages/invocation_page/index_test.ts
+++ b/milo/frontend/resultui/src/pages/invocation_page/index_test.ts
@@ -39,7 +39,7 @@
const cmd = {} as Partial<Commands> as Commands;
await page.onBeforeEnter(location, cmd);
page.connectedCallback();
- assert.strictEqual(page.invocationState.invocationId, location.params['invocation_id']);
+ assert.strictEqual(page.invocationState.invocationId, location.params['invocation_id'] as string);
page.disconnectedCallback();
});
diff --git a/milo/frontend/resultui/src/pages/test_results_tab.ts b/milo/frontend/resultui/src/pages/test_results_tab.ts
index 5b17e4d..b57ebda 100644
--- a/milo/frontend/resultui/src/pages/test_results_tab.ts
+++ b/milo/frontend/resultui/src/pages/test_results_tab.ts
@@ -327,7 +327,7 @@
private renderMain() {
const state = this.invocationState;
- if (state.initialized && !state.invocationId) {
+ if (state.invocationId === '') {
return html`
<div id="no-invocation">
No associated invocation.<br>
