remove node deletion for error tolerant discovery (microsoft#22207)

helps with a part of
microsoft#21757

Author: eleanorjboyd

pythonFiles/vscode_pytest/__init__.py

@@ -302,12 +302,12 @@ def pytest_sessionfinish(session, exitstatus):
     session -- the pytest session object.
     exitstatus -- the status code of the session.
 
-    0: All tests passed successfully.
-    1: One or more tests failed.
-    2: Pytest was unable to start or run any tests due to issues with test discovery or test collection.
-    3: Pytest was interrupted by the user, for example by pressing Ctrl+C during test execution.
-    4: Pytest encountered an internal error or exception during test execution.
-    5: Pytest was unable to find any tests to run.
+    Exit code 0: All tests were collected and passed successfully
+    Exit code 1: Tests were collected and run but some of the tests failed
+    Exit code 2: Test execution was interrupted by the user
+    Exit code 3: Internal error happened while executing tests
+    Exit code 4: pytest command line usage error
+    Exit code 5: No tests were collected
     """
     cwd = pathlib.Path.cwd()
     if IS_DISCOVERY:

src/client/testing/testController/common/resultResolver.ts

@@ -103,14 +103,6 @@ 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);
-        } else {
-            // Delete everything from the test controller.
-            const errorNode = this.testController.items.get(`DiscoveryError:${workspacePath}`);
-            this.testController.items.replace([]);
-            // Add back the error node if it exists.
-            if (errorNode !== undefined) {
-                this.testController.items.add(errorNode);
-            }
         }
 
         sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, {

src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts

@@ -120,9 +120,10 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
             }
         });
         result?.proc?.on('close', (code, signal) => {
-            if (code !== 0) {
+            // pytest exits with code of 5 when 0 tests are found- this is not a failure for discovery.
+            if (code !== 0 && code !== 5) {
                 traceError(
-                    `Subprocess exited unsuccessfully with exit code ${code} and signal ${signal}. Creating and sending error discovery payload`,
+                    `Subprocess exited unsuccessfully with exit code ${code} and signal ${signal} on workspace ${uri.fsPath}. Creating and sending error discovery payload`,
                 );
                 // if the child process exited with a non-zero exit code, then we need to send the error payload.
                 this.testServer.triggerDiscoveryDataReceivedEvent({

src/test/testing/testController/resultResolver.unit.test.ts

@@ -195,6 +195,69 @@ suite('Result Resolver tests', () => {
                 cancelationToken, // token
             );
         });
+        test('resolveDiscovery should create error and not clear test items to allow for error tolerant discovery', async () => {
+            // test specific constants used expected values
+            testProvider = 'pytest';
+            workspaceUri = Uri.file('/foo/bar');
+            resultResolver = new ResultResolver.PythonResultResolver(testController, testProvider, workspaceUri);
+            const errorMessage = 'error msg A';
+            const expectedErrorMessage = `${defaultErrorMessage}\r\n ${errorMessage}`;
+
+            // create test result node
+            const tests: DiscoveredTestNode = {
+                path: 'path',
+                name: 'name',
+                type_: 'folder',
+                id_: 'id',
+                children: [],
+            };
+            // stub out return values of functions called in resolveDiscovery
+            const errorPayload: DiscoveredTestPayload = {
+                cwd: workspaceUri.fsPath,
+                status: 'error',
+                error: [errorMessage],
+            };
+            const regPayload: DiscoveredTestPayload = {
+                cwd: workspaceUri.fsPath,
+                status: 'success',
+                error: [errorMessage],
+                tests,
+            };
+            const errorTestItemOptions: testItemUtilities.ErrorTestItemOptions = {
+                id: 'id',
+                label: 'label',
+                error: 'error',
+            };
+
+            // stub out functionality of buildErrorNodeOptions and createErrorTestItem which are called in resolveDiscovery
+            const buildErrorNodeOptionsStub = sinon.stub(util, 'buildErrorNodeOptions').returns(errorTestItemOptions);
+            const createErrorTestItemStub = sinon.stub(testItemUtilities, 'createErrorTestItem').returns(blankTestItem);
+
+            // stub out functionality of populateTestTreeStub which is called in resolveDiscovery
+            sinon.stub(util, 'populateTestTree').returns();
+            // add spies to insure these aren't called
+            const deleteSpy = sinon.spy(testController.items, 'delete');
+            const replaceSpy = sinon.spy(testController.items, 'replace');
+            // call resolve discovery
+            let deferredTillEOT: Deferred<void> = createDeferred<void>();
+            resultResolver.resolveDiscovery(regPayload, deferredTillEOT, cancelationToken);
+            deferredTillEOT = createDeferred<void>();
+            resultResolver.resolveDiscovery(errorPayload, deferredTillEOT, cancelationToken);
+
+            // assert the stub functions were called with the correct parameters
+
+            // builds an error node root
+            sinon.assert.calledWithMatch(buildErrorNodeOptionsStub, workspaceUri, expectedErrorMessage, testProvider);
+            // builds an error item
+            sinon.assert.calledWithMatch(createErrorTestItemStub, sinon.match.any, sinon.match.any);
+
+            if (!deleteSpy.calledOnce) {
+                throw new Error("The delete method was called, but it shouldn't have been.");
+            }
+            if (replaceSpy.called) {
+                throw new Error("The replace method was called, but it shouldn't have been.");
+            }
+        });
     });
     suite('Test execution result resolver', () => {
         let resultResolver: ResultResolver.PythonResultResolver;