Correct unittest discovery for n-depth test source trees (microsoft#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 () => {