Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch testing output to test result panel #22039

Merged
merged 26 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f5230da
switch testing output to test result panel
eleanorjboyd Sep 20, 2023
1aebbb7
remove sleep
eleanorjboyd Sep 21, 2023
d2ffd0d
fix existing tests
eleanorjboyd Sep 25, 2023
48b68a5
add output channel msg
eleanorjboyd Sep 25, 2023
83934c7
add test python side
eleanorjboyd Sep 25, 2023
93196a3
fix new line
eleanorjboyd Sep 25, 2023
8d94714
Merge branch 'main' into switch-output-location
eleanorjboyd Sep 25, 2023
3fc1f20
update with comment
eleanorjboyd Sep 25, 2023
7455d87
remove unneeded import
eleanorjboyd Sep 25, 2023
4c891f1
edits from feedback and collect discovery
eleanorjboyd Sep 25, 2023
5d8e61f
remove color addition
eleanorjboyd Sep 25, 2023
506c320
Merge branch 'main' into switch-output-location
eleanorjboyd Oct 9, 2023
9f270d6
switch testing output to test result panel
eleanorjboyd Sep 20, 2023
09e3868
fix existing tests
eleanorjboyd Sep 25, 2023
9f4598a
add output channel msg
eleanorjboyd Sep 25, 2023
3405f27
fix new line
eleanorjboyd Sep 25, 2023
b2ab4f7
edits from feedback and collect discovery
eleanorjboyd Sep 25, 2023
9a67e66
remove color addition
eleanorjboyd Sep 25, 2023
6a1f01b
fix syntax issues from merge
eleanorjboyd Oct 9, 2023
9db80b9
fix tests
eleanorjboyd Oct 9, 2023
a7c6c61
fix linting
eleanorjboyd Oct 9, 2023
2d0830a
fix failing tests
eleanorjboyd Oct 9, 2023
6ecaa9a
fix to dynamic print
eleanorjboyd Oct 9, 2023
f9bc755
switch to using constant for deprecation msg
eleanorjboyd Oct 9, 2023
1bfc34b
Update src/client/testing/testController/common/utils.ts
eleanorjboyd Oct 10, 2023
6b8444a
Merge branch 'main' into switch-output-location
eleanorjboyd Oct 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pythonFiles/tests/pytestadapter/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def runner_with_cwd(
"pytest",
"-p",
"vscode_pytest",
"-s",
] + args
listener: socket.socket = create_server()
_, port = listener.getsockname()
Expand Down
2 changes: 2 additions & 0 deletions pythonFiles/tests/pytestadapter/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from . import expected_discovery_test_output
from .helpers import TEST_DATA_PATH, runner, runner_with_cwd

pytestmark = pytest.mark.skip(reason="Skipping all tests in this module")


def test_import_error(tmp_path):
"""Test pytest discovery on a file that has a pytest marker but does not import pytest.
Expand Down
376 changes: 190 additions & 186 deletions pythonFiles/tests/pytestadapter/test_execution.py

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pythonFiles/vscode_pytest/run_pytest_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
# Clear the buffer as complete JSON object is received
buffer = b""

# Process the JSON data
# Process the JSON data.
eleanorjboyd marked this conversation as resolved.
Show resolved Hide resolved
print("Received JSON data in run script")
break
except json.JSONDecodeError:
Expand Down
41 changes: 32 additions & 9 deletions src/client/testing/testController/common/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
createEOTPayload,
createExecutionErrorPayload,
extractJsonPayload,
fixLogLines,
} from './utils';
import { createDeferred } from '../../../common/utils/async';

Expand Down Expand Up @@ -86,7 +87,7 @@ export class PythonTestServer implements ITestServer, Disposable {
// what payload is so small it doesn't include the whole UUID think got this
if (extractedJsonPayload.uuid !== undefined && extractedJsonPayload.cleanedJsonData !== undefined) {
// if a full json was found in the buffer, fire the data received event then keep cycling with the remaining raw data.
traceInfo(`Firing data received event, ${extractedJsonPayload.cleanedJsonData}`);
traceLog(`Firing data received event, ${extractedJsonPayload.cleanedJsonData}`);
karthiknadig marked this conversation as resolved.
Show resolved Hide resolved
this._fireDataReceived(extractedJsonPayload.uuid, extractedJsonPayload.cleanedJsonData);
}
buffer = Buffer.from(extractedJsonPayload.remainingRawData);
Expand Down Expand Up @@ -170,6 +171,7 @@ export class PythonTestServer implements ITestServer, Disposable {
callback?: () => void,
): Promise<void> {
const { uuid } = options;
const isDiscovery = testIds === undefined;

const pythonPathParts: string[] = process.env.PYTHONPATH?.split(path.delimiter) ?? [];
const pythonPathCommand = [options.cwd, ...pythonPathParts].join(path.delimiter);
Expand All @@ -189,14 +191,22 @@ export class PythonTestServer implements ITestServer, Disposable {
resource: options.workspaceFolder,
};
const execService = await this.executionFactory.createActivatedEnvironment(creationOptions);

// Add the generated UUID to the data to be sent (expecting to receive it back).
// first check if we have testIds passed in (in case of execution) and
// insert appropriate flag and test id array
const args = [options.command.script, '--port', this.getPort().toString(), '--uuid', uuid].concat(
options.command.args,
);

// If the user didn't explicit dictate the color during run, then add it
if (isRun) {
if (!args.includes('--color=no')) {
if (!args.includes('--color=yes')) {
args.push('--color=yes');
}
}
}
karthiknadig marked this conversation as resolved.
Show resolved Hide resolved

if (options.outChannel) {
options.outChannel.appendLine(`python ${args.join(' ')}`);
}
Expand Down Expand Up @@ -232,15 +242,28 @@ export class PythonTestServer implements ITestServer, Disposable {

// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
result?.proc?.stdout?.on('data', (data) => {
spawnOptions?.outputChannel?.append(data.toString());
});
result?.proc?.stderr?.on('data', (data) => {
spawnOptions?.outputChannel?.append(data.toString());
});
// Discovery output should be sent to the output channel, run output should be sent to the test run instance.
if (isDiscovery) {
result?.proc?.stdout?.on('data', (data) => {
const out = fixLogLines(data.toString());
traceLog(out);
});
result?.proc?.stderr?.on('data', (data) => {
const out = fixLogLines(data.toString());
traceLog(out);
});
} else {
result?.proc?.stdout?.on('data', (data) => {
runInstance?.appendOutput(`${fixLogLines(data.toString())}\r\n`);
});
result?.proc?.stderr?.on('data', (data) => {
runInstance?.appendOutput(`${fixLogLines(data.toString())}\r\n`);
});
}

result?.proc?.on('exit', (code, signal) => {
// if the child has testIds then this is a run request
if (code !== 0 && testIds && testIds?.length !== 0) {
if (code !== 0 && !isDiscovery) {
traceError(
`Subprocess exited unsuccessfully with exit code ${code} and signal ${signal}. Creating and sending error execution payload`,
);
Expand Down
22 changes: 16 additions & 6 deletions src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import {
import { IConfigurationService, ITestOutputChannel } from '../../../common/types';
import { Deferred, createDeferred } from '../../../common/utils/async';
import { EXTENSION_ROOT_DIR } from '../../../constants';
import { traceError, traceInfo, traceVerbose } from '../../../logging';
import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging';
import {
DataReceivedEvent,
DiscoveredTestPayload,
ITestDiscoveryAdapter,
ITestResultResolver,
ITestServer,
} from '../common/types';
import { createDiscoveryErrorPayload, createEOTPayload } from '../common/utils';
import { createDiscoveryErrorPayload, createEOTPayload, fixLogLines } from '../common/utils';

/**
* Wrapper class for unittest test discovery. This is where we call `runTestCommand`. #this seems incorrectly copied
Expand Down Expand Up @@ -84,17 +84,24 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
// delete UUID following entire discovery finishing.
const deferredExec = createDeferred<ExecutionResult<string>>();
const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(pytestArgs);

let execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(pytestArgs);
// filter out color=yes from pytestArgs
execArgs = execArgs.filter((item) => item !== '--color=yes');
eleanorjboyd marked this conversation as resolved.
Show resolved Hide resolved
eleanorjboyd marked this conversation as resolved.
Show resolved Hide resolved
traceVerbose(`Running pytest discovery with command: ${execArgs.join(' ')}`);
const result = execService?.execObservable(execArgs, spawnOptions);

// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
result?.proc?.stdout?.on('data', (data) => {
spawnOptions.outputChannel?.append(data.toString());
const out = fixLogLines(data.toString());
traceLog(out);
// spawnOptions.outputChannel?.append(data.toString());
});
result?.proc?.stderr?.on('data', (data) => {
spawnOptions.outputChannel?.append(data.toString());
const out = fixLogLines(data.toString());
traceError(out);
// spawnOptions.outputChannel?.append(data.toString());
});
result?.proc?.on('exit', (code, signal) => {
if (code !== 0) {
Expand All @@ -112,7 +119,10 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
data: JSON.stringify(createEOTPayload(true)),
});
}
deferredExec.resolve({ stdout: '', stderr: '' });
deferredExec.resolve({
stdout: '',
stderr: '',
});
deferred.resolve();
});

Expand Down
18 changes: 15 additions & 3 deletions src/client/testing/testController/pytest/pytestExecutionAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,15 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);

try {
const colorOff = pytestArgs.includes('--color=no');
// Remove positional test folders and files, we will add as needed per node
const testArgs = removePositionalFoldersAndFiles(pytestArgs);
// If the user didn't explicit dictate the color, then add it
if (!colorOff) {
if (!testArgs.includes('--color=yes')) {
testArgs.push('--color=yes');
}
}

// if user has provided `--rootdir` then use that, otherwise add `cwd`
if (testArgs.filter((a) => a.startsWith('--rootdir')).length === 0) {
Expand Down Expand Up @@ -166,7 +173,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {

const deferredExec = createDeferred<ExecutionResult<string>>();
const result = execService?.execObservable(runArgs, spawnOptions);

runInstance?.token.onCancellationRequested(() => {
traceInfo('Test run cancelled, killing pytest subprocess.');
result?.proc?.kill();
Expand All @@ -175,10 +181,16 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
// Take all output from the subprocess and add it to the test output channel. This will be the pytest output.
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
result?.proc?.stdout?.on('data', (data) => {
this.outputChannel?.append(data.toString());
const out = utils.fixLogLines(data.toString());
runInstance?.appendOutput(`${out}\r\n`);
eleanorjboyd marked this conversation as resolved.
Show resolved Hide resolved
// with traceInfo, gets full message, without traceInfo, gets truncated
// traceInfo(`${out}\r\n`);
});
result?.proc?.stderr?.on('data', (data) => {
this.outputChannel?.append(data.toString());
const out = utils.fixLogLines(data.toString());
runInstance?.appendOutput(`${out}\r\n`);
// traceInfo(`${out}\r\n`);
// console.log(`${out}\r\n`);
});

result?.proc?.on('exit', (code, signal) => {
Expand Down
13 changes: 13 additions & 0 deletions src/testTestingRootWkspc/loggingWorkspace/test_logging.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
import logging


def test_logging(caplog):
logger = logging.getLogger(__name__)
caplog.set_level(logging.DEBUG) # Set minimum log level to capture

logger.debug("This is a debug message.")
logger.info("This is an info message.")
logger.warning("This is a warning message.")
logger.error("This is an error message.")
logger.critical("This is a critical message.")