From da8aaa09cc42b2e2a5fa630d53ce1700d24afa07 Mon Sep 17 00:00:00 2001 From: Nikolay Kondratyev <4085884+kondratyev-nv@users.noreply.github.com> Date: Wed, 27 May 2020 15:55:15 +0300 Subject: [PATCH 1/2] Refactor errored suites reporting to new API --- src/pytest/pytestTestCollectionParser.ts | 14 ++---- src/pytest/pytestTestRunner.ts | 36 +++++--------- src/pythonTestAdapter.ts | 8 +--- src/testRunner.ts | 7 +-- src/unittest/unittestSuitParser.ts | 16 +++---- src/unittest/unittestTestRunner.ts | 36 +++++--------- test/tests/environmentParsing.test.ts | 6 +-- test/tests/pytestArguments.test.ts | 38 +++++++-------- test/tests/pytestGeneral.test.ts | 37 +++++++------- test/tests/pythonTestAdapter.test.ts | 61 +++--------------------- test/tests/testCancellation.test.ts | 9 ++-- test/tests/unittestGeneral.test.ts | 53 ++++++++++---------- test/tests/unittestSuitParser.test.ts | 17 +++---- test/utils/helpers.ts | 18 +++++++ 14 files changed, 141 insertions(+), 215 deletions(-) diff --git a/src/pytest/pytestTestCollectionParser.ts b/src/pytest/pytestTestCollectionParser.ts index 24d319b..34e2c6a 100644 --- a/src/pytest/pytestTestCollectionParser.ts +++ b/src/pytest/pytestTestCollectionParser.ts @@ -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); @@ -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 }) => ({ + const discoveryErrorSuites = aggregatedErrors.map(({ file, message }) => ({ 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 { diff --git a/src/pytest/pytestTestRunner.ts b/src/pytest/pytestTestRunner.ts index 92769e5..b9814ce 100644 --- a/src/pytest/pytestTestRunner.ts +++ b/src/pytest/pytestTestRunner.ts @@ -1,8 +1,7 @@ -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'; @@ -10,7 +9,7 @@ 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'; @@ -104,10 +103,10 @@ pytest.main(sys.argv[1:], plugins=[PythonTestExplorerDiscoveryOutputPlugin()])`; }; } - public async load(config: IWorkspaceConfiguration): Promise { + public async load(config: IWorkspaceConfiguration): Promise { 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()}`); @@ -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, }; } diff --git a/src/pythonTestAdapter.ts b/src/pythonTestAdapter.ts index b0bb581..90a1871 100644 --- a/src/pythonTestAdapter.ts +++ b/src/pythonTestAdapter.ts @@ -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 }); diff --git a/src/testRunner.ts b/src/testRunner.ts index e32f199..382d21b 100644 --- a/src/testRunner.ts +++ b/src/testRunner.ts @@ -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; + load(config: IWorkspaceConfiguration): Promise; run(config: IWorkspaceConfiguration, test: string): Promise; diff --git a/src/unittest/unittestSuitParser.ts b/src/unittest/unittestSuitParser.ts index 35481c7..1113c83 100644 --- a/src/unittest/unittestSuitParser.ts +++ b/src/unittest/unittestSuitParser.ts @@ -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()) @@ -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 }) => ({ + const discoveryErrorSuites = aggregatedErrors.map(({ id, file, message }) => ({ 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]) => { @@ -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[] { diff --git a/src/unittest/unittestTestRunner.ts b/src/unittest/unittestTestRunner.ts index 49c8eee..c1eb21a 100644 --- a/src/unittest/unittestTestRunner.ts +++ b/src/unittest/unittestTestRunner.ts @@ -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'; @@ -45,10 +44,10 @@ export class UnittestTestRunner implements ITestRunner { } } - public async load(config: IWorkspaceConfiguration): Promise { + public async load(config: IWorkspaceConfiguration): Promise { 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); @@ -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, }; } diff --git a/test/tests/environmentParsing.test.ts b/test/tests/environmentParsing.test.ts index 0b80841..501924c 100644 --- a/test/tests/environmentParsing.test.ts +++ b/test/tests/environmentParsing.test.ts @@ -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 () => { @@ -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; }); }); }); diff --git a/test/tests/pytestArguments.test.ts b/test/tests/pytestArguments.test.ts index adcd5b2..6402ef8 100644 --- a/test/tests/pytestArguments.test.ts +++ b/test/tests/pytestArguments.test.ts @@ -4,7 +4,7 @@ import * as path from 'path'; import { IWorkspaceConfiguration } from '../../src/configuration/workspaceConfiguration'; import { PytestTestRunner } from '../../src/pytest/pytestTestRunner'; -import { createPytestConfiguration, extractExpectedState, findTestSuiteByLabel, logger } from '../utils/helpers'; +import { createPytestConfiguration, extractExpectedState, extractErroredTests, findTestSuiteByLabel, logger } from '../utils/helpers'; suite('Pytest test discovery with additional arguments', async () => { const config: IWorkspaceConfiguration = createPytestConfiguration( @@ -22,9 +22,9 @@ suite('Pytest test discovery with additional arguments', async () => { const runner = new PytestTestRunner('some-id', logger()); test('should discover tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const expectedSuites = [ 'arithmetic.py', 'describe_test.py', @@ -59,9 +59,9 @@ suite('Run pytest tests with additional arguments', () => { const runner = new PytestTestRunner('some-id', logger()); test('should run all tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Pytest tests'); const states = await runner.run(config, runner.adapterId); expect(states).to.be.not.empty; @@ -81,9 +81,9 @@ suite('Run pytest tests with additional arguments', () => { } ].forEach(({ suite, cases }) => { test(`should run doctest ${suite} suite`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suiteToRun = findTestSuiteByLabel(mainSuite!, suite); expect(suiteToRun).to.be.not.undefined; const states = await runner.run(config, suiteToRun!.id); @@ -104,9 +104,9 @@ suite('Run pytest tests with additional arguments', () => { 'arithmetic.add_failed' ].forEach(testMethod => { test(`should run doctest ${testMethod} test`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suite = findTestSuiteByLabel(mainSuite!, testMethod); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); @@ -134,9 +134,9 @@ suite('Filter pytest tests by mark arguments', () => { '-m', 'add_test_passed' ]); - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Pytest tests'); const labels = mainSuite!.children.map(x => x.file); expect(labels).to.have.members(markedTests.map(t => path.join(config.getCwd(), t.module))); @@ -150,9 +150,9 @@ suite('Filter pytest tests by mark arguments', () => { '-m', 'add_test_passed' ]); - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Pytest tests'); const states = await runner.run(config, runner.adapterId); expect(states).to.be.not.empty; @@ -172,9 +172,9 @@ suite('Filter pytest tests by mark arguments', () => { '-m', 'not add_test_passed' ]); - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Pytest tests'); const states = await runner.run(config, runner.adapterId); expect(states).to.be.not.empty; @@ -199,9 +199,9 @@ suite('Pytest tests with additional positional arguments', () => { const runner = new PytestTestRunner('some-id', logger()); test('should discover tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const expectedSuites = [ 'add_test.py', 'add_test.py' @@ -211,9 +211,9 @@ suite('Pytest tests with additional positional arguments', () => { }); test('should run all tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Pytest tests'); const states = await runner.run(config, runner.adapterId); expect(states).to.be.not.empty; diff --git a/test/tests/pytestGeneral.test.ts b/test/tests/pytestGeneral.test.ts index 9f76af8..a8f1276 100644 --- a/test/tests/pytestGeneral.test.ts +++ b/test/tests/pytestGeneral.test.ts @@ -4,7 +4,7 @@ import * as path from 'path'; import { IWorkspaceConfiguration } from '../../src/configuration/workspaceConfiguration'; import { PytestTestRunner } from '../../src/pytest/pytestTestRunner'; -import { createPytestConfiguration, extractExpectedState, findTestSuiteByLabel, logger } from '../utils/helpers'; +import { createPytestConfiguration, extractExpectedState, extractErroredTests, findTestSuiteByLabel, logger } from '../utils/helpers'; suite('Pytest test discovery with errors', async () => { const config: IWorkspaceConfiguration = createPytestConfiguration( @@ -13,7 +13,7 @@ suite('Pytest test discovery with errors', async () => { const runner = new PytestTestRunner('some-id', logger()); test('should discover tests with errors', async () => { - const { suite: mainSuite } = await runner.load(config); + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; const expectedSuites = [ 'describe_test.py', @@ -44,7 +44,7 @@ suite('Run pytest tests with discovery errors', () => { 'Discovery error in non_existing_module_test.py' ].forEach(testMethod => { test(`should run ${testMethod} test`, async () => { - const { suite: mainSuite } = await runner.load(config); + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; const suite = findTestSuiteByLabel(mainSuite!, testMethod); expect(suite).to.be.not.undefined; @@ -72,23 +72,22 @@ suite('Pytest test discovery', async () => { 'python_extension_configured_pytest' ); expect(runner).to.be.not.null; - const { suite: mainSuite, errors } = await runner.load(configForEmptySuiteCollection); - expect(errors).to.be.empty; + const mainSuite = await runner.load(configForEmptySuiteCollection); expect(mainSuite).to.be.undefined; }); test('should discover any tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Pytest tests'); expect(mainSuite!.children).to.be.not.empty; }); test('should discover tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const expectedSuites = [ 'describe_test.py', 'env_variables_test.py', @@ -113,9 +112,9 @@ suite('Run pytest tests', () => { const runner = new PytestTestRunner('some-id', logger()); test('should run all tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Pytest tests'); const states = await runner.run(config, runner.adapterId); expect(states).to.be.not.empty; @@ -168,9 +167,9 @@ suite('Run pytest tests', () => { } ].forEach(({ suite, cases }) => { test(`should run ${suite.label} suite`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suiteToRun = findTestSuiteByLabel(mainSuite!, suite.label, suite.description); expect(suiteToRun).to.be.not.undefined; const states = await runner.run(config, suiteToRun!.id); @@ -197,9 +196,9 @@ suite('Run pytest tests', () => { 'removes_item_from_list_passed' ].forEach(testMethod => { test(`should run ${testMethod} test`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suite = findTestSuiteByLabel(mainSuite!, testMethod); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); @@ -212,9 +211,9 @@ suite('Run pytest tests', () => { }); test('should capture output from failing test', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suite = findTestSuiteByLabel(mainSuite!, 'test_two_plus_two_is_five_failed'); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); @@ -234,9 +233,9 @@ suite('Run pytest tests', () => { 'test_environment_variable_from_process_passed' ].forEach(testMethod => { test(`should load evironment variables for ${testMethod} test`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suite = findTestSuiteByLabel(mainSuite!, testMethod); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); diff --git a/test/tests/pythonTestAdapter.test.ts b/test/tests/pythonTestAdapter.test.ts index 8f92763..382a493 100644 --- a/test/tests/pythonTestAdapter.test.ts +++ b/test/tests/pythonTestAdapter.test.ts @@ -13,6 +13,7 @@ import { createPytestConfiguration, createUnittestConfiguration, extractExpectedState, + extractErroredTests, findTestSuiteByLabel, findWorkspaceFolder, logger @@ -95,7 +96,7 @@ import { test(`test execution events should be successfully fired for ${label}`, async () => { const adapter = new PythonTestAdapter(workspaceFolder, runner, configurationFactory, logger()); - const { suite: mainSuite } = await runner.load(configurationFactory.get(workspaceFolder)); + const mainSuite = await runner.load(configurationFactory.get(workspaceFolder)); // expect(errors).to.be.empty; expect(mainSuite).to.be.not.undefined; const suites = testsToRun.map(t => findTestSuiteByLabel(mainSuite!, t)!); @@ -193,18 +194,6 @@ suite('Adapter events with pytest runner and invalid files during discovery', () finishedEvent = event; } }); - const states: TestEvent[] = []; - adapter.testStates(event => { - if (event.type === 'started') { - startedNotifications++; - } else if (event.type === 'finished') { - finishedNotifications++; - } else if (event.type === 'test') { - states.push(event); - } else { - /* */ - } - }); await adapter.load(); expect(startedNotifications).to.be.eq(1); @@ -213,23 +202,12 @@ suite('Adapter events with pytest runner and invalid files during discovery', () expect(finishedEvent!.errorMessage).to.be.undefined; expect(finishedEvent!.suite).to.be.not.undefined; expect(finishedEvent!.suite!.children).to.be.not.empty; - expect(states).to.have.length(2); - expect(states.map(s => ({ state: s.state, id: s.test }))).to.have.deep.members([ - { - state: 'errored', - id: path.join(workspaceFolder.uri.fsPath, 'test', 'import_error_tests', 'invalid_syntax_test.py'), - }, - { - state: 'errored', - id: path.join(workspaceFolder.uri.fsPath, 'test', 'import_error_tests', 'non_existing_module_test.py'), - } - ]); }); test('test execution events should be successfully fired for pytest', async () => { - const { suite: mainSuite, errors } = await runner.load(configurationFactory.get(workspaceFolder)); - expect(errors).to.have.length(2); + const mainSuite = await runner.load(configurationFactory.get(workspaceFolder)); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.have.length(2); const suites = testsToRun.map(t => findTestSuiteByLabel(mainSuite!, t)!); let startedNotifications = 0; @@ -300,18 +278,6 @@ suite('Adapter events with unittest runner and invalid files during discovery', finishedEvent = event; } }); - const states: TestEvent[] = []; - adapter.testStates(event => { - if (event.type === 'started') { - startedNotifications++; - } else if (event.type === 'finished') { - finishedNotifications++; - } else if (event.type === 'test') { - states.push(event); - } else { - /* */ - } - }); await adapter.load(); expect(startedNotifications).to.be.eq(1); @@ -320,27 +286,12 @@ suite('Adapter events with unittest runner and invalid files during discovery', expect(finishedEvent!.errorMessage).to.be.undefined; expect(finishedEvent!.suite).to.be.not.undefined; expect(finishedEvent!.suite!.children).to.be.not.empty; - expect(states).to.have.length(3); - expect(states.map(s => ({ state: s.state, id: s.test }))).to.have.deep.members([ - { - state: 'errored', - id: 'invalid_tests.test_invalid_syntax_failed', - }, - { - state: 'errored', - id: 'invalid_tests.test_invalid_test_id.InvalidTestIdTests_failed', - }, - { - state: 'errored', - id: 'test_invalid_import_failed', - } - ]); }); test('test execution events should be successfully fired for unittest', async () => { - const { suite: mainSuite, errors } = await runner.load(configurationFactory.get(workspaceFolder)); - expect(errors).to.have.length(3); + const mainSuite = await runner.load(configurationFactory.get(workspaceFolder)); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.have.length(3); const suites = testsToRun.map(t => findTestSuiteByLabel(mainSuite!, t)!); let startedNotifications = 0; diff --git a/test/tests/testCancellation.test.ts b/test/tests/testCancellation.test.ts index 4b8f188..6fc5558 100644 --- a/test/tests/testCancellation.test.ts +++ b/test/tests/testCancellation.test.ts @@ -9,6 +9,7 @@ import { createPytestConfiguration, createUnittestConfiguration, extractExpectedState, + extractErroredTests, findTestSuiteByLabel, logger, sleep @@ -31,9 +32,9 @@ import { suite(`Test cancellation with ${label}`, () => { test('should run and cancel all tests', async () => { - const { suite: mainSuite, errors } = await runner.load(configuration); - expect(errors).to.be.empty; + const mainSuite = await runner.load(configuration); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const statesPromise = runner.run(configuration, mainSuite!.id); await sleep(1000); runner.cancel(); @@ -51,9 +52,9 @@ import { }); test('should run and cancel single test', async () => { - const { suite: mainSuite, errors } = await runner.load(configuration); - expect(errors).to.be.empty; + const mainSuite = await runner.load(configuration); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suite = findTestSuiteByLabel(mainSuite!, 'test_sleep'); expect(suite).to.be.not.undefined; const statesPromise = runner.run(configuration, suite!.id); diff --git a/test/tests/unittestGeneral.test.ts b/test/tests/unittestGeneral.test.ts index 125b6ac..283e4af 100644 --- a/test/tests/unittestGeneral.test.ts +++ b/test/tests/unittestGeneral.test.ts @@ -4,7 +4,7 @@ import * as vscode from 'vscode'; import { IWorkspaceConfiguration } from '../../src/configuration/workspaceConfiguration'; import { UnittestTestRunner } from '../../src/unittest/unittestTestRunner'; -import { createUnittestConfiguration, extractExpectedState, findTestSuiteByLabel, logger } from '../utils/helpers'; +import { createUnittestConfiguration, extractExpectedState, extractErroredTests, findTestSuiteByLabel, logger } from '../utils/helpers'; suite('Unittest test discovery', () => { const config: IWorkspaceConfiguration = createUnittestConfiguration('unittest'); @@ -19,23 +19,22 @@ suite('Unittest test discovery', () => { const configForEmptySuiteCollection: IWorkspaceConfiguration = createUnittestConfiguration( 'python_extension_configured_unittest' ); - const { suite: mainSuite, errors } = await runner.load(configForEmptySuiteCollection); - expect(errors).to.be.empty; + const mainSuite = await runner.load(configForEmptySuiteCollection); expect(mainSuite).to.be.undefined; }); test('should discover any tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.not.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.not.empty; expect(mainSuite!.label).to.be.eq('Unittest tests'); expect(mainSuite!.children).to.be.not.empty; }); test('should discover tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.not.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.not.empty; const expectedSuites = [ 'TestWithOutputBeforeImport', 'TestWithSetUpClassMethod', @@ -67,9 +66,11 @@ suite('Unittest test discovery', () => { } ].forEach(({ testId, error }) => { test(`should show errors for invalid test ${testId}`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.not.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + + const errors = extractErroredTests(mainSuite!); + expect(errors).to.be.not.empty; expect(errors.map(x => x.id)).to.contain(testId); const invalidTest = errors.filter(x => x.id === testId)[0]; expect(invalidTest.message).to.match(error); @@ -82,9 +83,9 @@ suite('Run unittest tests', () => { const runner = new UnittestTestRunner('some-id', logger()); test('should run all tests', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.not.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.not.empty; expect(mainSuite!.label).to.be.eq('Unittest tests'); const states = await runner.run(config, mainSuite!.id); expect(states).to.be.not.empty; @@ -100,9 +101,9 @@ suite('Run unittest tests', () => { { testCase: 'AddTests', description: 'basic_tests.test_add' } ].forEach(({ testCase, description }) => { test(`should run ${testCase} suite`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.not.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.not.empty; const suite = findTestSuiteByLabel(mainSuite!, testCase, description); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); @@ -122,9 +123,9 @@ suite('Run unittest tests', () => { 'test_set_up_called_before_test_case2_passed' ].forEach(testMethod => { test(`should run ${testMethod} test`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.not.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.not.empty; const suite = findTestSuiteByLabel(mainSuite!, testMethod); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); @@ -141,9 +142,9 @@ suite('Run unittest tests', () => { 'test_basic_two_plus_two_is_five_failed' ].forEach(testMethod => { test(`should capture output from ${testMethod} test`, async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.not.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.not.empty; const suite = findTestSuiteByLabel(mainSuite!, testMethod); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); @@ -162,7 +163,7 @@ suite('Run unittest tests', () => { }); test('should run single test even when test name if the prefix for the other', async () => { - const { suite: mainSuite } = await runner.load(config); + const mainSuite = await runner.load(config); const testToRun = findTestSuiteByLabel(mainSuite!, 'test_string_passed'); expect(testToRun).to.be.not.undefined; const testToSkip = findTestSuiteByLabel(mainSuite!, 'test_string_passed_capitalize_passed'); @@ -206,9 +207,9 @@ suite('Unittest run and discovery with start folder in config', () => { const runner = new UnittestTestRunner('some-id', logger()); test('should discover tests with start folder in config', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const expectedSuites = [ 'AddTestsWithoutInit' ]; @@ -217,9 +218,9 @@ suite('Unittest run and discovery with start folder in config', () => { }); test('should run all tests with start folder in config', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; expect(mainSuite!.label).to.be.eq('Unittest tests'); const states = await runner.run(config, mainSuite!.id); expect(states).to.be.not.empty; @@ -230,9 +231,9 @@ suite('Unittest run and discovery with start folder in config', () => { }); test('should run suite with start folder in config', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suite = findTestSuiteByLabel(mainSuite!, 'AddTestsWithoutInit'); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); @@ -244,9 +245,9 @@ suite('Unittest run and discovery with start folder in config', () => { }); test('should run test from suite with start folder in config', async () => { - const { suite: mainSuite, errors } = await runner.load(config); - expect(errors).to.be.empty; + const mainSuite = await runner.load(config); expect(mainSuite).to.be.not.undefined; + expect(extractErroredTests(mainSuite!)).to.be.empty; const suite = findTestSuiteByLabel(mainSuite!, 'test_two_plus_one_is_three_passed'); expect(suite).to.be.not.undefined; const states = await runner.run(config, suite!.id); diff --git a/test/tests/unittestSuitParser.test.ts b/test/tests/unittestSuitParser.test.ts index 2e178d6..21fbcba 100644 --- a/test/tests/unittestSuitParser.test.ts +++ b/test/tests/unittestSuitParser.test.ts @@ -4,10 +4,11 @@ import * as path from 'path'; import { TestSuiteInfo } from 'vscode-test-adapter-api'; import { parseTestStates, parseTestSuites } from '../../src/unittest/unittestSuitParser'; +import { extractErroredTestsFromArray } from '../utils/helpers'; suite('Unittest suite parser', () => { test('should return empty suite when input is empty', () => { - const { suites, errors } = parseTestSuites( + const suites = parseTestSuites( `some string without dots ==DISCOVERED TESTS BEGIN== { "tests": [], "errors": [] } @@ -15,7 +16,7 @@ suite('Unittest suite parser', () => { '/some/prefix/path' ); expect(suites).to.be.empty; - expect(errors).to.be.empty; + expect(extractErroredTestsFromArray(suites)).to.be.empty; }); test('should create single test and suite for a single test', () => { @@ -25,11 +26,11 @@ suite('Unittest suite parser', () => { const expectedTestLabel = 'test_function'; const expectedTestId = expectedSuitId + '.test_function'; - const { suites, errors } = parseTestSuites( + const suites = parseTestSuites( '==DISCOVERED TESTS BEGIN=={ "tests": [{ "id": "some_test_module.TestCase1.test_function" }], "errors": [] }==DISCOVERED TESTS END==', prefixPath ); - expect(errors).to.be.empty; + expect(extractErroredTestsFromArray(suites)).to.be.empty; expect(suites).to.have.length(1); expect(suites[0].type).to.be.eq('suite'); @@ -60,12 +61,12 @@ suite('Unittest suite parser', () => { label, })); - const { suites, errors } = parseTestSuites( + const suites = parseTestSuites( '==DISCOVERED TESTS BEGIN=={ "tests": [' + expectedTests.map(t => (`{ "id": "${t.id}" }`)).join(',\n') + '], "errors": [] }==DISCOVERED TESTS END==', prefixPath ); - expect(errors).to.be.empty; expect(suites).to.have.length(1); + expect(extractErroredTestsFromArray(suites)).to.be.empty; expect(suites[0].type).to.be.eq('suite'); const singleSuit: TestSuiteInfo = suites[0] as TestSuiteInfo; @@ -95,11 +96,11 @@ suite('Unittest suite parser', () => { label, })); - const { suites, errors } = parseTestSuites( + const suites = parseTestSuites( '==DISCOVERED TESTS BEGIN=={ "tests": [' + expectedTests.map(t => (`{ "id": "${t.id}" }`)).join(',\n') + '], "errors": [] }==DISCOVERED TESTS END==', prefixPath ); - expect(errors).to.be.empty; + expect(extractErroredTestsFromArray(suites)).to.be.empty; expect(suites).to.have.length(1); expect(suites[0].type).to.be.eq('suite'); diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts index ba38b18..fa3658e 100644 --- a/test/utils/helpers.ts +++ b/test/utils/helpers.ts @@ -20,6 +20,24 @@ export function logger(): ILogger { }, }; } +export function extractErroredTestsFromArray(tests: (TestSuiteInfo | TestInfo)[]): (TestSuiteInfo | TestInfo)[] { + let errors: (TestSuiteInfo | TestInfo)[] = []; + for (const test of tests) { + errors = errors.concat(extractErroredTests(test)); + } + return errors; +} + +export function extractErroredTests(suite: TestSuiteInfo | TestInfo): (TestSuiteInfo | TestInfo)[] { + let errors = []; + if (suite.errored) { + errors.push(suite); + } + if (suite.type === 'suite') { + errors = errors.concat(extractErroredTestsFromArray(suite.children)); + } + return errors; +} export function extractExpectedState(name: string) { if (name.includes('[')) { From 1a2beeba0a90cc9c4640f203992664b281d3c324 Mon Sep 17 00:00:00 2001 From: Nikolay Kondratyev <4085884+kondratyev-nv@users.noreply.github.com> Date: Sat, 30 May 2020 20:21:00 +0300 Subject: [PATCH 2/2] Fix test explorer API version --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index a68de3c..e16ebf2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1717,9 +1717,9 @@ } }, "vscode-test-adapter-api": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/vscode-test-adapter-api/-/vscode-test-adapter-api-1.8.0.tgz", - "integrity": "sha512-sNSxc2ec0JHl7PACebJwg/Ew/3FiAXBndHDS4Zp27l893wdIyhSeZL9y7j57YV6dDUkNnaRg2QcyIIBvlk8obw==" + "version": "1.9.0", + "resolved": "https://registry.npmjs.org/vscode-test-adapter-api/-/vscode-test-adapter-api-1.9.0.tgz", + "integrity": "sha512-lltjehUP0J9H3R/HBctjlqeUCwn2t9Lbhj2Y500ib+j5Y4H3hw+hVTzuSsfw16LtxY37knlU39QIlasa7svzOQ==" }, "which": { "version": "2.0.2", diff --git a/package.json b/package.json index 1b200bc..0fb2cf3 100644 --- a/package.json +++ b/package.json @@ -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": {