Skip to content

Commit

Permalink
switch to using envvars for port and uuid in unittest (#22131)
Browse files Browse the repository at this point in the history
closes #22130
  • Loading branch information
eleanorjboyd authored Oct 3, 2023
1 parent fc62bd8 commit a363381
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 141 deletions.
30 changes: 2 additions & 28 deletions pythonFiles/tests/unittestadapter/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,13 @@
from typing import List

import pytest
from unittestadapter.discovery import (
DEFAULT_PORT,
discover_tests,
parse_discovery_cli_args,
)
from unittestadapter.discovery import discover_tests
from unittestadapter.utils import TestNodeTypeEnum, parse_unittest_args

from . import expected_discovery_test_output
from .helpers import TEST_DATA_PATH, is_same_tree


@pytest.mark.parametrize(
"args, expected",
[
(["--port", "6767", "--uuid", "some-uuid"], (6767, "some-uuid")),
(["--foo", "something", "--bar", "another"], (int(DEFAULT_PORT), None)),
(["--port", "4444", "--foo", "something", "--port", "9999"], (9999, None)),
(
["--uuid", "first-uuid", "--bar", "other", "--uuid", "second-uuid"],
(int(DEFAULT_PORT), "second-uuid"),
),
],
)
def test_parse_cli_args(args: List[str], expected: List[str]) -> None:
"""The parse_cli_args function should parse and return the port and uuid passed as command-line options.
If there were no --port or --uuid command-line option, it should return default values).
If there are multiple options, the last one wins.
"""
actual = parse_discovery_cli_args(args)

assert expected == actual


@pytest.mark.parametrize(
"args, expected",
[
Expand Down
40 changes: 1 addition & 39 deletions pythonFiles/tests/unittestadapter/test_execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,55 +4,17 @@
import os
import pathlib
import sys
from typing import List

import pytest

script_dir = pathlib.Path(__file__).parent.parent
sys.path.insert(0, os.fspath(script_dir / "lib" / "python"))

from unittestadapter.execution import parse_execution_cli_args, run_tests
from unittestadapter.execution import run_tests

TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data"


@pytest.mark.parametrize(
"args, expected",
[
(
[
"--port",
"111",
"--uuid",
"fake-uuid",
],
(111, "fake-uuid"),
),
(
["--port", "111", "--uuid", "fake-uuid"],
(111, "fake-uuid"),
),
(
[
"--port",
"111",
"--uuid",
"fake-uuid",
"-v",
"-s",
],
(111, "fake-uuid"),
),
],
)
def test_parse_execution_cli_args(args: List[str], expected: List[str]) -> None:
"""The parse_execution_cli_args function should return values for the port, uuid, and testids arguments
when passed as command-line options, and ignore unrecognized arguments.
"""
actual = parse_execution_cli_args(args)
assert actual == expected


def test_no_ids_run() -> None:
"""This test runs on an empty array of test_ids, therefore it should return
an empty dict for the result.
Expand Down
36 changes: 9 additions & 27 deletions pythonFiles/unittestadapter/discovery.py
Original file line number Diff line number Diff line change
@@ -1,46 +1,27 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

import argparse
import json
import os
import pathlib
import sys
import traceback
import unittest
from typing import List, Optional, Tuple, Union
from typing import List, Optional, Union

script_dir = pathlib.Path(__file__).parent.parent
sys.path.append(os.fspath(script_dir))
sys.path.insert(0, os.fspath(script_dir / "lib" / "python"))

from testing_tools import socket_manager
from typing_extensions import Literal, NotRequired, TypedDict

# If I use from utils then there will be an import error in test_discovery.py.
from unittestadapter.utils import TestNode, build_test_tree, parse_unittest_args

from typing_extensions import NotRequired, TypedDict, Literal

DEFAULT_PORT = "45454"


def parse_discovery_cli_args(args: List[str]) -> Tuple[int, Union[str, None]]:
"""Parse command-line arguments that should be processed by the script.
So far this includes the port number that it needs to connect to, and the uuid passed by the TS side.
The port is passed to the discovery.py script when it is executed, and
defaults to DEFAULT_PORT if it can't be parsed.
The uuid should be passed to the discovery.py script when it is executed, and defaults to None if it can't be parsed.
If the arguments appear several times, the value returned by parse_cli_args will be the value of the last argument.
"""
arg_parser = argparse.ArgumentParser()
arg_parser.add_argument("--port", default=DEFAULT_PORT)
arg_parser.add_argument("--uuid")
parsed_args, _ = arg_parser.parse_known_args(args)

return int(parsed_args.port), parsed_args.uuid


class PayloadDict(TypedDict):
cwd: str
status: Literal["success", "error"]
Expand Down Expand Up @@ -141,15 +122,16 @@ def post_response(
start_dir, pattern, top_level_dir = parse_unittest_args(argv[index + 1 :])

# Perform test discovery.
port, uuid = parse_discovery_cli_args(argv[:index])
testPort = int(os.environ.get("TEST_PORT", DEFAULT_PORT))
testUuid = os.environ.get("TEST_UUID")
# Post this discovery payload.
if uuid is not None:
payload = discover_tests(start_dir, pattern, top_level_dir, uuid)
post_response(payload, port, uuid)
if testUuid is not None:
payload = discover_tests(start_dir, pattern, top_level_dir, testUuid)
post_response(payload, testPort, testUuid)
# Post EOT token.
eot_payload: EOTPayloadDict = {"command_type": "discovery", "eot": True}
post_response(eot_payload, port, uuid)
post_response(eot_payload, testPort, testUuid)
else:
print("Error: no uuid provided or parsed.")
eot_payload: EOTPayloadDict = {"command_type": "discovery", "eot": True}
post_response(eot_payload, port, "")
post_response(eot_payload, testPort, "")
45 changes: 11 additions & 34 deletions pythonFiles/unittestadapter/execution.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

import argparse
import atexit
import enum
import json
Expand All @@ -18,40 +17,17 @@
sys.path.append(os.fspath(script_dir))
sys.path.insert(0, os.fspath(script_dir / "lib" / "python"))

from typing_extensions import Literal, NotRequired, TypeAlias, TypedDict

from testing_tools import process_json_util, socket_manager
from typing_extensions import Literal, NotRequired, TypeAlias, TypedDict
from unittestadapter.utils import parse_unittest_args

DEFAULT_PORT = "45454"


def parse_execution_cli_args(
args: List[str],
) -> Tuple[int, Union[str, None]]:
"""Parse command-line arguments that should be processed by the script.
So far this includes the port number that it needs to connect to, the uuid passed by the TS side,
and the list of test ids to report.
The port is passed to the execution.py script when it is executed, and
defaults to DEFAULT_PORT if it can't be parsed.
The list of test ids is passed to the execution.py script when it is executed, and defaults to an empty list if it can't be parsed.
The uuid should be passed to the execution.py script when it is executed, and defaults to None if it can't be parsed.
If the arguments appear several times, the value returned by parse_cli_args will be the value of the last argument.
"""
arg_parser = argparse.ArgumentParser()
arg_parser.add_argument("--port", default=DEFAULT_PORT)
arg_parser.add_argument("--uuid")
parsed_args, _ = arg_parser.parse_known_args(args)

return (int(parsed_args.port), parsed_args.uuid)


ErrorType = Union[
Tuple[Type[BaseException], BaseException, TracebackType], Tuple[None, None, None]
]
PORT = 0
UUID = 0
testPort = 0
testUuid = 0
START_DIR = ""


Expand Down Expand Up @@ -148,9 +124,9 @@ def formatResult(
"subtest": subtest.id() if subtest else None,
}
self.formatted[test_id] = result
if PORT == 0 or UUID == 0:
if testPort == 0 or testUuid == 0:
print("Error sending response, port or uuid unknown to python server.")
send_run_data(result, PORT, UUID)
send_run_data(result, testPort, testUuid)


class TestExecutionStatus(str, enum.Enum):
Expand Down Expand Up @@ -322,11 +298,12 @@ def post_response(
print(f"Error: Could not connect to runTestIdsPort: {e}")
print("Error: Could not connect to runTestIdsPort")

PORT, UUID = parse_execution_cli_args(argv[:index])
testPort = int(os.environ.get("TEST_PORT", DEFAULT_PORT))
testUuid = os.environ.get("TEST_UUID")
if test_ids_from_buffer:
# Perform test execution.
payload = run_tests(
start_dir, test_ids_from_buffer, pattern, top_level_dir, UUID
start_dir, test_ids_from_buffer, pattern, top_level_dir, testUuid
)
else:
cwd = os.path.abspath(start_dir)
Expand All @@ -338,8 +315,8 @@ def post_response(
"result": None,
}
eot_payload: EOTPayloadDict = {"command_type": "execution", "eot": True}
if UUID is None:
if testUuid is None:
print("Error sending response, uuid unknown to python server.")
post_response(eot_payload, PORT, "unknown")
post_response(eot_payload, testPort, "unknown")
else:
post_response(eot_payload, PORT, UUID)
post_response(eot_payload, testPort, testUuid)
4 changes: 2 additions & 2 deletions pythonFiles/vscode_pytest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ def send_post_request(
cls_encoder -- a custom encoder if needed.
"""
testPort = os.getenv("TEST_PORT", 45454)
testuuid = os.getenv("TEST_UUID")
testUuid = os.getenv("TEST_UUID")
addr = ("localhost", int(testPort))
global __socket

Expand All @@ -698,7 +698,7 @@ def send_post_request(
data = json.dumps(payload, cls=cls_encoder)
request = f"""Content-Length: {len(data)}
Content-Type: application/json
Request-uuid: {testuuid}
Request-uuid: {testUuid}
{data}"""

Expand Down
13 changes: 6 additions & 7 deletions src/client/testing/testController/common/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,11 @@ export class PythonTestServer implements ITestServer, Disposable {
cwd: options.cwd,
throwOnStdErr: true,
outputChannel: options.outChannel,
extraVariables: { PYTHONPATH: pythonPathCommand },
extraVariables: {
PYTHONPATH: pythonPathCommand,
TEST_UUID: uuid.toString(),
TEST_PORT: this.getPort().toString(),
},
};

if (spawnOptions.extraVariables) spawnOptions.extraVariables.RUN_TEST_IDS_PORT = runTestIdPort;
Expand All @@ -191,12 +195,7 @@ export class PythonTestServer implements ITestServer, Disposable {
};
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,
);
const args = [options.command.script].concat(options.command.args);

if (options.outChannel) {
options.outChannel.appendLine(`python ${args.join(' ')}`);
Expand Down
18 changes: 14 additions & 4 deletions src/test/testing/testController/server.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,16 @@ suite('Python Test Server, Send command etc', () => {
RUN_TEST_IDS_PORT_CONST,
'Expect test id port to be in extra variables and set correctly',
);
assert.strictEqual(
options2.extraVariables.TEST_UUID,
FAKE_UUID,
'Expect test uuid to be in extra variables and set correctly',
);
assert.strictEqual(
options2.extraVariables.TEST_PORT,
12345,
'Expect server port to be set correctly as a env var',
);
} catch (e) {
assert(false, 'Error parsing data, extra variables do not match');
}
Expand All @@ -203,6 +213,8 @@ suite('Python Test Server, Send command etc', () => {
return Promise.resolve(execService.object);
});
server = new PythonTestServer(execFactory.object, debugLauncher);
sinon.stub(server, 'getPort').returns(12345);
// const portServer = server.getPort();
await server.serverReady();
const options = {
command: { script: 'myscript', args: ['-foo', 'foo'] },
Expand All @@ -215,8 +227,7 @@ suite('Python Test Server, Send command etc', () => {
await deferred2.promise;
mockProc.trigger('close');

const port = server.getPort();
const expectedArgs = ['myscript', '--port', `${port}`, '--uuid', FAKE_UUID, '-foo', 'foo'];
const expectedArgs = ['myscript', '-foo', 'foo'];
execService.verify((x) => x.execObservable(expectedArgs, typeMoq.It.isAny()), typeMoq.Times.once());
});

Expand Down Expand Up @@ -254,8 +265,7 @@ suite('Python Test Server, Send command etc', () => {
await deferred.promise;
mockProc.trigger('close');

const port = server.getPort();
const expected = ['python', 'myscript', '--port', `${port}`, '--uuid', FAKE_UUID, '-foo', 'foo'].join(' ');
const expected = ['python', 'myscript', '-foo', 'foo'].join(' ');

assert.deepStrictEqual(output2, [expected]);
});
Expand Down

0 comments on commit a363381

Please sign in to comment.