diff --git a/frontend/src/pages/RunDetails.test.tsx b/frontend/src/pages/RunDetails.test.tsx index 6c274259ebb..5c4d264a6e1 100644 --- a/frontend/src/pages/RunDetails.test.tsx +++ b/frontend/src/pages/RunDetails.test.tsx @@ -729,7 +729,7 @@ describe('RunDetails', () => { }); it('resumes auto refreshing if window loses focus and then regains it', async () => { - // Declare that the run has not completed + // Declare that the run has not finished testRun.run!.status = NodePhase.PENDING; tree = shallow(); await TestUtils.flushPromises(); @@ -750,7 +750,7 @@ describe('RunDetails', () => { }); it('does not resume auto refreshing if window loses focus and then regains it but run is finished', async () => { - // Declare that the run has completed + // Declare that the run has finished testRun.run!.status = NodePhase.SUCCEEDED; tree = shallow(); await TestUtils.flushPromises(); diff --git a/frontend/src/pages/RunDetails.tsx b/frontend/src/pages/RunDetails.tsx index 636b0a3d6c5..bfd987ef24b 100644 --- a/frontend/src/pages/RunDetails.tsx +++ b/frontend/src/pages/RunDetails.tsx @@ -30,7 +30,7 @@ import WorkflowParser from '../lib/WorkflowParser'; import { ApiExperiment } from '../apis/experiment'; import { ApiRun } from '../apis/run'; import { Apis } from '../lib/Apis'; -import { NodePhase, statusToIcon, hasCompleted } from './Status'; +import { NodePhase, statusToIcon, hasFinished } from './Status'; import { OutputArtifactLoader } from '../lib/OutputArtifactLoader'; import { Page } from './Page'; import { RoutePage, RouteParams, QUERY_PARAMS } from '../components/Router'; @@ -83,8 +83,9 @@ interface RunDetailsState { } class RunDetails extends Page { - private _onBlur: any; - private _onFocus: any; + private _onBlur: EventListener; + private _onFocus: EventListener; + private readonly AUTO_REFRESH_INTERVAL = 5000; private _interval?: NodeJS.Timeout; @@ -249,7 +250,6 @@ class RunDetails extends Page { public async componentDidMount(): Promise { window.addEventListener('focus', this._onFocus); window.addEventListener('blur', this._onBlur); - await this.refresh(); await this._startAutoRefresh(); } @@ -258,14 +258,13 @@ class RunDetails extends Page { } public async onFocusHandler(): Promise { - await this.refresh(); await this._startAutoRefresh(); } public componentWillUnmount(): void { + this._stopAutoRefresh(); window.removeEventListener('focus', this._onFocus); window.removeEventListener('blur', this._onBlur); - this._stopAutoRefresh(); } public async refresh(): Promise { @@ -287,7 +286,7 @@ class RunDetails extends Page { let runFinished = this.state.runFinished; // If the run has finished, stop auto refreshing - if (hasCompleted(runMetadata.status as NodePhase)) { + if (hasFinished(runMetadata.status as NodePhase)) { this._stopAutoRefresh(); // This prevents other events, such as onFocus, from resuming the autorefresh runFinished = true; @@ -349,15 +348,19 @@ class RunDetails extends Page { } private async _startAutoRefresh(): Promise { - if (this.state.runFinished) { - return; + // If the run was not finished last time we checked, check again in case anything changed + // before proceeding to set auto-refresh interval + if (!this.state.runFinished) { + // refresh() updates runFinished's value + await this.refresh(); } - // Check if undefined to avoid setting multiple intervals - if (this._interval === undefined) { + // Only set interval if run has not finished, and verify that the interval is undefined to + // avoid setting multiple intervals + if (!this.state.runFinished && this._interval === undefined) { this._interval = setInterval( () => this.refresh(), - 5000 + this.AUTO_REFRESH_INTERVAL ); } } diff --git a/frontend/src/pages/Status.test.tsx b/frontend/src/pages/Status.test.tsx index b568f4e11ce..a3cfaefbd1d 100644 --- a/frontend/src/pages/Status.test.tsx +++ b/frontend/src/pages/Status.test.tsx @@ -16,7 +16,7 @@ import * as Utils from '../lib/Utils'; import { shallow } from 'enzyme'; -import { statusToIcon, NodePhase, hasCompleted } from './Status'; +import { statusToIcon, NodePhase, hasFinished } from './Status'; describe('Status', () => { @@ -69,21 +69,21 @@ describe('Status', () => { )); }); - describe('hasCompleted', () => { + describe('hasFinished', () => { [NodePhase.ERROR, NodePhase.FAILED, NodePhase.SUCCEEDED, NodePhase.SKIPPED].forEach(status => { - it(`returns \'true\' to true if status is: ${status}`, () => { - expect(hasCompleted(status)).toBe(true); + it(`returns \'true\' if status is: ${status}`, () => { + expect(hasFinished(status)).toBe(true); }); }); [NodePhase.PENDING, NodePhase.RUNNING, NodePhase.UNKNOWN].forEach(status => { - it(`returns \'false\' to true if status is: ${status}`, () => { - expect(hasCompleted(status)).toBe(false); + it(`returns \'false\' if status is: ${status}`, () => { + expect(hasFinished(status)).toBe(false); }); }); - it('returns \'false\' to true if status is undefined', () => { - expect(hasCompleted(undefined)).toBe(false); + it('returns \'false\' if status is undefined', () => { + expect(hasFinished(undefined)).toBe(false); }); }); }); diff --git a/frontend/src/pages/Status.tsx b/frontend/src/pages/Status.tsx index b410564dd1a..c9c431aba93 100644 --- a/frontend/src/pages/Status.tsx +++ b/frontend/src/pages/Status.tsx @@ -35,7 +35,7 @@ export enum NodePhase { UNKNOWN = 'Unknown', } -export function hasCompleted(status?: NodePhase): boolean { +export function hasFinished(status?: NodePhase): boolean { if (!status) { return false; }