From 354b3dbb4413c94a8a320ec76bd487d7ed681906 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Tue, 26 Jun 2018 10:02:11 -0700 Subject: [PATCH 01/13] Alter the discovery of tests such that tests are properly grouped within their suite --- src/client/unittests/main.ts | 1 + src/client/unittests/unittest/services/parserService.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/unittests/main.ts b/src/client/unittests/main.ts index 2f76ad448f9f..c803425ec1fa 100644 --- a/src/client/unittests/main.ts +++ b/src/client/unittests/main.ts @@ -91,6 +91,7 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di if (this.testResultDisplay) { this.testResultDisplay.enabled = false; } + // tslint:disable-next-line:no-suspicious-comment // TODO: Why are we disposing, what happens when tests are enabled. if (this.workspaceTestManagerService) { this.workspaceTestManagerService.dispose(); diff --git a/src/client/unittests/unittest/services/parserService.ts b/src/client/unittests/unittest/services/parserService.ts index 1b4acdd194c9..6022a8cf9b29 100644 --- a/src/client/unittests/unittest/services/parserService.ts +++ b/src/client/unittests/unittest/services/parserService.ts @@ -79,7 +79,7 @@ export class TestsParser implements ITestsParser { } // Check if we already have this test file - const classNameToRun = className; + const classNameToRun = `${path.parse(filePath).name}.${className}`; let testSuite = testFile.suites.find(cls => cls.nameToRun === classNameToRun); if (!testSuite) { testSuite = { @@ -88,7 +88,7 @@ export class TestsParser implements ITestsParser { suites: [], isUnitTest: true, isInstance: false, - nameToRun: `${path.parse(filePath).name}.${classNameToRun}`, + nameToRun: classNameToRun, xmlName: '', status: TestStatus.Idle, time: 0 From d6688a871be8f9274fd59a5de01b05431fb9b882 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Tue, 26 Jun 2018 14:49:18 -0700 Subject: [PATCH 02/13] Avoid making local variable being overriden by a for-loop variable --- .../visualstudio_py_testlauncher.py | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/pythonFiles/PythonTools/visualstudio_py_testlauncher.py b/pythonFiles/PythonTools/visualstudio_py_testlauncher.py index 7ed86ecaa320..270d87be3399 100644 --- a/pythonFiles/PythonTools/visualstudio_py_testlauncher.py +++ b/pythonFiles/PythonTools/visualstudio_py_testlauncher.py @@ -1,16 +1,16 @@ # Python Tools for Visual Studio # Copyright(c) Microsoft Corporation # All rights reserved. -# +# # Licensed under the Apache License, Version 2.0 (the License); you may not use # this file except in compliance with the License. You may obtain a copy of the # License at http://www.apache.org/licenses/LICENSE-2.0 -# +# # THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS # OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY # IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, # MERCHANTABLITY OR NON-INFRINGEMENT. -# +# # See the Apache Version 2.0 License for specific language governing # permissions and limitations under the License. @@ -43,11 +43,11 @@ def __init__(self, old_out, is_stdout): def flush(self): if self.old_out: self.old_out.flush() - + def writelines(self, lines): for line in lines: self.write(line) - + @property def encoding(self): return 'utf8' @@ -58,13 +58,13 @@ def write(self, value): self.old_out.write(value) # flush immediately, else things go wonky and out of order self.flush() - + def isatty(self): return True def next(self): pass - + @property def name(self): if self.is_stdout: @@ -84,7 +84,7 @@ def write(self, data): _channel.send_event('stdout' if self.is_stdout else 'stderr', content=data) self.buffer.write(data) - def flush(self): + def flush(self): self.buffer.flush() def truncate(self, pos = None): @@ -137,7 +137,7 @@ def startTest(self, test): super(VsTestResult, self).startTest(test) if _channel is not None: _channel.send_event( - name='start', + name='start', test = test.id() ) @@ -177,7 +177,7 @@ def sendResult(self, test, outcome, trace = None): tb = ''.join(formatted) message = str(trace[1]) _channel.send_event( - name='result', + name='result', outcome=outcome, traceback = tb, message = message, @@ -196,7 +196,7 @@ def stopTests(): class ExitCommand(Exception): pass -def signal_handler(signal, frame): +def signal_handler(signal, frame): raise ExitCommand() def main(): @@ -223,7 +223,7 @@ def main(): if opts.debug: from ptvsd.visualstudio_py_debugger import DONT_DEBUG, DEBUG_ENTRYPOINTS, get_code - + sys.path[0] = os.getcwd() if opts.result_port: try: @@ -243,7 +243,7 @@ def main(): pass elif opts.mixed_mode: - # For mixed-mode attach, there's no ptvsd and hence no wait_for_attach(), + # For mixed-mode attach, there's no ptvsd and hence no wait_for_attach(), # so we have to use Win32 API in a loop to do the same thing. from time import sleep from ctypes import windll, c_char @@ -278,43 +278,44 @@ def main(): opts.up = 'test*.py' tests = unittest.defaultTestLoader.discover(opts.us, opts.up) else: - # loadTestsFromNames doesn't work well (with duplicate file names or class names) + # loadTestsFromNames doesn't work well (with duplicate file names or class names) # Easier approach is find the test suite and use that for running loader = unittest.TestLoader() # opts.us will be passed in suites = loader.discover(opts.us, pattern=os.path.basename(opts.testFile)) suite = None - tests = None + tests = None if opts.tests is None: # Run everything in the test file tests = suites else: # Run a specific test class or test method - for suite in suites._tests: - for cls in suite._tests: + for test_suite in suites._tests: + for cls in test_suite._tests: try: for m in cls._tests: testId = m.id() if testId.startswith(opts.tests[0]): suite = cls + break if testId == opts.tests[0]: tests = m break except Exception as err: - errorMessage = traceback.format_exception() + errorMessage = traceback.format_exception() pass if tests is None: tests = suite if tests is None and suite is None: _channel.send_event( - name='error', + name='error', outcome='', traceback = '', message = 'Failed to identify the test', test = '' ) if opts.uvInt is None: - opts.uvInt = 0 + opts.uvInt = 0 if opts.uf is not None: runner = unittest.TextTestRunner(verbosity=opts.uvInt, resultclass=VsTestResult, failfast=True) else: From 6265a9e41bed313c14cf1f0cb57cae87fbfc50bb Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Tue, 26 Jun 2018 15:06:03 -0700 Subject: [PATCH 03/13] Correct parsing for n-depth unit test folders --- src/client/unittests/unittest/services/parserService.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/unittests/unittest/services/parserService.ts b/src/client/unittests/unittest/services/parserService.ts index 6022a8cf9b29..0e3974485493 100644 --- a/src/client/unittests/unittest/services/parserService.ts +++ b/src/client/unittests/unittest/services/parserService.ts @@ -78,8 +78,10 @@ export class TestsParser implements ITestsParser { testFiles.push(testFile); } - // Check if we already have this test file - const classNameToRun = `${path.parse(filePath).name}.${className}`; + // Check if we already have this suite + const thePath: path.ParsedPath = path.parse(filePath); + const relativePath: string = thePath.dir.substring(rootDirectory.length + 1); + const classNameToRun: string = `${relativePath.replace(path.sep, '.')}.${thePath.name}.${className}`; let testSuite = testFile.suites.find(cls => cls.nameToRun === classNameToRun); if (!testSuite) { testSuite = { From a872f257b0dae962c864c0c9d54d5c3b3855ca07 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Tue, 26 Jun 2018 15:06:37 -0700 Subject: [PATCH 04/13] Correct linting issue reported by tslint --- src/client/unittests/unittest/socketServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/unittests/unittest/socketServer.ts b/src/client/unittests/unittest/socketServer.ts index da83a350e61a..c75e33c46c4a 100644 --- a/src/client/unittests/unittest/socketServer.ts +++ b/src/client/unittests/unittest/socketServer.ts @@ -29,7 +29,7 @@ export class UnitTestSocketServer extends EventEmitter implements IUnitTestSocke this.server = undefined; } } - public start(options: { port?: number, host?: string } = { port: 0, host: 'localhost' }): Promise { + public start(options: { port?: number; host?: string } = { port: 0, host: 'localhost' }): Promise { this.startedDef = createDeferred(); this.server = net.createServer(this.connectionListener.bind(this)); this.server!.maxConnections = MaxConnections; From ce1f59015554ee15b43784c006b5e4d6c171601c Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 09:46:14 -0700 Subject: [PATCH 05/13] Replace my path-building solution with a reduced-testId solution instead --- .../unittests/unittest/services/parserService.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/client/unittests/unittest/services/parserService.ts b/src/client/unittests/unittest/services/parserService.ts index 0e3974485493..f74af6c846f0 100644 --- a/src/client/unittests/unittest/services/parserService.ts +++ b/src/client/unittests/unittest/services/parserService.ts @@ -60,6 +60,7 @@ export class TestsParser implements ITestsParser { const paths = testIdParts.slice(0, testIdParts.length - 2); const filePath = `${path.join(rootDirectory, ...paths)}.py`; const functionName = testIdParts.pop()!; + const suiteToRun = testIdParts.join('.'); const className = testIdParts.pop()!; // Check if we already have this test file @@ -70,7 +71,7 @@ export class TestsParser implements ITestsParser { fullPath: filePath, functions: [], suites: [], - nameToRun: `${className}.${functionName}`, + nameToRun: suiteToRun, xmlName: '', status: TestStatus.Idle, time: 0 @@ -79,10 +80,8 @@ export class TestsParser implements ITestsParser { } // Check if we already have this suite - const thePath: path.ParsedPath = path.parse(filePath); - const relativePath: string = thePath.dir.substring(rootDirectory.length + 1); - const classNameToRun: string = `${relativePath.replace(path.sep, '.')}.${thePath.name}.${className}`; - let testSuite = testFile.suites.find(cls => cls.nameToRun === classNameToRun); + // nameToRun = testId - method name + let testSuite = testFile.suites.find(cls => cls.nameToRun === suiteToRun); if (!testSuite) { testSuite = { name: className, @@ -90,7 +89,7 @@ export class TestsParser implements ITestsParser { suites: [], isUnitTest: true, isInstance: false, - nameToRun: classNameToRun, + nameToRun: suiteToRun, xmlName: '', status: TestStatus.Idle, time: 0 From 5fca6eec7c61f77137ea47552ea951633d744782 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 10:22:20 -0700 Subject: [PATCH 06/13] Add news file for bugfix --- news/2 Fixes/2044.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/2044.md diff --git a/news/2 Fixes/2044.md b/news/2 Fixes/2044.md new file mode 100644 index 000000000000..6118357acaa2 --- /dev/null +++ b/news/2 Fixes/2044.md @@ -0,0 +1 @@ +Store testId for files & suites during unittest discovery From 59b13750bb70cbb4acdd0d3e1a3ce4cb063a8412 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 14:38:26 -0700 Subject: [PATCH 07/13] Make parserService methods public to unit test them --- .../unittests/unittest/services/parserService.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/client/unittests/unittest/services/parserService.ts b/src/client/unittests/unittest/services/parserService.ts index f74af6c846f0..49df60577cd1 100644 --- a/src/client/unittests/unittest/services/parserService.ts +++ b/src/client/unittests/unittest/services/parserService.ts @@ -3,9 +3,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; -import { ITestsHelper, ITestsParser, TestDiscoveryOptions, TestFile, TestFunction, Tests, TestStatus } from '../../common/types'; - -type UnitTestParserOptions = TestDiscoveryOptions & { startDirectory: string }; +import { ITestsHelper, ITestsParser, TestDiscoveryOptions, TestFile, TestFunction, Tests, TestStatus, UnitTestParserOptions } from '../../common/types'; @injectable() export class TestsParser implements ITestsParser { @@ -18,7 +16,7 @@ export class TestsParser implements ITestsParser { } return this.parseTestIds(testsDirectory, testIds); } - private getTestIds(content: string): string[] { + public getTestIds(content: string): string[] { let startedCollecting = false; return content.split(/\r?\n/g) .map(line => { @@ -32,7 +30,7 @@ export class TestsParser implements ITestsParser { }) .filter(line => line.length > 0); } - private parseTestIds(rootDirectory: string, testIds: string[]): Tests { + public parseTestIds(rootDirectory: string, testIds: string[]): Tests { const testFiles: TestFile[] = []; testIds.forEach(testId => this.addTestId(rootDirectory, testId, testFiles)); @@ -44,13 +42,13 @@ export class TestsParser implements ITestsParser { * TestIds are fully qualified including the method names. * E.g. tone_test.Failing2Tests.test_failure * Where tone_test = folder, Failing2Tests = class/suite, test_failure = method. - * @private + * @public * @param {string} rootDirectory * @param {string[]} testIds * @returns {Tests} * @memberof TestsParser */ - private addTestId(rootDirectory: string, testId: string, testFiles: TestFile[]) { + public addTestId(rootDirectory: string, testId: string, testFiles: TestFile[]) { const testIdParts = testId.split('.'); // We must have a file, class and function name if (testIdParts.length <= 2) { From e98a47e47cac8e133f1a8f1934f79e807bf7a2fa Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 14:40:51 -0700 Subject: [PATCH 08/13] Fix linter problem --- src/client/unittests/unittest/services/parserService.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/client/unittests/unittest/services/parserService.ts b/src/client/unittests/unittest/services/parserService.ts index 49df60577cd1..30baa278ebab 100644 --- a/src/client/unittests/unittest/services/parserService.ts +++ b/src/client/unittests/unittest/services/parserService.ts @@ -3,7 +3,9 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; -import { ITestsHelper, ITestsParser, TestDiscoveryOptions, TestFile, TestFunction, Tests, TestStatus, UnitTestParserOptions } from '../../common/types'; +import { ITestsHelper, ITestsParser, TestFile, + TestFunction, Tests, TestStatus, + UnitTestParserOptions } from '../../common/types'; @injectable() export class TestsParser implements ITestsParser { From 0f740c1f121d69b0b9ea0282d6c0f0e29f22540a Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 14:42:33 -0700 Subject: [PATCH 09/13] Share UnitTestParserOptions to help testing --- src/client/unittests/common/types.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/unittests/common/types.ts b/src/client/unittests/common/types.ts index 67e2663efab9..6f11f95d490f 100644 --- a/src/client/unittests/common/types.ts +++ b/src/client/unittests/common/types.ts @@ -24,6 +24,8 @@ export type TestRunOptions = { debug?: boolean; }; +export type UnitTestParserOptions = TestDiscoveryOptions & { startDirectory: string }; + export type TestFolder = TestResult & { name: string; testFiles: TestFile[]; From 8b7b660bba266347ba1b60a8cefa7ca8d6d40d08 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 17:24:58 -0700 Subject: [PATCH 10/13] Begin adding unit tests for discovery and parsing unittest --- .../unittest/unittest.discovery.unit.test.ts | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/src/test/unittests/unittest/unittest.discovery.unit.test.ts b/src/test/unittests/unittest/unittest.discovery.unit.test.ts index b70ad76845d5..74287899d6e1 100644 --- a/src/test/unittests/unittest/unittest.discovery.unit.test.ts +++ b/src/test/unittests/unittest/unittest.discovery.unit.test.ts @@ -9,12 +9,16 @@ import { expect, use } from 'chai'; import * as chaipromise from 'chai-as-promised'; import * as path from 'path'; import * as typeMoq from 'typemoq'; -import { CancellationToken } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { IServiceContainer } from '../../../client/ioc/types'; import { UNITTEST_PROVIDER } from '../../../client/unittests/common/constants'; -import { ITestDiscoveryService, ITestRunner, ITestsParser, Options, TestDiscoveryOptions, Tests } from '../../../client/unittests/common/types'; +import { TestsHelper } from '../../../client/unittests/common/testUtils'; +import { TestFlatteningVisitor } from '../../../client/unittests/common/testVisitors/flatteningVisitor'; +import { ITestDiscoveryService, ITestRunner, ITestsParser, + Options, TestDiscoveryOptions, Tests, UnitTestParserOptions } from '../../../client/unittests/common/types'; import { IArgumentsHelper } from '../../../client/unittests/types'; import { TestDiscoveryService } from '../../../client/unittests/unittest/services/discoveryService'; +import { TestsParser } from '../../../client/unittests/unittest/services/parserService'; use(chaipromise); @@ -23,10 +27,11 @@ suite('Unit Tests - Unittest - Discovery', () => { let argsHelper: typeMoq.IMock; let testParser: typeMoq.IMock; let runner: typeMoq.IMock; + let serviceContainer: typeMoq.IMock; const dir = path.join('a', 'b', 'c'); const pattern = 'Pattern_To_Search_For'; setup(() => { - const serviceContainer = typeMoq.Mock.ofType(); + serviceContainer = typeMoq.Mock.ofType(); argsHelper = typeMoq.Mock.ofType(); testParser = typeMoq.Mock.ofType(); runner = typeMoq.Mock.ofType(); @@ -300,4 +305,45 @@ suite('Unit Tests - Unittest - Discovery', () => { runner.verifyAll(); testParser.verifyAll(); }); + test('Ensure discovery resolves test suites in n-depth directories', async () => { + const testHelper: TestsHelper = new TestsHelper(new TestFlatteningVisitor(), serviceContainer.object); + + const testsParser: TestsParser = new TestsParser(testHelper); + + const opts = typeMoq.Mock.ofType(); + const token = typeMoq.Mock.ofType(); + const wspace = typeMoq.Mock.ofType(); + opts.setup(o => o.token).returns(() => token.object); + opts.setup(o => o.workspaceFolder).returns(() => wspace.object); + token.setup(t => t.isCancellationRequested) + .returns(() => true); + opts.setup(o => o.cwd).returns(() => '/home/user/dev'); + opts.setup(o => o.startDirectory).returns(() => '/home/user/dev/tests'); + + const discoveryOutput: string = ['start', + 'apptests.debug.class_name.RootClassName.test_root', + 'apptests.debug.class_name.RootClassName.test_root_other', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first_other', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second_other', + ''].join('\n'); + + const tests: Tests = testsParser.parse(discoveryOutput, opts.object); + + expect(tests.testFiles.length).to.be.equal(3); + expect(tests.testFunctions.length).to.be.equal(6); + expect(tests.testSuites.length).to.be.equal(3); + expect(tests.testFolders.length).to.be.equal(1); + }); + + // no start directory given + // relative start directory given + // absolute start directory given + + // no content given + // corrupted/invalid content given + // correct content given + // incorrect content given (no tests in the given path) + }); From 075122bc9792fd9f66ece09999d8e28108496101 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 18:18:28 -0700 Subject: [PATCH 11/13] Add further unit tests for unittest parsing --- .../unittest/unittest.discovery.unit.test.ts | 215 +++++++++++++++++- 1 file changed, 208 insertions(+), 7 deletions(-) diff --git a/src/test/unittests/unittest/unittest.discovery.unit.test.ts b/src/test/unittests/unittest/unittest.discovery.unit.test.ts index 74287899d6e1..fe54b33f7b1b 100644 --- a/src/test/unittests/unittest/unittest.discovery.unit.test.ts +++ b/src/test/unittests/unittest/unittest.discovery.unit.test.ts @@ -335,15 +335,216 @@ suite('Unit Tests - Unittest - Discovery', () => { expect(tests.testFunctions.length).to.be.equal(6); expect(tests.testSuites.length).to.be.equal(3); expect(tests.testFolders.length).to.be.equal(1); + + // now ensure that each test function belongs within a single test suite... + tests.testFunctions.forEach(fn => { + if (fn.parentTestSuite) { + const testPrefix: boolean = fn.testFunction.nameToRun.startsWith(fn.parentTestSuite.nameToRun); + expect(testPrefix).to.equal(true, + [`function ${fn.testFunction.name} has a parent suite ${fn.parentTestSuite.name}, `, + `but the parent suite 'nameToRun' (${fn.parentTestSuite.nameToRun}) isn't the `, + `prefix to the functions 'nameToRun' (${fn.testFunction.nameToRun})`].join('')); + } + }); }); + test('Ensure discovery resolves test files in n-depth directories', async () => { + const testHelper: TestsHelper = new TestsHelper(new TestFlatteningVisitor(), serviceContainer.object); + + const testsParser: TestsParser = new TestsParser(testHelper); + + const opts = typeMoq.Mock.ofType(); + const token = typeMoq.Mock.ofType(); + const wspace = typeMoq.Mock.ofType(); + opts.setup(o => o.token).returns(() => token.object); + opts.setup(o => o.workspaceFolder).returns(() => wspace.object); + token.setup(t => t.isCancellationRequested) + .returns(() => true); + opts.setup(o => o.cwd).returns(() => '/home/user/dev'); + opts.setup(o => o.startDirectory).returns(() => '/home/user/dev/tests'); + + const discoveryOutput: string = ['start', + 'apptests.debug.class_name.RootClassName.test_root', + 'apptests.debug.class_name.RootClassName.test_root_other', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first_other', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second_other', + ''].join('\n'); + + const tests: Tests = testsParser.parse(discoveryOutput, opts.object); + + expect(tests.testFiles.length).to.be.equal(3); + expect(tests.testFunctions.length).to.be.equal(6); + expect(tests.testSuites.length).to.be.equal(3); + expect(tests.testFolders.length).to.be.equal(1); + + // now ensure that the 'nameToRun' for each test function begins with its file's a single test suite... + tests.testFunctions.forEach(fn => { + if (fn.parentTestSuite) { + const testPrefix: boolean = fn.testFunction.nameToRun.startsWith(fn.parentTestFile.nameToRun); + expect(testPrefix).to.equal(true, + [`function ${fn.testFunction.name} was found in file ${fn.parentTestFile.name}, `, + `but the parent file 'nameToRun' (${fn.parentTestFile.nameToRun}) isn't the `, + `prefix to the functions 'nameToRun' (${fn.testFunction.nameToRun})`].join('')); + } + }); + }); + test('Ensure discovery resolves test suites in n-depth directories when no start directory is given', async () => { + const testHelper: TestsHelper = new TestsHelper(new TestFlatteningVisitor(), serviceContainer.object); + + const testsParser: TestsParser = new TestsParser(testHelper); - // no start directory given - // relative start directory given - // absolute start directory given + const opts = typeMoq.Mock.ofType(); + const token = typeMoq.Mock.ofType(); + const wspace = typeMoq.Mock.ofType(); + opts.setup(o => o.token).returns(() => token.object); + opts.setup(o => o.workspaceFolder).returns(() => wspace.object); + token.setup(t => t.isCancellationRequested) + .returns(() => true); + opts.setup(o => o.cwd).returns(() => '/home/user/dev'); + opts.setup(o => o.startDirectory).returns(() => ''); + + const discoveryOutput: string = ['start', + 'apptests.debug.class_name.RootClassName.test_root', + 'apptests.debug.class_name.RootClassName.test_root_other', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first_other', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second_other', + ''].join('\n'); - // no content given - // corrupted/invalid content given - // correct content given - // incorrect content given (no tests in the given path) + const tests: Tests = testsParser.parse(discoveryOutput, opts.object); + expect(tests.testFiles.length).to.be.equal(3); + expect(tests.testFunctions.length).to.be.equal(6); + expect(tests.testSuites.length).to.be.equal(3); + expect(tests.testFolders.length).to.be.equal(1); + + // now ensure that each test function belongs within a single test suite... + tests.testFunctions.forEach(fn => { + if (fn.parentTestSuite) { + const testPrefix: boolean = fn.testFunction.nameToRun.startsWith(fn.parentTestSuite.nameToRun); + expect(testPrefix).to.equal(true, + [`function ${fn.testFunction.name} has a parent suite ${fn.parentTestSuite.name}, `, + `but the parent suite 'nameToRun' (${fn.parentTestSuite.nameToRun}) isn't the `, + `prefix to the functions 'nameToRun' (${fn.testFunction.nameToRun})`].join('')); + } + }); + }); + test('Ensure discovery resolves test suites in n-depth directories when a relative start directory is given', async () => { + const testHelper: TestsHelper = new TestsHelper(new TestFlatteningVisitor(), serviceContainer.object); + + const testsParser: TestsParser = new TestsParser(testHelper); + + const opts = typeMoq.Mock.ofType(); + const token = typeMoq.Mock.ofType(); + const wspace = typeMoq.Mock.ofType(); + opts.setup(o => o.token).returns(() => token.object); + opts.setup(o => o.workspaceFolder).returns(() => wspace.object); + token.setup(t => t.isCancellationRequested) + .returns(() => true); + opts.setup(o => o.cwd).returns(() => '/home/user/dev'); + opts.setup(o => o.startDirectory).returns(() => './tests'); + + const discoveryOutput: string = ['start', + 'apptests.debug.class_name.RootClassName.test_root', + 'apptests.debug.class_name.RootClassName.test_root_other', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first', + 'apptests.debug.first.class_name.FirstLevelClassName.test_first_other', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second', + 'apptests.debug.first.second.class_name.SecondLevelClassName.test_second_other', + ''].join('\n'); + + const tests: Tests = testsParser.parse(discoveryOutput, opts.object); + + expect(tests.testFiles.length).to.be.equal(3); + expect(tests.testFunctions.length).to.be.equal(6); + expect(tests.testSuites.length).to.be.equal(3); + expect(tests.testFolders.length).to.be.equal(1); + + // now ensure that each test function belongs within a single test suite... + tests.testFunctions.forEach(fn => { + if (fn.parentTestSuite) { + const testPrefix: boolean = fn.testFunction.nameToRun.startsWith(fn.parentTestSuite.nameToRun); + expect(testPrefix).to.equal(true, + [`function ${fn.testFunction.name} has a parent suite ${fn.parentTestSuite.name}, `, + `but the parent suite 'nameToRun' (${fn.parentTestSuite.nameToRun}) isn't the `, + `prefix to the functions 'nameToRun' (${fn.testFunction.nameToRun})`].join('')); + } + }); + }); + test('Ensure discovery will not fail with blank content' , async () => { + const testHelper: TestsHelper = new TestsHelper(new TestFlatteningVisitor(), serviceContainer.object); + + const testsParser: TestsParser = new TestsParser(testHelper); + + const opts = typeMoq.Mock.ofType(); + const token = typeMoq.Mock.ofType(); + const wspace = typeMoq.Mock.ofType(); + opts.setup(o => o.token).returns(() => token.object); + opts.setup(o => o.workspaceFolder).returns(() => wspace.object); + token.setup(t => t.isCancellationRequested) + .returns(() => true); + opts.setup(o => o.cwd).returns(() => '/home/user/dev'); + opts.setup(o => o.startDirectory).returns(() => './tests'); + + const tests: Tests = testsParser.parse('', opts.object); + + expect(tests.testFiles.length).to.be.equal(0); + expect(tests.testFunctions.length).to.be.equal(0); + expect(tests.testSuites.length).to.be.equal(0); + expect(tests.testFolders.length).to.be.equal(0); + }); + test('Ensure discovery will not fail with corrupt content', async () => { + const testHelper: TestsHelper = new TestsHelper(new TestFlatteningVisitor(), serviceContainer.object); + + const testsParser: TestsParser = new TestsParser(testHelper); + + const opts = typeMoq.Mock.ofType(); + const token = typeMoq.Mock.ofType(); + const wspace = typeMoq.Mock.ofType(); + opts.setup(o => o.token).returns(() => token.object); + opts.setup(o => o.workspaceFolder).returns(() => wspace.object); + token.setup(t => t.isCancellationRequested) + .returns(() => true); + opts.setup(o => o.cwd).returns(() => '/home/user/dev'); + opts.setup(o => o.startDirectory).returns(() => './tests'); + + const discoveryOutput: string = ['a;lskdjfa', + 'allikbrilkpdbfkdfbalk;nfm', + '', + ';;h,spmn,nlikmslkjls.bmnl;klkjna;jdfngad,lmvnjkldfhb', + ''].join('\n'); + + const tests: Tests = testsParser.parse(discoveryOutput, opts.object); + + expect(tests.testFiles.length).to.be.equal(0); + expect(tests.testFunctions.length).to.be.equal(0); + expect(tests.testSuites.length).to.be.equal(0); + expect(tests.testFolders.length).to.be.equal(0); + }); + test('Ensure discovery resolves when no tests are found in the given path', async () => { + const testHelper: TestsHelper = new TestsHelper(new TestFlatteningVisitor(), serviceContainer.object); + + const testsParser: TestsParser = new TestsParser(testHelper); + + const opts = typeMoq.Mock.ofType(); + const token = typeMoq.Mock.ofType(); + const wspace = typeMoq.Mock.ofType(); + opts.setup(o => o.token).returns(() => token.object); + opts.setup(o => o.workspaceFolder).returns(() => wspace.object); + token.setup(t => t.isCancellationRequested) + .returns(() => true); + opts.setup(o => o.cwd).returns(() => '/home/user/dev'); + opts.setup(o => o.startDirectory).returns(() => './tests'); + + const discoveryOutput: string = 'start'; + + const tests: Tests = testsParser.parse(discoveryOutput, opts.object); + + expect(tests.testFiles.length).to.be.equal(0); + expect(tests.testFunctions.length).to.be.equal(0); + expect(tests.testSuites.length).to.be.equal(0); + expect(tests.testFolders.length).to.be.equal(0); + }); }); From 6288a4de06bbc69d68d6539b4c23a52841981116 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 27 Jun 2018 19:30:44 -0700 Subject: [PATCH 12/13] Restore the 'private' scope of methods within the TestParser class - we don't need to test these directly (yet) --- src/client/unittests/unittest/services/parserService.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/client/unittests/unittest/services/parserService.ts b/src/client/unittests/unittest/services/parserService.ts index 30baa278ebab..4b0b3f5359bd 100644 --- a/src/client/unittests/unittest/services/parserService.ts +++ b/src/client/unittests/unittest/services/parserService.ts @@ -18,7 +18,7 @@ export class TestsParser implements ITestsParser { } return this.parseTestIds(testsDirectory, testIds); } - public getTestIds(content: string): string[] { + private getTestIds(content: string): string[] { let startedCollecting = false; return content.split(/\r?\n/g) .map(line => { @@ -32,7 +32,7 @@ export class TestsParser implements ITestsParser { }) .filter(line => line.length > 0); } - public parseTestIds(rootDirectory: string, testIds: string[]): Tests { + private parseTestIds(rootDirectory: string, testIds: string[]): Tests { const testFiles: TestFile[] = []; testIds.forEach(testId => this.addTestId(rootDirectory, testId, testFiles)); @@ -44,13 +44,13 @@ export class TestsParser implements ITestsParser { * TestIds are fully qualified including the method names. * E.g. tone_test.Failing2Tests.test_failure * Where tone_test = folder, Failing2Tests = class/suite, test_failure = method. - * @public + * @private * @param {string} rootDirectory * @param {string[]} testIds * @returns {Tests} * @memberof TestsParser */ - public addTestId(rootDirectory: string, testId: string, testFiles: TestFile[]) { + private addTestId(rootDirectory: string, testId: string, testFiles: TestFile[]) { const testIdParts = testId.split('.'); // We must have a file, class and function name if (testIdParts.length <= 2) { From 837ccc6876749473401a9ff88478a1dde6a6fa36 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Thu, 28 Jun 2018 00:05:49 -0700 Subject: [PATCH 13/13] Include full relative namespace to each test selected - for file-only test select - for suite-only test select --- src/client/unittests/unittest/services/parserService.ts | 2 +- src/test/unittests/unittest/unittest.discovery.test.ts | 8 ++++---- src/test/unittests/unittest/unittest.test.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/client/unittests/unittest/services/parserService.ts b/src/client/unittests/unittest/services/parserService.ts index 4b0b3f5359bd..6e99f1044d3a 100644 --- a/src/client/unittests/unittest/services/parserService.ts +++ b/src/client/unittests/unittest/services/parserService.ts @@ -71,7 +71,7 @@ export class TestsParser implements ITestsParser { fullPath: filePath, functions: [], suites: [], - nameToRun: suiteToRun, + nameToRun: `${suiteToRun}.${functionName}`, xmlName: '', status: TestStatus.Idle, time: 0 diff --git a/src/test/unittests/unittest/unittest.discovery.test.ts b/src/test/unittests/unittest/unittest.discovery.test.ts index d3e8c5b477b5..12427c955f5a 100644 --- a/src/test/unittests/unittest/unittest.discovery.test.ts +++ b/src/test/unittests/unittest/unittest.discovery.test.ts @@ -87,7 +87,7 @@ suite('Unit Tests - unittest - discovery with mocked process output', () => { assert.equal(tests.testFiles.length, 1, 'Incorrect number of test files'); assert.equal(tests.testFunctions.length, 3, 'Incorrect number of test functions'); assert.equal(tests.testSuites.length, 1, 'Incorrect number of test suites'); - assert.equal(tests.testFiles.some(t => t.name === 'test_one.py' && t.nameToRun === 'Test_test1.test_A'), true, 'Test File not found'); + assert.equal(tests.testFiles.some(t => t.name === 'test_one.py' && t.nameToRun === 'test_one.Test_test1.test_A'), true, 'Test File not found'); }); test('Discover Tests', async () => { @@ -110,8 +110,8 @@ suite('Unit Tests - unittest - discovery with mocked process output', () => { assert.equal(tests.testFiles.length, 2, 'Incorrect number of test files'); assert.equal(tests.testFunctions.length, 9, 'Incorrect number of test functions'); assert.equal(tests.testSuites.length, 3, 'Incorrect number of test suites'); - assert.equal(tests.testFiles.some(t => t.name === 'test_unittest_one.py' && t.nameToRun === 'Test_test1.test_A'), true, 'Test File not found'); - assert.equal(tests.testFiles.some(t => t.name === 'test_unittest_two.py' && t.nameToRun === 'Test_test2.test_A2'), true, 'Test File not found'); + assert.equal(tests.testFiles.some(t => t.name === 'test_unittest_one.py' && t.nameToRun === 'test_unittest_one.Test_test1.test_A'), true, 'Test File not found'); + assert.equal(tests.testFiles.some(t => t.name === 'test_unittest_two.py' && t.nameToRun === 'test_unittest_two.Test_test2.test_A2'), true, 'Test File not found'); }); test('Discover Tests (pattern = *_test_*.py)', async () => { @@ -127,7 +127,7 @@ suite('Unit Tests - unittest - discovery with mocked process output', () => { assert.equal(tests.testFiles.length, 1, 'Incorrect number of test files'); assert.equal(tests.testFunctions.length, 2, 'Incorrect number of test functions'); assert.equal(tests.testSuites.length, 1, 'Incorrect number of test suites'); - assert.equal(tests.testFiles.some(t => t.name === 'unittest_three_test.py' && t.nameToRun === 'Test_test3.test_A'), true, 'Test File not found'); + assert.equal(tests.testFiles.some(t => t.name === 'unittest_three_test.py' && t.nameToRun === 'unittest_three_test.Test_test3.test_A'), true, 'Test File not found'); }); test('Setting cwd should return tests', async () => { diff --git a/src/test/unittests/unittest/unittest.test.ts b/src/test/unittests/unittest/unittest.test.ts index ebad6a6b5d18..4152ef540be9 100644 --- a/src/test/unittests/unittest/unittest.test.ts +++ b/src/test/unittests/unittest/unittest.test.ts @@ -58,6 +58,6 @@ suite('Unit Tests - unittest - discovery against actual python process', () => { assert.equal(tests.testFiles.length, 1, 'Incorrect number of test files'); assert.equal(tests.testFunctions.length, 3, 'Incorrect number of test functions'); assert.equal(tests.testSuites.length, 1, 'Incorrect number of test suites'); - assert.equal(tests.testFiles.some(t => t.name === 'test_one.py' && t.nameToRun === 'Test_test1.test_A'), true, 'Test File not found'); + assert.equal(tests.testFiles.some(t => t.name === 'test_one.py' && t.nameToRun === 'test_one.Test_test1.test_A'), true, 'Test File not found'); }); });