diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 15efc7aa4bb8..90eecd54d78d 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -92,6 +92,18 @@ export class PythonResultResolver implements ITestResultResolver { ); errorNodeLabel.isTrusted = true; errorNode.error = errorNodeLabel; + } else if (rawTestData.status === 'error-empty') { + // search this.testController.items find item with uri and delete it + if (rawTestData.error) { + const uri = rawTestData.error[0]; + this.testController.items.forEach((item) => { + item.children.forEach((child) => { + if (child.id === uri) { + item.children.delete(child.id); + } + }); + }); + } } else { // remove error node only if no errors exist. this.testController.items.delete(`DiscoveryError:${workspacePath}`); @@ -102,7 +114,8 @@ export class PythonResultResolver implements ITestResultResolver { // If the test root for this folder exists: Workspace refresh, update its children. // Otherwise, it is a freshly discovered workspace, and we need to create a new test root and populate the test tree. - populateTestTree(this.testController, rawTestData.tests, undefined, this, token); + const node = this.testController.items.get(workspacePath); + populateTestTree(this.testController, rawTestData.tests, node, this, token); } sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index e51270eb4f9e..ed089bb881ab 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -211,7 +211,7 @@ export interface ITestResultResolver { export interface ITestDiscoveryAdapter { // ** first line old method signature, second line new method signature discoverTests(uri: Uri): Promise; - discoverTests(uri: Uri, executionFactory: IPythonExecutionFactory): Promise; + discoverTests(uri: Uri, executionFactory: IPythonExecutionFactory, fileUri?: Uri): Promise; } // interface for execution/runner adapter @@ -251,7 +251,7 @@ export type DiscoveredTestNode = DiscoveredTestCommon & { export type DiscoveredTestPayload = { cwd: string; tests?: DiscoveredTestNode; - status: 'success' | 'error'; + status: 'success' | 'error' | 'error-empty'; error?: string[]; }; diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 23ee881a405a..8381eb3ba6fd 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -269,7 +269,18 @@ export function populateTestTree( resultResolver.runIdToVSid.set(child.runID, child.id_); resultResolver.vsIdToRunId.set(child.id_, child.runID); } else { - let node = testController.items.get(child.path); + // testRoot.children.get(child.path); + let node = testRoot!.children.get(child.path); + if (node !== undefined) { + console.log('WE MADE IT'); + } + if (child.path.includes('.py')) { + console.log('IS A FILE DELETE CHIDLREN'); + // delete the given file node from the children of its parent + testRoot!.children.delete(child.path); + // mark the node as undefined so a new node must be created below + node = undefined; + } if (!node) { node = testController.createTestItem(child.id_, child.name, Uri.file(child.path)); @@ -326,6 +337,14 @@ export function createDiscoveryErrorPayload( }; } +export function createEmptyFileDiscoveryPayload(cwd: string, fileUri: Uri): DiscoveredTestPayload { + return { + cwd, + status: 'error-empty', + error: [fileUri.fsPath], + }; +} + export function createEOTPayload(executionBool: boolean): EOTTestPayload { return { commandType: executionBool ? 'execution' : 'discovery', diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index bc9d2ca8299f..7ecb54e4f53a 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -258,7 +258,11 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } private async refreshTestDataInternal(uri?: Resource): Promise { + // the refresh ends up here (after being called on file save), the uri is of the file + // this method is confusing, it contains old and new code from before and after the rewrite, please make + // sure to focus on the rewrite as this will be the code we will keep moving forward this.refreshingStartedEvent.fire(); + // refresh button = uri is undefined if (uri) { const settings = this.configSettings.getSettings(uri); const workspace = this.workspaceService.getWorkspaceFolder(uri); @@ -268,14 +272,17 @@ export class PythonTestController implements ITestController, IExtensionSingleAc // ** experiment to roll out NEW test discovery mechanism if (settings.testing.pytestEnabled) { if (pythonTestAdapterRewriteEnabled(this.serviceContainer)) { + // this here is where the rewrite is enabled traceInfo(`Running discovery for pytest using the new test adapter.`); if (workspace && workspace.uri) { const testAdapter = this.testAdapters.get(workspace.uri); if (testAdapter) { + // this calls discovery, it need to be updated to include the file uri testAdapter.discoverTests( this.testController, this.refreshCancellation.token, this.pythonExecFactory, + uri, ); } else { traceError('Unable to find test adapter for workspace.'); @@ -317,6 +324,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc // This handles the case where user removes test settings. Which should remove the // tests for that particular case from the tree view if (workspace) { + console.log('no longer deleting workspace', workspace); const toDelete: string[] = []; this.testController.items.forEach((i: TestItem) => { const w = this.workspaceService.getWorkspaceFolder(i.uri); @@ -549,11 +557,13 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } private watchForTestContentChangeOnSave(): void { + // When a file is saved, it will trigger this action this.disposables.push( onDidSaveTextDocument(async (doc: TextDocument) => { if (doc.fileName.endsWith('.py')) { traceVerbose(`Testing: Trigger refresh after saving ${doc.uri.fsPath}`); this.sendTriggerTelemetry('watching'); + // it will call refresh and add the doc.uri (the file uri being saved) this.refreshData.trigger(doc.uri, false); } }), diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 39dc87ed12c1..72269645b49e 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -22,6 +22,7 @@ import { MESSAGE_ON_TESTING_OUTPUT_MOVE, createDiscoveryErrorPayload, createEOTPayload, + createEmptyFileDiscoveryPayload, createTestingDeferred, fixLogLinesNoTrailing, } from '../common/utils'; @@ -39,7 +40,12 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { private readonly envVarsService?: IEnvironmentVariablesProvider, ) {} - async discoverTests(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise { + async discoverTests( + uri: Uri, + executionFactory?: IPythonExecutionFactory, + fileUri?: Uri, + ): Promise { + // after the workspaceTesetAdapter we jump here const uuid = this.testServer.createUUID(uri.fsPath); const deferredTillEOT: Deferred = createDeferred(); const dataReceivedDisposable = this.testServer.onDiscoveryDataReceived(async (e: DataReceivedEvent) => { @@ -51,7 +57,8 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { dataReceivedDisposable.dispose(); }; try { - await this.runPytestDiscovery(uri, uuid, executionFactory); + // then we go here + await this.runPytestDiscovery(uri, uuid, executionFactory, fileUri); } finally { await deferredTillEOT.promise; traceVerbose(`deferredTill EOT resolved for ${uri.fsPath}`); @@ -62,11 +69,25 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { return discoveryPayload; } - async runPytestDiscovery(uri: Uri, uuid: string, executionFactory?: IPythonExecutionFactory): Promise { + async runPytestDiscovery( + uri: Uri, + uuid: string, + executionFactory?: IPythonExecutionFactory, + fileUri?: Uri, + ): Promise { + // this is where we really call the run discovery const relativePathToPytest = 'pythonFiles'; const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest); const settings = this.configSettings.getSettings(uri); - const { pytestArgs } = settings.testing; + /* + + here we should be able to retrieve another setting which is the new setting we want to create. + I would name it something like `python.testing.DiscoveryOnOnlySavedFile`, I would need to talk with my team about an + official name so just use a placeholder for now. You can follow how other settings work (such as pytest args), + to get an idea on how to create a new setting and access it here in the code + + */ + let { pytestArgs } = settings.testing; const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath; // get and edit env vars @@ -98,6 +119,20 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { }; const execService = await executionFactory?.createActivatedEnvironment(creationOptions); // delete UUID following entire discovery finishing. + /* + here we are putting together all the args for the pytest discovery. + THIS SECTION I AM OPEN TO SUGGESTIONS, i am not sure if my suggested flow below is the best so please let me know if you have a better idea. + + 1. check to see if -k is already being used + 2. if it isn't then add `-k uriOfSavedFile` to the args + + */ + if (fileUri !== undefined) { + // filter out arg "." if it exits + const filteredPytestArgs = pytestArgs.filter((arg) => arg !== '.'); + filteredPytestArgs.push(fileUri.fsPath); + pytestArgs = filteredPytestArgs; + } const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(pytestArgs); traceVerbose(`Running pytest discovery with command: ${execArgs.join(' ')} for workspace ${uri.fsPath}.`); @@ -143,6 +178,20 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { data: JSON.stringify(createEOTPayload(true)), }); } + + if (code === 5 && fileUri !== undefined) { + // if no tests are found, then we need to send the error payload. + this.testServer.triggerDiscoveryDataReceivedEvent({ + uuid, + data: JSON.stringify(createEmptyFileDiscoveryPayload(cwd, fileUri)), + }); + // then send a EOT payload + this.testServer.triggerDiscoveryDataReceivedEvent({ + uuid, + data: JSON.stringify(createEOTPayload(true)), + }); + } + // deferredTillEOT is resolved when all data sent on stdout and stderr is received, close event is only called when this occurs // due to the sync reading of the output. deferredTillExecClose?.resolve(); diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index 35a5b7a24418..835f041e8c51 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -114,7 +114,10 @@ export class WorkspaceTestAdapter { testController: TestController, token?: CancellationToken, executionFactory?: IPythonExecutionFactory, + fileUri?: Uri, ): Promise { + // Here is where discover tests goes when called by the controller + // this then determines which adapter to use (either the run or discovery one) sendTelemetryEvent(EventName.UNITTEST_DISCOVERING, undefined, { tool: this.testProvider }); // Discovery is expensive. If it is already running, use the existing promise. @@ -128,8 +131,10 @@ export class WorkspaceTestAdapter { try { // ** execution factory only defined for new rewrite way if (executionFactory !== undefined) { - await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory); + // goes here + await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory, fileUri); } else { + // ignore this, it is the old way await this.discoveryAdapter.discoverTests(this.workspaceUri); } deferred.resolve();