Merge pull request #143 from kondratyev-nv/better_unittest_error_repo…

…rting

Better unittest error reporting

src/unittest/unittestScripts.ts

@@ -4,8 +4,9 @@ export const TEST_RESULT_PREFIX = 'TEST_EXECUTION_RESULT';
 
 export const UNITTEST_TEST_RUNNER_SCRIPT = `
 from __future__ import print_function
-from unittest import TextTestRunner, TextTestResult, TestLoader, TestSuite, defaultTestLoader as loader, util
+from unittest import TextTestRunner, TextTestResult, TestLoader, TestSuite, defaultTestLoader, util
 import sys
+import os
 import base64
 import json
 import traceback
@@ -16,11 +17,25 @@ STDOUT_LINE = '\\nStdout:\\n%s'
 STDERR_LINE = '\\nStderr:\\n%s'
 
 
+def writeln(stream, value=None):
+    if value:
+        stream.write(value)
+    stream.write(os.linesep)
+
+
 def write_test_state(stream, state, result):
     message = base64.b64encode(result[1].encode('utf8')).decode('ascii')
-    stream.writeln()
-    stream.writeln("{}:{}:{}:{}".format(TEST_RESULT_PREFIX,
-                                        state, result[0].id(), message))
+    writeln(stream)
+    writeln(stream, "{}:{}:{}:{}".format(TEST_RESULT_PREFIX,
+                                         state, result[0].id(), message))
+
+
+def full_class_name(o):
+  module = o.__class__.__module__
+  if module is None or module == str.__class__.__module__:
+    return o.__class__.__name__
+  else:
+    return module + '.' + o.__class__.__name__
 
 
 class TextTestResultWithSuccesses(TextTestResult):
@@ -87,17 +102,47 @@ class InvalidTest:
         self.test = test
         self.exception = exception
 
+    def id(self):
+        return self.test
+
+
+def get_invalid_test_name(test):
+    if hasattr(test, '_testMethodName'):
+        return test._testMethodName
+    return util.strclass(test.__class__)
+
+
+def get_python3_invalid_test(test):
+    if hasattr(test, '_exception'):
+        return InvalidTest(get_invalid_test_name(test), test._exception)
+    return None
+
+
+def get_python2_invalid_test(test):
+    test_class_name = full_class_name(test)
+    if test_class_name == 'unittest.loader.ModuleImportFailure' or test_class_name == 'unittest.loader.LoadTestsFailure':
+        result = TextTestResult(sys.stderr, True, 1)
+        test.run(result)
+        if not result.errors:
+            return InvalidTest(get_invalid_test_name(test), "Failed to load test: " + test_class_name)
+        return InvalidTest(get_invalid_test_name(test), "\\n".join(list(map(lambda e: e[1], result.errors))))
+    return None
+
 
 def check_test_ids(tests):
     valid_tests = []
     invalid_tests = []
     for test in tests:
-        if hasattr(test, '_exception'):
-            if hasattr(test, '_testMethodName'):
-                invalid_tests.append(InvalidTest(test._testMethodName, test._exception))
-            else:
-                invalid_tests.append(InvalidTest(util.strclass(test.__class__), test._exception))
+        p3error = get_python3_invalid_test(test)
+        if p3error is not None:
+            invalid_tests.append(p3error)
             continue
+
+        p2error = get_python2_invalid_test(test)
+        if p2error is not None:
+            invalid_tests.append(p2error)
+            continue
+
         try:
             test.id()  # check if test id is valid
             valid_tests.append(test)
@@ -110,7 +155,7 @@ def check_test_ids(tests):
 
 
 def discover_tests(start_directory, pattern):
-    tests = get_tests(loader.discover(start_directory, pattern=pattern))
+    tests = get_tests(defaultTestLoader.discover(start_directory, pattern=pattern))
     return check_test_ids(tests)
 
 
@@ -124,8 +169,9 @@ def run_tests(start_directory, pattern, test_ids):
     runner = TextTestRunner(
         buffer=True, resultclass=TextTestResultWithSuccesses, stream=sys.stdout)
     available_tests, invalid_tests = discover_tests(start_directory, pattern)
-    tests = filter_by_test_ids(available_tests, test_ids)
-    result = runner.run(TestSuite(tests))
+    result = runner.run(TestSuite(filter_by_test_ids(available_tests, test_ids)))
+    for invalid_test in filter_by_test_ids(invalid_tests, test_ids):
+        write_test_state(sys.stdout, "failed", (invalid_test, str(invalid_test.exception)))
 
 
 def extract_errors(tests):

src/unittest/unittestSuitParser.ts

@@ -1,3 +1,4 @@
+import * as fs from 'fs';
 import { Base64 } from 'js-base64';
 import * as os from 'os';
 import * as path from 'path';
@@ -37,10 +38,15 @@ export function parseTestSuites(content: string, cwd: string): {
             message: messages.map(e => e.message).join(os.EOL),
         }))
         .filter(e => e.id)
-        .map(e => ({ id: e.id!, message: e.message }));
-    const discoveryErrorSuites = aggregatedErrors.map(({ id }) => <TestSuiteInfo | TestInfo>({
+        .map(e => ({
+            id: e.id!,
+            file: errorSuiteFilePathBySuiteId(cwd, e.id!.testId),
+            message: e.message,
+        }));
+    const discoveryErrorSuites = aggregatedErrors.map(({ id, file }) => <TestSuiteInfo | TestInfo>({
         type: 'test' as 'test',
         id: id.testId,
+        file,
         label: id.testLabel,
     }));
     const suites = Array.from(groupBy(allTests, t => t.suiteId).entries())
@@ -119,7 +125,11 @@ function toState(value: string): 'running' | 'passed' | 'failed' | 'skipped' | u
 function splitTestId(testId: string) {
     const separatorIndex = testId.lastIndexOf('.');
     if (separatorIndex < 0) {
-        return null;
+        return {
+            suiteId: testId,
+            testId,
+            testLabel: testId,
+        };
     }
     return {
         suiteId: testId.substring(0, separatorIndex),
@@ -128,6 +138,16 @@ function splitTestId(testId: string) {
     };
 }
 
+function errorSuiteFilePathBySuiteId(cwd: string, suiteId: string) {
+    // <path>.<path>.<file>
+    const relativePath = suiteId.split('.').join('/');
+    const filePathCandidate = path.resolve(cwd, relativePath + '.py');
+    if (fs.existsSync(filePathCandidate) && fs.lstatSync(filePathCandidate).isFile()) {
+        return filePathCandidate;
+    }
+    return undefined;
+}
+
 function filePathBySuiteId(cwd: string, suiteId: string) {
     const separatorIndex = suiteId.lastIndexOf('.');
     if (separatorIndex < 0) {

test/index.ts

@@ -11,7 +11,7 @@
 // a possible error to the callback or null if none.
 
 import * as testRunner from 'vscode/lib/testrunner';
-import { getReporter, getPythonExecutable } from './testConfiguration';
+import { getReporter, getPythonExecutable } from './utils/testConfiguration';
 import { runScript } from '../src/pythonRunner';
 
 runScript({
@@ -33,4 +33,3 @@ testRunner.configure({
 });
 
 module.exports = testRunner;
-

test/test_samples/unittest/invalid_tests/test_invalid_syntax_failed.py

@@ -0,0 +1,5 @@
+import unittest
+
+class InvalidSyntaxTests(unittest.TestCase):
+def test_with_invalid_syntax(self):
+    self.assertEqual(3, 2 + 1)

test/test_samples/unittest/test_invalid_test_id.py → test/test_samples/unittest/invalid_tests/test_invalid_test_id.py

@@ -1,7 +1,7 @@
 import unittest
 
 
-class InvalidTestIdTests(unittest.TestCase):
+class InvalidTestIdTests_failed(unittest.TestCase):
     def __init__(self, methodName):
         unittest.TestCase.__init__(self, methodName)
         self.id = 123

test/test_samples/unittest/test_invalid_import_failed.py

@@ -0,0 +1,6 @@
+import unittest
+import some_non_existing_module
+
+class InvalidImportTests(unittest.TestCase):
+    def test_with_invalid_import(self):
+        self.assertEqual(3, 2 + 1)

test/environmentParsing.test.ts → test/tests/environmentParsing.test.ts

@@ -2,10 +2,10 @@ import { expect } from 'chai';
 import 'mocha';
 import * as path from 'path';
 
-import { IPytestConfiguration, IUnittestConfiguration } from '../src/configuration/workspaceConfiguration';
-import { PytestTestRunner } from '../src/pytest/pytestTestRunner';
-import { UnittestTestRunner } from '../src/unittest/unittestTestRunner';
-import { findWorkspaceFolder, logger } from './helpers';
+import { IPytestConfiguration, IUnittestConfiguration } from '../../src/configuration/workspaceConfiguration';
+import { PytestTestRunner } from '../../src/pytest/pytestTestRunner';
+import { UnittestTestRunner } from '../../src/unittest/unittestTestRunner';
+import { findWorkspaceFolder, logger } from '../utils/helpers';
 
 [
     {

test/idGenerator.test.ts → test/tests/idGenerator.test.ts

@@ -1,7 +1,7 @@
 import { expect } from 'chai';
 import 'mocha';
 
-import { nextId } from '../src/idGenerator';
+import { nextId } from '../../src/idGenerator';
 
 function hasDuplicates<T>(values: T[]) {
     return (new Set<T>(values)).size !== values.length;

test/placeholderAwareWorkspaceConfiguration.test.ts → test/tests/placeholderAwareWorkspaceConfiguration.test.ts

@@ -3,13 +3,13 @@ import 'mocha';
 import * as os from 'os';
 import * as path from 'path';
 
-import { PlaceholderAwareWorkspaceConfiguration } from '../src/configuration/placeholderAwareWorkspaceConfiguration';
+import { PlaceholderAwareWorkspaceConfiguration } from '../../src/configuration/placeholderAwareWorkspaceConfiguration';
 import {
     IPytestConfiguration,
     IUnittestConfiguration,
     IWorkspaceConfiguration
-} from '../src/configuration/workspaceConfiguration';
-import { findWorkspaceFolder, logger } from './helpers';
+} from '../../src/configuration/workspaceConfiguration';
+import { findWorkspaceFolder, logger } from '../utils/helpers';
 
 function getWorkspaceFolder() {
     return findWorkspaceFolder('empty_configuration')!;

test/pytestArguments.test.ts → test/tests/pytestArguments.test.ts

@@ -2,9 +2,9 @@ import { expect } from 'chai';
 import 'mocha';
 import * as path from 'path';
 
-import { IWorkspaceConfiguration } from '../src/configuration/workspaceConfiguration';
-import { PytestTestRunner } from '../src/pytest/pytestTestRunner';
-import { createPytestConfiguration, extractExpectedState, findTestSuiteByLabel, logger } from './helpers';
+import { IWorkspaceConfiguration } from '../../src/configuration/workspaceConfiguration';
+import { PytestTestRunner } from '../../src/pytest/pytestTestRunner';
+import { createPytestConfiguration, extractExpectedState, findTestSuiteByLabel, logger } from '../utils/helpers';
 
 suite('Pytest test discovery with additional arguments', async () => {
     const config: IWorkspaceConfiguration = createPytestConfiguration(

test/pytestGeneral.test.ts → test/tests/pytestGeneral.test.ts

@@ -2,9 +2,9 @@ import { expect } from 'chai';
 import 'mocha';
 import * as path from 'path';
 
-import { IWorkspaceConfiguration } from '../src/configuration/workspaceConfiguration';
-import { PytestTestRunner } from '../src/pytest/pytestTestRunner';
-import { createPytestConfiguration, extractExpectedState, findTestSuiteByLabel, logger } from './helpers';
+import { IWorkspaceConfiguration } from '../../src/configuration/workspaceConfiguration';
+import { PytestTestRunner } from '../../src/pytest/pytestTestRunner';
+import { createPytestConfiguration, extractExpectedState, findTestSuiteByLabel, logger } from '../utils/helpers';
 
 suite('Pytest test discovery with errors', async () => {
     const config: IWorkspaceConfiguration = createPytestConfiguration(