Skip to content

Commit 77a510c

Browse files
author
Eric Snow
authored
Print stdout/stderr when pytest fails (in adapter script). (microsoft#5512)
(for microsoft#5313)
1 parent f261e09 commit 77a510c

9 files changed

Lines changed: 84 additions & 22 deletions

File tree

‎build/unlocalizedFiles.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"src/client/terminals/codeExecution/helper.ts",
2020
"src/client/testing/common/debugLauncher.ts",
2121
"src/client/testing/common/managers/baseTestManager.ts",
22+
"src/client/testing/common/services/discovery.ts",
2223
"src/client/testing/configuration.ts",
2324
"src/client/testing/display/main.ts",
2425
"src/client/testing/main.ts",

‎news/2 Fixes/5313.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Always show pytest's output when it fails.

‎pythonFiles/testing_tools/adapter/pytest.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Copyright (c) Microsoft Corporation. All rights reserved.
22
# Licensed under the MIT License.
33

4-
from __future__ import absolute_import
4+
from __future__ import absolute_import, print_function
55

66
import os.path
77
import sys
@@ -33,15 +33,21 @@ def discover(pytestargs=None, hidestdio=False,
3333
pytestargs = _adjust_pytest_args(pytestargs)
3434
# We use this helper rather than "-pno:terminal" due to possible
3535
# platform-dependent issues.
36-
with util.hide_stdio() if hidestdio else util.noop_cm():
36+
with (util.hide_stdio() if hidestdio else util.noop_cm()) as stdio:
3737
ec = _pytest_main(pytestargs, [_plugin])
3838
# See: https://docs.pytest.org/en/latest/usage.html#possible-exit-codes
3939
if ec == 5:
4040
# No tests were discovered.
4141
pass
4242
elif ec != 0:
43+
if hidestdio:
44+
print(stdio.getvalue(), file=sys.stderr)
45+
sys.stdout.flush()
4346
raise Exception('pytest discovery failed (exit code {})'.format(ec))
4447
if not _plugin._started:
48+
if hidestdio:
49+
print(stdio.getvalue(), file=sys.stderr)
50+
sys.stdout.flush()
4551
raise Exception('pytest discovery did not start')
4652
return (
4753
_plugin._tests.parents,

‎pythonFiles/testing_tools/adapter/util.py‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,19 @@ def noop_cm():
1717
@contextlib.contextmanager
1818
def hide_stdio():
1919
"""Swallow stdout and stderr."""
20-
ignored = IgnoredIO()
20+
ignored = StdioStream()
2121
sys.stdout = ignored
2222
sys.stderr = ignored
2323
try:
24-
yield
24+
yield ignored
2525
finally:
2626
sys.stdout = sys.__stdout__
2727
sys.stderr = sys.__stderr__
2828

2929

30-
class IgnoredIO(StringIO):
31-
"""A noop "file"."""
32-
def write(self, msg):
33-
pass
30+
if sys.version_info < (3,):
31+
class StdioStream(StringIO):
32+
def write(self, msg):
33+
StringIO.write(self, msg.decode())
34+
else:
35+
StdioStream = StringIO

‎pythonFiles/tests/testing_tools/adapter/.data/syntax-error/tests/__init__.py‎

Whitespace-only changes.

‎pythonFiles/tests/testing_tools/adapter/.data/syntax-error/tests/test_spam.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
def test_simple():
3+
assert True
4+
5+
6+
# A syntax error:
7+
:

‎pythonFiles/tests/testing_tools/adapter/test_functional.py‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,14 @@ def resolve_testroot(name):
2828
def run_adapter(cmd, tool, *cliargs):
2929
try:
3030
return _run_adapter(cmd, tool, *cliargs)
31-
except subprocess.CalledProcessError:
32-
# Re-run pytest but print out stdout & stderr this time
33-
try:
34-
return _run_adapter(cmd, tool, *cliargs, hidestdio=False)
35-
except subprocess.CalledProcessError as exc:
36-
print(exc.output)
31+
except subprocess.CalledProcessError as exc:
32+
print(exc.output)
3733

3834

3935
def _run_adapter(cmd, tool, *cliargs, **kwargs):
4036
hidestdio = kwargs.pop('hidestdio', True)
41-
assert not kwargs
42-
kwds = {}
37+
assert not kwargs or tuple(kwargs) == ('stderr',)
38+
kwds = kwargs
4339
argv = [sys.executable, SCRIPT, cmd, tool, '--'] + list(cliargs)
4440
if not hidestdio:
4541
argv.insert(4, '--no-hide-stdio')
@@ -235,6 +231,30 @@ def test_discover_not_found(self):
235231
# 'tests': [],
236232
# }])
237233

234+
@unittest.skip('broken in CI')
235+
def test_discover_bad_args(self):
236+
projroot, testroot = resolve_testroot('simple')
237+
238+
with self.assertRaises(subprocess.CalledProcessError) as cm:
239+
_run_adapter('discover', 'pytest',
240+
'--spam',
241+
'--rootdir', projroot,
242+
testroot,
243+
stderr=subprocess.STDOUT,
244+
)
245+
self.assertIn('(exit code 4)', cm.exception.output)
246+
247+
def test_discover_syntax_error(self):
248+
projroot, testroot = resolve_testroot('syntax-error')
249+
250+
with self.assertRaises(subprocess.CalledProcessError) as cm:
251+
_run_adapter('discover', 'pytest',
252+
'--rootdir', projroot,
253+
testroot,
254+
stderr=subprocess.STDOUT,
255+
)
256+
self.assertIn('(exit code 2)', cm.exception.output)
257+
238258

239259
COMPLEX = {
240260
'root': None,

‎src/client/testing/common/services/discovery.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,28 @@
33

44
'use strict';
55

6-
import { inject, injectable } from 'inversify';
6+
import { inject, injectable, named } from 'inversify';
77
import * as path from 'path';
8+
import { OutputChannel } from 'vscode';
89
import { traceError } from '../../../common/logger';
910
import { ExecutionFactoryCreateWithEnvironmentOptions, ExecutionResult, IPythonExecutionFactory, SpawnOptions } from '../../../common/process/types';
11+
import { IOutputChannel } from '../../../common/types';
1012
import { EXTENSION_ROOT_DIR } from '../../../constants';
1113
import { captureTelemetry } from '../../../telemetry';
1214
import { EventName } from '../../../telemetry/constants';
15+
import { TEST_OUTPUT_CHANNEL } from '../constants';
1316
import { ITestDiscoveryService, TestDiscoveryOptions, Tests } from '../types';
1417
import { DiscoveredTests, ITestDiscoveredTestParser } from './types';
1518

1619
const DISCOVERY_FILE = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'testing_tools', 'run_adapter.py');
1720

1821
@injectable()
1922
export class TestsDiscoveryService implements ITestDiscoveryService {
20-
constructor(@inject(IPythonExecutionFactory) private readonly execFactory: IPythonExecutionFactory,
21-
@inject(ITestDiscoveredTestParser) private readonly parser: ITestDiscoveredTestParser) { }
23+
constructor(
24+
@inject(IPythonExecutionFactory) private readonly execFactory: IPythonExecutionFactory,
25+
@inject(ITestDiscoveredTestParser) private readonly parser: ITestDiscoveredTestParser,
26+
@inject(IOutputChannel) @named(TEST_OUTPUT_CHANNEL) private readonly outChannel: OutputChannel
27+
) { }
2228
@captureTelemetry(EventName.UNITTEST_DISCOVER_WITH_PYCODE, undefined, true)
2329
public async discoverTests(options: TestDiscoveryOptions): Promise<Tests> {
2430
let output: ExecutionResult<string> | undefined;
@@ -45,6 +51,8 @@ export class TestsDiscoveryService implements ITestDiscoveryService {
4551
cwd: options.cwd,
4652
throwOnStdErr: true
4753
};
48-
return execService.exec([DISCOVERY_FILE, ...options.args], spawnOptions);
54+
const argv = [DISCOVERY_FILE, ...options.args];
55+
this.outChannel.appendLine(`python ${argv.join(' ')}`);
56+
return execService.exec(argv, spawnOptions);
4957
}
5058
}

‎src/test/testing/common/services/discovery.unit.test.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as assert from 'assert';
77
import * as path from 'path';
88
import { deepEqual, instance, mock, when } from 'ts-mockito';
99
import * as typemoq from 'typemoq';
10-
import { CancellationTokenSource, Uri } from 'vscode';
10+
import { CancellationTokenSource, OutputChannel, Uri, ViewColumn } from 'vscode';
1111
import { PythonExecutionFactory } from '../../../../client/common/process/pythonExecutionFactory';
1212
import { ExecutionFactoryCreateWithEnvironmentOptions, IPythonExecutionFactory, IPythonExecutionService, SpawnOptions } from '../../../../client/common/process/types';
1313
import { EXTENSION_ROOT_DIR } from '../../../../client/constants';
@@ -19,13 +19,16 @@ import { MockOutputChannel } from '../../../mockClasses';
1919

2020
// tslint:disable:no-unnecessary-override no-any
2121
suite('Unit Tests - Common Discovery', () => {
22+
let output: OutputChannel;
2223
let discovery: TestsDiscoveryService;
2324
let executionFactory: IPythonExecutionFactory;
2425
let parser: ITestDiscoveredTestParser;
2526
setup(() => {
27+
// tslint:disable-next-line:no-use-before-declare
28+
output = mock(StubOutput);
2629
executionFactory = mock(PythonExecutionFactory);
2730
parser = mock(TestDiscoveredTestParser);
28-
discovery = new TestsDiscoveryService(instance(executionFactory), instance(parser));
31+
discovery = new TestsDiscoveryService(instance(executionFactory), instance(parser), instance(output));
2932
});
3033
test('Use parser to parse results', async () => {
3134
const options: TestDiscoveryOptions = {
@@ -73,3 +76,17 @@ suite('Unit Tests - Common Discovery', () => {
7376
assert.deepEqual(result, executionResult);
7477
});
7578
});
79+
80+
// tslint:disable:no-empty
81+
82+
//class StubOutput implements OutputChannel {
83+
class StubOutput {
84+
constructor(public name: string) {}
85+
public append(_value: string) {}
86+
public appendLine(_value: string) {}
87+
public clear() {}
88+
//public show(_preserveFocus?: boolean) {}
89+
public show(_column?: ViewColumn | boolean, _preserveFocus?: boolean) {}
90+
public hide() {}
91+
public dispose() {}
92+
}

0 commit comments

Comments
 (0)