Correct unittest discovery for n-depth test source trees (#2066)

* Include full relative namespace to each test selected
- for file-only test select
- for suite-only test select

Author: d3r3kk

news/2 Fixes/2044.md

@@ -0,0 +1 @@
+Store testId for files & suites during unittest discovery

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:

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[];

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();

src/client/unittests/unittest/services/parserService.ts

@@ -3,9 +3,9 @@
 
 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, TestFile,
+    TestFunction, Tests, TestStatus,
+    UnitTestParserOptions } from '../../common/types';
 
 @injectable()
 export class TestsParser implements ITestsParser {
@@ -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,25 +71,25 @@ export class TestsParser implements ITestsParser {
                 fullPath: filePath,
                 functions: [],
                 suites: [],
-                nameToRun: `${className}.${functionName}`,
+                nameToRun: `${suiteToRun}.${functionName}`,
                 xmlName: '',
                 status: TestStatus.Idle,
                 time: 0
             };
             testFiles.push(testFile);
         }
 
-        // Check if we already have this test file
-        const classNameToRun = className;
-        let testSuite = testFile.suites.find(cls => cls.nameToRun === classNameToRun);
+        // Check if we already have this suite
+        // nameToRun = testId - method name
+        let testSuite = testFile.suites.find(cls => cls.nameToRun === suiteToRun);
         if (!testSuite) {
             testSuite = {
                 name: className,
                 functions: [],
                 suites: [],
                 isUnitTest: true,
                 isInstance: false,
-                nameToRun: `${path.parse(filePath).name}.${classNameToRun}`,
+                nameToRun: suiteToRun,
                 xmlName: '',
                 status: TestStatus.Idle,
                 time: 0

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<number> {
+    public start(options: { port?: number; host?: string } = { port: 0, host: 'localhost' }): Promise<number> {
         this.startedDef = createDeferred<number>();
         this.server = net.createServer(this.connectionListener.bind(this));
         this.server!.maxConnections = MaxConnections;

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 () => {