From c900d7671eea1944f9e164dd3d678bff907289c0 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Wed, 24 May 2023 14:59:55 -0700 Subject: [PATCH 1/2] testing: add proposed testInvalidateResults API For #134970 --- extensions/vscode-api-tests/package.json | 1 + src/vs/base/test/browser/dom.test.ts | 2 ++ .../api/browser/mainThreadTesting.ts | 12 ++++++++ .../workbench/api/common/extHost.protocol.ts | 2 ++ src/vs/workbench/api/common/extHostTesting.ts | 6 ++++ .../hierarchalByLocation.ts | 5 +++- .../contrib/testing/common/testResult.ts | 12 ++++++++ .../contrib/testing/common/testTypes.ts | 8 ++--- .../common/extensionsApiProposals.ts | 1 + .../test/browser/domActivityTracker.test.ts | 1 + ...vscode.proposed.testInvalidateResults.d.ts | 30 +++++++++++++++++++ 11 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 src/vscode-dts/vscode.proposed.testInvalidateResults.d.ts diff --git a/extensions/vscode-api-tests/package.json b/extensions/vscode-api-tests/package.json index a802f11c3150c..c0766007ad6c8 100644 --- a/extensions/vscode-api-tests/package.json +++ b/extensions/vscode-api-tests/package.json @@ -43,6 +43,7 @@ "tokenInformation", "treeItemCheckbox", "treeViewReveal", + "testInvalidateResults", "workspaceTrust", "telemetry", "windowActivity", diff --git a/src/vs/base/test/browser/dom.test.ts b/src/vs/base/test/browser/dom.test.ts index 87f5b2d4cab19..dfb9f0a342b8f 100644 --- a/src/vs/base/test/browser/dom.test.ts +++ b/src/vs/base/test/browser/dom.test.ts @@ -18,6 +18,8 @@ suite('dom', () => { assert(!element.classList.contains('bar')); assert(!element.classList.contains('foo')); assert(!element.classList.contains('')); + + }); test('removeClass', () => { diff --git a/src/vs/workbench/api/browser/mainThreadTesting.ts b/src/vs/workbench/api/browser/mainThreadTesting.ts index 676816725fc1d..d9ea8c1e67706 100644 --- a/src/vs/workbench/api/browser/mainThreadTesting.ts +++ b/src/vs/workbench/api/browser/mainThreadTesting.ts @@ -52,6 +52,18 @@ export class MainThreadTesting extends Disposable implements MainThreadTestingSh })); } + /** + * @inheritdoc + */ + $markTestRetired(testId: string): void { + for (const result of this.resultService.results) { + // all non-live results are already entirely outdated + if (result instanceof LiveTestResult) { + result.markRetired(testId); + } + } + } + /** * @inheritdoc */ diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 438fa03cd44c4..8d98f3c7fcabc 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2500,6 +2500,8 @@ export interface MainThreadTestingShape { $startedExtensionTestRun(req: ExtensionRunTestsRequest): void; /** Signals that an extension-provided test run finished. */ $finishedExtensionTestRun(runId: string): void; + /** Marks a test (or controller) as retired in all results. */ + $markTestRetired(testId: string): void; } // --- proxy identifiers diff --git a/src/vs/workbench/api/common/extHostTesting.ts b/src/vs/workbench/api/common/extHostTesting.ts index ab1ffe90cea81..5e2e6f1bc19c6 100644 --- a/src/vs/workbench/api/common/extHostTesting.ts +++ b/src/vs/workbench/api/common/extHostTesting.ts @@ -27,6 +27,7 @@ import { TestCommandId } from 'vs/workbench/contrib/testing/common/constants'; import { TestId, TestIdPathParts, TestPosition } from 'vs/workbench/contrib/testing/common/testId'; import { InvalidTestItemError } from 'vs/workbench/contrib/testing/common/testItemCollection'; import { AbstractIncrementalTestCollection, CoverageDetails, ICallProfileRunHandler, IFileCoverage, ISerializedTestResults, IStartControllerTests, IStartControllerTestsResult, ITestItem, ITestItemContext, IncrementalChangeCollector, IncrementalTestCollectionItem, InternalTestItem, TestResultState, TestRunProfileBitset, TestsDiff, TestsDiffOp, isStartControllerTests } from 'vs/workbench/contrib/testing/common/testTypes'; +import { checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; import type * as vscode from 'vscode'; interface ControllerInfo { @@ -137,6 +138,11 @@ export class ExtHostTesting implements ExtHostTestingShape { createTestRun: (request, name, persist = true) => { return this.runTracker.createTestRun(controllerId, collection, request, name, persist); }, + invalidateTestResults: item => { + checkProposedApiEnabled(extension, 'testInvalidateResults'); + const id = item ? TestId.fromExtHostTestItem(item, controllerId).toString() : controllerId; + return this.proxy.$markTestRetired(id); + }, set resolveHandler(fn) { collection.resolveHandler = fn; }, diff --git a/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts b/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts index b5894759b3fed..f1ae17cb78989 100644 --- a/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts +++ b/src/vs/workbench/contrib/testing/browser/explorerProjections/hierarchalByLocation.ts @@ -87,7 +87,10 @@ export class HierarchicalByLocationProjection extends Disposable implements ITes // when test states change, reflect in the tree this._register(results.onTestChanged(ev => { let result = ev.item; - if (result.ownComputedState === TestResultState.Unset) { + // if the state is unset, or the latest run is not making the change, + // double check that it's valid. Retire calls might cause previous + // emit a state change for a test run that's already long completed. + if (result.ownComputedState === TestResultState.Unset || ev.result !== results.results[0]) { const fallback = results.getStateById(result.item.extId); if (fallback) { result = fallback[1]; diff --git a/src/vs/workbench/contrib/testing/common/testResult.ts b/src/vs/workbench/contrib/testing/common/testResult.ts index 8627db9c2bc6d..c2130600cdbe7 100644 --- a/src/vs/workbench/contrib/testing/common/testResult.ts +++ b/src/vs/workbench/contrib/testing/common/testResult.ts @@ -449,6 +449,18 @@ export class LiveTestResult implements ITestResult { this.completeEmitter.fire(); } + /** + * Marks the test and all of its children in the run as retired. + */ + public markRetired(testId: string) { + for (const [id, test] of this.testById) { + if (!test.retired && id === testId || TestId.isChild(testId, id)) { + test.retired = true; + this.changeEmitter.fire({ reason: TestResultItemChangeReason.ComputedStateChange, item: test, result: this }); + } + } + } + /** * @inheritdoc */ diff --git a/src/vs/workbench/contrib/testing/common/testTypes.ts b/src/vs/workbench/contrib/testing/common/testTypes.ts index a740cd40d8a63..99590868fd052 100644 --- a/src/vs/workbench/contrib/testing/common/testTypes.ts +++ b/src/vs/workbench/contrib/testing/common/testTypes.ts @@ -469,12 +469,14 @@ export interface TestResultItem extends InternalTestItem { } export namespace TestResultItem { - /** Serialized version of the TestResultItem */ + /** + * Serialized version of the TestResultItem. Note that 'retired' is not + * included since all hydrated items are automatically retired. + */ export interface Serialized extends InternalTestItem.Serialized { tasks: ITestTaskState.Serialized[]; ownComputedState: TestResultState; computedState: TestResultState; - retired?: boolean; } export const serializeWithoutMessages = (original: TestResultItem): Serialized => ({ @@ -482,7 +484,6 @@ export namespace TestResultItem { ownComputedState: original.ownComputedState, computedState: original.computedState, tasks: original.tasks.map(ITestTaskState.serializeWithoutMessages), - retired: original.retired, }); export const serialize = (original: TestResultItem): Serialized => ({ @@ -490,7 +491,6 @@ export namespace TestResultItem { ownComputedState: original.ownComputedState, computedState: original.computedState, tasks: original.tasks.map(ITestTaskState.serialize), - retired: original.retired, }); export const deserialize = (serialized: Serialized): TestResultItem => ({ diff --git a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts index f1c61165b4c1a..9da19f74bda6b 100644 --- a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts +++ b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts @@ -83,6 +83,7 @@ export const allApiProposals = Object.freeze({ terminalDimensions: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.terminalDimensions.d.ts', terminalQuickFixProvider: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.terminalQuickFixProvider.d.ts', testCoverage: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.testCoverage.d.ts', + testInvalidateResults: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.testInvalidateResults.d.ts', testObserver: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.testObserver.d.ts', textSearchProvider: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.textSearchProvider.d.ts', timeline: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.timeline.d.ts', diff --git a/src/vs/workbench/services/userActivity/test/browser/domActivityTracker.test.ts b/src/vs/workbench/services/userActivity/test/browser/domActivityTracker.test.ts index a204126757598..4c443c006a135 100644 --- a/src/vs/workbench/services/userActivity/test/browser/domActivityTracker.test.ts +++ b/src/vs/workbench/services/userActivity/test/browser/domActivityTracker.test.ts @@ -27,6 +27,7 @@ suite('DomActivityTracker', () => { clock.restore(); }); + test('marks inactive on no input', () => { assert.equal(uas.isActive, true); clock.tick(maxTimeToBecomeIdle); diff --git a/src/vscode-dts/vscode.proposed.testInvalidateResults.d.ts b/src/vscode-dts/vscode.proposed.testInvalidateResults.d.ts new file mode 100644 index 0000000000000..35f955c2f4dc9 --- /dev/null +++ b/src/vscode-dts/vscode.proposed.testInvalidateResults.d.ts @@ -0,0 +1,30 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +// https://github.com/microsoft/vscode/issues/134970 + +declare module 'vscode' { + + export interface TestController { + + /** + * Marks an item's results as being outdated. This is commonly called when + * code or configuration changes and previous results should no longer + * be considered relevant. The same logic used to mark results as outdated + * may be used to drive {@link TestRunRequest.continuous continuous test runs}. + * + * If an item is passed to this method, test results for the item and all of + * its children will be marked as outdated. If no item is passed, then all + * test owned by the TestController will be marked as outdated. + * + * Any test runs started before the moment this method is called, including + * runs which may still be ongoing, will be marked as outdated and deprioritized + * in the editor's UI. + * + * @param item Item to mark as outdated. If undefined, all the controller's items are marked outdated. + */ + invalidateTestResults(item?: TestItem): void; + } +} From 00c8edc2ab997eb7d26da87938fc4e992ae8a1fc Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Wed, 24 May 2023 15:52:35 -0700 Subject: [PATCH 2/2] fix tests --- .../browser/explorerProjections/hierarchalByLocation.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/testing/test/browser/explorerProjections/hierarchalByLocation.test.ts b/src/vs/workbench/contrib/testing/test/browser/explorerProjections/hierarchalByLocation.test.ts index 44b67eaea34a3..502b66a905245 100644 --- a/src/vs/workbench/contrib/testing/test/browser/explorerProjections/hierarchalByLocation.test.ts +++ b/src/vs/workbench/contrib/testing/test/browser/explorerProjections/hierarchalByLocation.test.ts @@ -23,6 +23,7 @@ suite('Workbench - Testing Explorer Hierarchal by Location Projection', () => { setup(() => { onTestChanged = new Emitter(); resultsService = { + results: [], onResultsChanged: () => undefined, onTestChanged: onTestChanged.event, getStateById: () => ({ state: { state: 0 }, computedState: 0 }), @@ -102,7 +103,6 @@ suite('Workbench - Testing Explorer Hierarchal by Location Projection', () => { test('applies state changes', async () => { harness.flush(); - resultsService.getStateById = () => [undefined, resultInState(TestResultState.Failed)]; const resultInState = (state: TestResultState): TestResultItem => ({ item: { @@ -124,6 +124,7 @@ suite('Workbench - Testing Explorer Hierarchal by Location Projection', () => { }); // Applies the change: + resultsService.getStateById = () => [undefined, resultInState(TestResultState.Queued)]; onTestChanged.fire({ reason: TestResultItemChangeReason.OwnStateChange, result: null as any, @@ -139,6 +140,7 @@ suite('Workbench - Testing Explorer Hierarchal by Location Projection', () => { ]); // Falls back if moved into unset state: + resultsService.getStateById = () => [undefined, resultInState(TestResultState.Failed)]; onTestChanged.fire({ reason: TestResultItemChangeReason.OwnStateChange, result: null as any,