Eric Snow (ericsnowcurrently) · Apr 29, 2019 · Apr 29, 2019 · Apr 29, 2019 · Apr 29, 2019 · Apr 29, 2019
diff --git a/build/unlocalizedFiles.json b/build/unlocalizedFiles.json
@@ -19,6 +19,7 @@
     "src/client/terminals/codeExecution/helper.ts",
     "src/client/testing/common/debugLauncher.ts",
     "src/client/testing/common/managers/baseTestManager.ts",
+    "src/client/testing/common/services/discovery.ts",
     "src/client/testing/configuration.ts",
     "src/client/testing/display/main.ts",
     "src/client/testing/main.ts",

diff --git a/news/2 Fixes/5313.md b/news/2 Fixes/5313.md
@@ -0,0 +1 @@
+Always show pytest's output when it fails.
diff --git a/pythonFiles/testing_tools/adapter/pytest.py b/pythonFiles/testing_tools/adapter/pytest.py
@@ -1,7 +1,7 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the MIT License.
 
-from __future__ import absolute_import
+from __future__ import absolute_import, print_function
 
 import os.path
 import sys
@@ -33,15 +33,21 @@ def discover(pytestargs=None, hidestdio=False,
     pytestargs = _adjust_pytest_args(pytestargs)
     # We use this helper rather than "-pno:terminal" due to possible
     # platform-dependent issues.
-    with util.hide_stdio() if hidestdio else util.noop_cm():
+    with (util.hide_stdio() if hidestdio else util.noop_cm()) as stdio:
         ec = _pytest_main(pytestargs, [_plugin])
     # See: https://docs.pytest.org/en/latest/usage.html#possible-exit-codes
     if ec == 5:
         # No tests were discovered.
         pass
     elif ec != 0:
+        if hidestdio:
+            print(stdio.getvalue(), file=sys.stderr)
+            sys.stdout.flush()
         raise Exception('pytest discovery failed (exit code {})'.format(ec))
     if not _plugin._started:
+        if hidestdio:
+            print(stdio.getvalue(), file=sys.stderr)
+            sys.stdout.flush()
         raise Exception('pytest discovery did not start')
     return (
             _plugin._tests.parents,

diff --git a/pythonFiles/testing_tools/adapter/util.py b/pythonFiles/testing_tools/adapter/util.py
@@ -17,17 +17,19 @@ def noop_cm():
 @contextlib.contextmanager
 def hide_stdio():
     """Swallow stdout and stderr."""
-    ignored = IgnoredIO()
+    ignored = StdioStream()
     sys.stdout = ignored
     sys.stderr = ignored
     try:
-        yield
+        yield ignored
     finally:
         sys.stdout = sys.__stdout__
         sys.stderr = sys.__stderr__
 
 
-class IgnoredIO(StringIO):
-    """A noop "file"."""
-    def write(self, msg):
-        pass
+if sys.version_info < (3,):
+    class StdioStream(StringIO):
+        def write(self, msg):
+            StringIO.write(self, msg.decode())
+else:
+    StdioStream = StringIO
diff --git a/pythonFiles/tests/testing_tools/adapter/.data/syntax-error/tests/__init__.py b/pythonFiles/tests/testing_tools/adapter/.data/syntax-error/tests/__init__.py
diff --git a/pythonFiles/tests/testing_tools/adapter/.data/syntax-error/tests/test_spam.py b/pythonFiles/tests/testing_tools/adapter/.data/syntax-error/tests/test_spam.py
@@ -0,0 +1,7 @@
+
+def test_simple():
+    assert True
+
+
+# A syntax error:
+:
diff --git a/pythonFiles/tests/testing_tools/adapter/test_functional.py b/pythonFiles/tests/testing_tools/adapter/test_functional.py
@@ -28,18 +28,14 @@ def resolve_testroot(name):
 def run_adapter(cmd, tool, *cliargs):
     try:
         return _run_adapter(cmd, tool, *cliargs)
-    except subprocess.CalledProcessError:
-        # Re-run pytest but print out stdout & stderr this time
-        try:
-            return _run_adapter(cmd, tool, *cliargs, hidestdio=False)
-        except subprocess.CalledProcessError as exc:
-            print(exc.output)
+    except subprocess.CalledProcessError as exc:
+        print(exc.output)
 
 
 def _run_adapter(cmd, tool, *cliargs, **kwargs):
     hidestdio = kwargs.pop('hidestdio', True)
-    assert not kwargs
-    kwds = {}
+    assert not kwargs or tuple(kwargs) == ('stderr',)
+    kwds = kwargs
     argv = [sys.executable, SCRIPT, cmd, tool, '--'] + list(cliargs)
     if not hidestdio:
         argv.insert(4, '--no-hide-stdio')
@@ -235,6 +231,30 @@ def test_discover_not_found(self):
         #    'tests': [],
         #    }])
 
+    @unittest.skip('broken in CI')
+    def test_discover_bad_args(self):
+        projroot, testroot = resolve_testroot('simple')
+
+        with self.assertRaises(subprocess.CalledProcessError) as cm:
+            _run_adapter('discover', 'pytest',
+                         '--spam',
+                         '--rootdir', projroot,
+                         testroot,
+                         stderr=subprocess.STDOUT,
+                         )
+        self.assertIn('(exit code 4)', cm.exception.output)
+
+    def test_discover_syntax_error(self):
+        projroot, testroot = resolve_testroot('syntax-error')
+
+        with self.assertRaises(subprocess.CalledProcessError) as cm:
+            _run_adapter('discover', 'pytest',
+                         '--rootdir', projroot,
+                         testroot,
+                         stderr=subprocess.STDOUT,
+                         )
+        self.assertIn('(exit code 2)', cm.exception.output)
+
 
 COMPLEX = {
         'root': None,

diff --git a/src/client/testing/common/services/discovery.ts b/src/client/testing/common/services/discovery.ts
@@ -3,22 +3,28 @@
 
 'use strict';
 
-import { inject, injectable } from 'inversify';
+import { inject, injectable, named } from 'inversify';
 import * as path from 'path';
+import { OutputChannel } from 'vscode';
 import { traceError } from '../../../common/logger';
 import { ExecutionFactoryCreateWithEnvironmentOptions, ExecutionResult, IPythonExecutionFactory, SpawnOptions } from '../../../common/process/types';
+import { IOutputChannel } from '../../../common/types';
 import { EXTENSION_ROOT_DIR } from '../../../constants';
 import { captureTelemetry } from '../../../telemetry';
 import { EventName } from '../../../telemetry/constants';
+import { TEST_OUTPUT_CHANNEL } from '../constants';
 import { ITestDiscoveryService, TestDiscoveryOptions, Tests } from '../types';
 import { DiscoveredTests, ITestDiscoveredTestParser } from './types';
 
 const DISCOVERY_FILE = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'testing_tools', 'run_adapter.py');
 
 @injectable()
 export class TestsDiscoveryService implements ITestDiscoveryService {
-    constructor(@inject(IPythonExecutionFactory) private readonly execFactory: IPythonExecutionFactory,
-        @inject(ITestDiscoveredTestParser) private readonly parser: ITestDiscoveredTestParser) { }
+    constructor(
+        @inject(IPythonExecutionFactory) private readonly execFactory: IPythonExecutionFactory,
+        @inject(ITestDiscoveredTestParser) private readonly parser: ITestDiscoveredTestParser,
+        @inject(IOutputChannel) @named(TEST_OUTPUT_CHANNEL) private readonly outChannel: OutputChannel
+    ) { }
     @captureTelemetry(EventName.UNITTEST_DISCOVER_WITH_PYCODE, undefined, true)
     public async discoverTests(options: TestDiscoveryOptions): Promise<Tests> {
         let output: ExecutionResult<string> | undefined;
@@ -45,6 +51,8 @@ export class TestsDiscoveryService implements ITestDiscoveryService {
             cwd: options.cwd,
             throwOnStdErr: true
         };
-        return execService.exec([DISCOVERY_FILE, ...options.args], spawnOptions);
+        const argv = [DISCOVERY_FILE, ...options.args];
+        this.outChannel.appendLine(`python ${argv.join(' ')}`);
+        return execService.exec(argv, spawnOptions);
     }
 }
diff --git a/src/test/testing/common/services/discovery.unit.test.ts b/src/test/testing/common/services/discovery.unit.test.ts
@@ -7,7 +7,7 @@ import * as assert from 'assert';
 import * as path from 'path';
 import { deepEqual, instance, mock, when } from 'ts-mockito';
 import * as typemoq from 'typemoq';
-import { CancellationTokenSource, Uri } from 'vscode';
+import { CancellationTokenSource, OutputChannel, Uri, ViewColumn } from 'vscode';
 import { PythonExecutionFactory } from '../../../../client/common/process/pythonExecutionFactory';
 import { ExecutionFactoryCreateWithEnvironmentOptions, IPythonExecutionFactory, IPythonExecutionService, SpawnOptions } from '../../../../client/common/process/types';
 import { EXTENSION_ROOT_DIR } from '../../../../client/constants';
@@ -19,13 +19,16 @@ import { MockOutputChannel } from '../../../mockClasses';
 
 // tslint:disable:no-unnecessary-override no-any
 suite('Unit Tests - Common Discovery', () => {
+    let output: OutputChannel;
     let discovery: TestsDiscoveryService;
     let executionFactory: IPythonExecutionFactory;
     let parser: ITestDiscoveredTestParser;
     setup(() => {
+        // tslint:disable-next-line:no-use-before-declare
+        output = mock(StubOutput);
         executionFactory = mock(PythonExecutionFactory);
         parser = mock(TestDiscoveredTestParser);
-        discovery = new TestsDiscoveryService(instance(executionFactory), instance(parser));
+        discovery = new TestsDiscoveryService(instance(executionFactory), instance(parser), instance(output));
     });
     test('Use parser to parse results', async () => {
         const options: TestDiscoveryOptions = {
@@ -73,3 +76,17 @@ suite('Unit Tests - Common Discovery', () => {
         assert.deepEqual(result, executionResult);
     });
 });
+
+// tslint:disable:no-empty
+
+//class StubOutput implements OutputChannel {
+class StubOutput {
+    constructor(public name: string) {}
+    public append(_value: string) {}
+    public appendLine(_value: string) {}
+    public clear() {}
+    //public show(_preserveFocus?: boolean) {}
+    public show(_column?: ViewColumn | boolean, _preserveFocus?: boolean) {}
+    public hide() {}
+    public dispose() {}
+}