Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor errored suites reporting to new API #153

Merged
merged 2 commits into from
May 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"tmp": "^0.2.0",
"tslib": "^1.11.1",
"untildify": "^4.0.0",
"vscode-test-adapter-api": "^1.8.0",
"vscode-test-adapter-api": "^1.9.0",
"xml2js": "^0.4.23"
},
"devDependencies": {
Expand Down
14 changes: 5 additions & 9 deletions src/pytest/pytestTestCollectionParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ interface IDiscoveryResultJson {
errors: { file: string, message: number }[];
}

export function parseTestSuites(content: string, cwd: string): {
suites: (TestSuiteInfo | TestInfo)[],
errors: { id: string, message: string }[]
} {
export function parseTestSuites(content: string, cwd: string): (TestSuiteInfo | TestInfo)[] {
const from = content.indexOf(DISCOVERED_TESTS_START_MARK);
const to = content.indexOf(DISCOVERED_TESTS_END_MARK);
const discoveredTestsJson = content.substring(from + DISCOVERED_TESTS_START_MARK.length, to);
Expand Down Expand Up @@ -47,16 +44,15 @@ export function parseTestSuites(content: string, cwd: string): {
file: path.resolve(cwd, file),
message: messages.map(e => e.message).join(os.EOL),
}));
const discoveryErrorSuites = aggregatedErrors.map(({ file }) => <TestSuiteInfo | TestInfo>({
const discoveryErrorSuites = aggregatedErrors.map(({ file, message }) => <TestSuiteInfo | TestInfo>({
type: 'test' as 'test',
id: file,
file,
label: `Discovery error in ${path.basename(file)}`,
errored: true,
message,
}));
return {
suites: suites.concat(discoveryErrorSuites),
errors: aggregatedErrors.map(e => ({ id: e.file, message: e.message })),
};
return suites.concat(discoveryErrorSuites);
}

interface ITestCaseSplit {
Expand Down
36 changes: 12 additions & 24 deletions src/pytest/pytestTestRunner.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import * as os from 'os';
import * as path from 'path';
import * as tmp from 'tmp';
import {
TestEvent
TestEvent, TestSuiteInfo
} from 'vscode-test-adapter-api';

import { ArgumentParser } from 'argparse';
import { IWorkspaceConfiguration } from '../configuration/workspaceConfiguration';
import { EnvironmentVariablesLoader } from '../environmentVariablesLoader';
import { ILogger } from '../logging/logger';
import { IProcessExecution, runScript } from '../pythonRunner';
import { IDebugConfiguration, IDiscoveryResult, ITestRunner } from '../testRunner';
import { IDebugConfiguration, ITestRunner } from '../testRunner';
import { empty, setDescriptionForEqualLabels } from '../utilities';
import { parseTestStates } from './pytestJunitTestStatesParser';
import { parseTestSuites } from './pytestTestCollectionParser';
Expand Down Expand Up @@ -104,10 +103,10 @@ pytest.main(sys.argv[1:], plugins=[PythonTestExplorerDiscoveryOutputPlugin()])`;
};
}

public async load(config: IWorkspaceConfiguration): Promise<IDiscoveryResult> {
public async load(config: IWorkspaceConfiguration): Promise<TestSuiteInfo | undefined> {
if (!config.getPytestConfiguration().isPytestEnabled) {
this.logger.log('info', 'Pytest test discovery is disabled');
return { suite: undefined, errors: [] };
return undefined;
}
const additionalEnvironment = await EnvironmentVariablesLoader.load(config.envFile(), process.env, this.logger);
this.logger.log('info', `Discovering tests using python path '${config.pythonPath()}' in ${config.getCwd()}`);
Expand All @@ -123,29 +122,18 @@ pytest.main(sys.argv[1:], plugins=[PythonTestExplorerDiscoveryOutputPlugin()])`;
environment: additionalEnvironment,
}).complete();

const { suites, errors } = parseTestSuites(result.output, config.getCwd());
if (!empty(errors)) {
errors.forEach(error =>
this.logger.log(
'warn',
`Error while collecting tests from file ${error.id}: ${os.EOL}${error.message}`
)
);
}
if (empty(suites)) {
const tests = parseTestSuites(result.output, config.getCwd());
if (empty(tests)) {
this.logger.log('warn', 'No tests discovered');
return { suite: undefined, errors };
return undefined;
}

setDescriptionForEqualLabels(suites, path.sep);
setDescriptionForEqualLabels(tests, path.sep);
return {
suite: {
type: 'suite',
id: this.adapterId,
label: 'Pytest tests',
children: suites,
},
errors,
type: 'suite',
id: this.adapterId,
label: 'Pytest tests',
children: tests,
};
}

Expand Down
8 changes: 1 addition & 7 deletions src/pythonTestAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,11 @@ export class PythonTestAdapter implements TestAdapter {

this.testsById.clear();
const config = this.configurationFactory.get(this.workspaceFolder);
const { suite, errors } = await this.testRunner.load(config);
const suite = await this.testRunner.load(config);
this.saveToMap(suite);
this.sortTests(suite);

this.testsEmitter.fire({ type: 'finished', suite });
errors.map(e => ({
type: 'test' as 'test',
test: e.id,
state: 'errored' as 'errored',
message: e.message,
})).forEach(state => this.testStatesEmitter.fire(state));
} catch (error) {
this.logger.log('crit', `Test loading failed: ${error}`);
this.testsEmitter.fire({ type: 'finished', suite: undefined, errorMessage: error.stack });
Expand Down
7 changes: 1 addition & 6 deletions src/testRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,10 @@ export interface IDebugConfiguration {
env: IEnvironmentVariables;
}

export interface IDiscoveryResult {
suite?: TestSuiteInfo;
errors: { id: string, message: string }[];
}

export interface ITestRunner {
readonly adapterId: string;

load(config: IWorkspaceConfiguration): Promise<IDiscoveryResult>;
load(config: IWorkspaceConfiguration): Promise<TestSuiteInfo | undefined>;

run(config: IWorkspaceConfiguration, test: string): Promise<TestEvent[]>;

Expand Down
16 changes: 6 additions & 10 deletions src/unittest/unittestSuitParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,13 @@ interface IDiscoveryResultJson {
errors: { class: string, message: number }[];
}

export function parseTestSuites(content: string, cwd: string): {
suites: (TestSuiteInfo | TestInfo)[],
errors: { id: string, message: string }[]
} {
export function parseTestSuites(content: string, cwd: string): (TestSuiteInfo | TestInfo)[] {
const from = content.indexOf(DISCOVERED_TESTS_START_MARK);
const to = content.indexOf(DISCOVERED_TESTS_END_MARK);
const discoveredTestsJson = content.substring(from + DISCOVERED_TESTS_START_MARK.length, to);
const discoveryResult = JSON.parse(discoveredTestsJson) as IDiscoveryResultJson;
if (!discoveryResult) {
return { suites: [], errors: [] };
return [];
}
const allTests = (discoveryResult.tests || [])
.map(line => line.id.trim())
Expand All @@ -43,11 +40,13 @@ export function parseTestSuites(content: string, cwd: string): {
file: errorSuiteFilePathBySuiteId(cwd, e.id!.testId),
message: e.message,
}));
const discoveryErrorSuites = aggregatedErrors.map(({ id, file }) => <TestSuiteInfo | TestInfo>({
const discoveryErrorSuites = aggregatedErrors.map(({ id, file, message }) => <TestSuiteInfo | TestInfo>({
type: 'test' as 'test',
id: id.testId,
file,
label: id.testLabel,
errored: true,
message,
}));
const suites = Array.from(groupBy(allTests, t => t.suiteId).entries())
.map(([suiteId, tests]) => {
Expand All @@ -68,10 +67,7 @@ export function parseTestSuites(content: string, cwd: string): {
};
});

return {
suites: suites.concat(discoveryErrorSuites),
errors: aggregatedErrors.map(e => ({ id: e.id.testId, message: e.message })),
};
return suites.concat(discoveryErrorSuites);
}

export function parseTestStates(output: string): TestEvent[] {
Expand Down
36 changes: 12 additions & 24 deletions src/unittest/unittestTestRunner.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import * as os from 'os';
import * as path from 'path';
import {
TestEvent
TestEvent, TestSuiteInfo
} from 'vscode-test-adapter-api';

import { IWorkspaceConfiguration } from '../configuration/workspaceConfiguration';
import { EnvironmentVariablesLoader } from '../environmentVariablesLoader';
import { ILogger } from '../logging/logger';
import { IProcessExecution, runScript } from '../pythonRunner';
import { IDebugConfiguration, IDiscoveryResult, ITestRunner } from '../testRunner';
import { IDebugConfiguration, ITestRunner } from '../testRunner';
import { empty, setDescriptionForEqualLabels } from '../utilities';
import { UNITTEST_TEST_RUNNER_SCRIPT } from './unittestScripts';
import { parseTestStates, parseTestSuites } from './unittestSuitParser';
Expand Down Expand Up @@ -45,10 +44,10 @@ export class UnittestTestRunner implements ITestRunner {
}
}

public async load(config: IWorkspaceConfiguration): Promise<IDiscoveryResult> {
public async load(config: IWorkspaceConfiguration): Promise<TestSuiteInfo | undefined> {
if (!config.getUnittestConfiguration().isUnittestEnabled) {
this.logger.log('info', 'Unittest test discovery is disabled');
return { suite: undefined, errors: [] };
return undefined;
}

const additionalEnvironment = await EnvironmentVariablesLoader.load(config.envFile(), process.env, this.logger);
Expand All @@ -64,32 +63,21 @@ export class UnittestTestRunner implements ITestRunner {
environment: additionalEnvironment,
}).complete();

const { suites, errors } = parseTestSuites(
const tests = parseTestSuites(
result.output,
path.resolve(config.getCwd(), unittestArguments.startDirectory)
);
if (!empty(errors)) {
errors.forEach(error =>
this.logger.log(
'warn',
`Error while collecting tests from file ${error.id}: ${os.EOL}${error.message}`
)
);
}
if (empty(suites)) {
if (empty(tests)) {
this.logger.log('warn', 'No tests discovered');
return { suite: undefined, errors: [] };
return undefined;
}
setDescriptionForEqualLabels(suites, '.');
setDescriptionForEqualLabels(tests, '.');

return {
suite: {
type: 'suite',
id: this.adapterId,
label: 'Unittest tests',
children: suites,
},
errors,
type: 'suite',
id: this.adapterId,
label: 'Unittest tests',
children: tests,
};
}

Expand Down
6 changes: 2 additions & 4 deletions test/tests/environmentParsing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ import { findWorkspaceFolder, logger } from '../utils/helpers';
},
};
const suites = await runner.load(config);
expect(suites.suite).to.be.undefined;
expect(suites.errors).to.be.empty;
expect(suites).to.be.undefined;
});

test('should not fail on not existent .env file', async () => {
Expand Down Expand Up @@ -80,8 +79,7 @@ import { findWorkspaceFolder, logger } from '../utils/helpers';
},
};
const suites = await runner.load(config);
expect(suites.suite).to.be.undefined;
expect(suites.errors).to.be.empty;
expect(suites).to.be.undefined;
});
});
});
Loading