Skip to content

Commit

Permalink
remove node deletion for error tolerant discovery (#22207)
Browse files Browse the repository at this point in the history
helps with a part of
#21757
  • Loading branch information
eleanorjboyd authored Oct 17, 2023
1 parent ebaf8fe commit 754f8ef
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 16 deletions.
12 changes: 6 additions & 6 deletions pythonFiles/vscode_pytest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 0 additions & 8 deletions src/client/testing/testController/common/resultResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
63 changes: 63 additions & 0 deletions src/test/testing/testController/resultResolver.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 754f8ef

Please sign in to comment.