From 4addc039e5a20a34d7e2d37ee4257d666734934c Mon Sep 17 00:00:00 2001 From: Andrei Chekun Date: Wed, 30 Oct 2024 13:07:51 +0100 Subject: [PATCH] test.py: Add discovery for C++ tests for pytest Code based on https://github.com/pytest-dev/pytest-cpp. Updated, customized, enhanced to suit current needs. Modify generate report to not modify the names, since it will break xdist way of working. Instead modification will be done in post collect but before executing the tests. --- test/conftest.py | 47 +++++++- test/pylib/cpp/__init__.py | 0 test/pylib/cpp/common_cpp_conftest.py | 130 +++++++++++++++++++++ test/pylib/cpp/facade.py | 80 +++++++++++++ test/pylib/cpp/item.py | 155 ++++++++++++++++++++++++++ test/pylib/report_plugin.py | 15 +-- test/pylib/util.py | 8 ++ test/topology/conftest.py | 6 - 8 files changed, 425 insertions(+), 16 deletions(-) create mode 100644 test/pylib/cpp/__init__.py create mode 100644 test/pylib/cpp/common_cpp_conftest.py create mode 100644 test/pylib/cpp/facade.py create mode 100644 test/pylib/cpp/item.py diff --git a/test/conftest.py b/test/conftest.py index 7e44063535..89269ecc1a 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,3 +1,9 @@ +# +# Copyright (C) 2024-present ScyllaDB +# +# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 +# + import pytest from test.pylib.report_plugin import ReportPlugin @@ -9,7 +15,7 @@ ALL_MODES = {'debug': 'Debug', 'coverage': 'Coverage'} def pytest_addoption(parser): - parser.addoption('--mode', choices=ALL_MODES.keys(), dest="mode", + parser.addoption('--mode', choices=ALL_MODES.keys(), action="append", dest="modes", help="Run only tests for given build mode(s)") parser.addoption('--tmpdir', action='store', default='testlog', help='''Path to temporary test data and log files. The data is further segregated per build mode. Default: ./testlog.''', ) @@ -20,8 +26,43 @@ def pytest_addoption(parser): def build_mode(request): """ This fixture returns current build mode. + This is for running tests through the test.py script, where only one mode is passed to the test """ - return request.config.getoption('mode') + # to avoid issues when there's no provided mode parameter, do it in two steps: get the parameter and if it's not + # None, get the first value from the list + mode = request.config.getoption("modes") + if mode: + return mode[0] + return mode def pytest_configure(config): - config.pluginmanager.register(ReportPlugin()) \ No newline at end of file + config.pluginmanager.register(ReportPlugin()) + +def pytest_collection_modifyitems(config, items): + """ + This is a standard pytest method. + This is needed to modify the test names with dev mode and run id to differ them one from another + """ + run_id = config.getoption('run_id', None) + + for item in items: + # check if this is custom cpp tests that have additional attributes for name modification + if hasattr(item, 'mode'): + # modify name with mode that is always present in cpp tests + item.nodeid = f'{item.nodeid}.{item.mode}' + item.name = f'{item.name}.{item.mode}' + if item.run_id: + item.nodeid = f'{item.nodeid}.{item.run_id}' + item.name = f'{item.name}.{item.run_id}' + else: + # here go python tests that are executed through test.py + # since test.py is responsible for creating several tests with the required mode, + # a list with modes contains only one value, + # that's why in name modification the first element is used + modes = config.getoption('modes') + if modes: + item._nodeid = f'{item._nodeid}.{modes[0]}' + item.name = f'{item.name}.{modes[0]}' + if run_id: + item._nodeid = f'{item._nodeid}.{run_id}' + item.name = f'{item.name}.{run_id}' diff --git a/test/pylib/cpp/__init__.py b/test/pylib/cpp/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/pylib/cpp/common_cpp_conftest.py b/test/pylib/cpp/common_cpp_conftest.py new file mode 100644 index 0000000000..3b791640c9 --- /dev/null +++ b/test/pylib/cpp/common_cpp_conftest.py @@ -0,0 +1,130 @@ +# +# Copyright (C) 2025-present ScyllaDB +# +# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 +# +import collections +import subprocess +from copy import copy +from functools import cache +from pathlib import Path, PosixPath + +import yaml +from pytest import Collector + +from test.pylib.cpp.facade import CppTestFacade +from test.pylib.cpp.item import CppFile +from test.pylib.util import get_modes_to_run + +ALL_MODES = { + 'debug': 'Debug', + 'release': 'RelWithDebInfo', + 'dev': 'Dev', + 'sanitize': 'Sanitize', + 'coverage': 'Coverage', +} +DEBUG_MODES = { + 'debug': 'Debug', + 'sanitize': 'Sanitize', +} +DEFAULT_ARGS = [ + '--overprovisioned', + '--unsafe-bypass-fsync 1', + '--kernel-page-cache 1', + '--blocked-reactor-notify-ms 2000000', + '--collectd 0', + '--max-networking-io-control-blocks=100', +] + + +def get_disabled_tests(config: dict, modes: [str]) -> dict[str, set[str]]: + """ + Get the dict with disabled tests. + Pytest spawns one process, so all modes should be handled there instead one by one as test.py does. + """ + disabled_tests = {} + for mode in modes: + # Skip tests disabled in suite.yaml + disabled_tests_for_mode = set(config.get('disable', [])) + # Skip tests disabled in the specific mode. + disabled_tests_for_mode.update(config.get('skip_in_' + mode, [])) + # If this mode is one of the debug modes, and there are + # tests disabled in a debug mode, add these tests to the skip list. + if mode in DEBUG_MODES: + disabled_tests_for_mode.update(config.get('skip_in_debug_modes', [])) + # If a test is listed in run_in_, it should only be enabled in + # this mode. Tests not listed in any run_in_ directive should + # run in all modes. Inverting this, we should disable all tests + # that are listed explicitly in some run_in_ where m != mode + # This, of course, may create ambiguity with skip_* settings, + # since the priority of the two is undefined, but oh well. + run_in_m = set(config.get('run_in_' + mode, [])) + for a in ALL_MODES: + if a == mode: + continue + skip_in_m = set(config.get('run_in_' + a, [])) + disabled_tests_for_mode.update(skip_in_m - run_in_m) + disabled_tests[mode] = disabled_tests_for_mode + return disabled_tests + + +def read_suite_config(directory: Path) -> dict[str, str]: + """ + Helper method that will return the configuration from the suite.yaml file + """ + with open(directory / 'suite.yaml', 'r') as cfg_file: + cfg = yaml.safe_load(cfg_file.read()) + if not isinstance(cfg, dict): + raise RuntimeError('Failed to load tests: suite.yaml is empty') + return cfg + +def get_root_path(session) -> Path: + return Path(session.config.rootpath).parent + +def collect_items(file_path: PosixPath, parent: Collector, facade: CppTestFacade) -> object: + """ + Collect c++ test based on the .cc files. C++ test binaries are located in different directory, so the method will take care + to provide the correct path to the binary based on the file name and mode. + """ + run_id = parent.config.getoption('run_id') + modes = get_modes_to_run(parent.session) + project_root = Path(parent.session.config.rootpath).parent + suite_config = read_suite_config(file_path.parent) + no_parallel_cases = suite_config.get('no_parallel_cases', []) + disabled_tests = get_disabled_tests(suite_config, modes) + args = copy(DEFAULT_ARGS) + custom_args_config = suite_config.get('custom_args', {}) + test_name = file_path.stem + no_parallel_run = True if test_name in no_parallel_cases else False + + custom_args = custom_args_config.get(file_path.stem, ['-c2 -m2G']) + if len(custom_args) > 1: + return CppFile.from_parent(parent=parent, path=file_path, arguments=args, parameters=custom_args, + no_parallel_run=no_parallel_run, modes=modes, disabled_tests=disabled_tests, + run_id=run_id, facade=facade, project_root=project_root) + else: + args.extend(custom_args) + return CppFile.from_parent(parent=parent, path=file_path, arguments=args, no_parallel_run=no_parallel_run, + modes=modes, disabled_tests=disabled_tests, run_id=run_id, facade=facade, project_root=project_root) + + +@cache +def get_combined_tests(session): + suites = collections.defaultdict() + executable = get_root_path(session) / 'combined_tests' + args = [executable, '--list_content'] + + output = subprocess.check_output( + args, + stderr=subprocess.STDOUT, + universal_newlines=True, + ) + current_suite = '' + for line in output.splitlines(): + if not line.startswith(' '): + current_suite = line.strip().rstrip('*') + suites[current_suite] = [] + else: + case_name = line.strip().rstrip('*') + suites[current_suite].append(case_name) + return suites \ No newline at end of file diff --git a/test/pylib/cpp/facade.py b/test/pylib/cpp/facade.py new file mode 100644 index 0000000000..f12717059d --- /dev/null +++ b/test/pylib/cpp/facade.py @@ -0,0 +1,80 @@ +# +# Copyright (c) 2014 Bruno Oliveira +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Copyright (C) 2025-present ScyllaDB +# +# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 +# +from __future__ import annotations + +import shlex +import subprocess +from abc import ABC +from pathlib import Path +from subprocess import TimeoutExpired +from typing import Sequence + +from pytest import Config + + +class CppTestFailure(Exception): + def __init__(self, filename: str, line_num: int, contents: str) -> None: + self.filename = filename + self.line_num = line_num + self.lines = contents.splitlines() + + def get_lines(self) -> list[tuple[str, tuple[str, ...]]]: + m = ("red", "bold") + return [(x, m) for x in self.lines] + + def get_file_reference(self) -> tuple[str, int]: + return self.filename, self.line_num + +class CppTestFailureList(Exception): + def __init__(self, failures: Sequence[CppTestFailure]) -> None: + self.failures = list(failures) + +class CppTestFacade(ABC): + def __init__(self, config: Config, combined_tests: dict[str, list[str]] = None): + self.temp_dir: Path = Path(config.getoption('tmpdir')) + self.combined_suites: dict[str, list[str]] = combined_tests + + def list_tests(self, executable: Path , no_parallel: bool) -> tuple[bool,list[str]]: + raise NotImplementedError + + def run_test(self, executable: Path, original_name: str, test_id: str, mode:str, file_name: Path, test_args: Sequence[str] = ()) -> tuple[Sequence[CppTestFailure] | None, str]: + raise NotImplementedError + + +def run_process(args: list[str], timeout): + args = shlex.split(' '.join(args)) + p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + try: + stdout, stderr = p.communicate(timeout=timeout) + except TimeoutExpired: + print('Timeout reached') + p.kill() + stdout = p.stdout.read() + stderr = p.stderr.read() + except KeyboardInterrupt: + p.kill() + raise + return p, stderr, stdout diff --git a/test/pylib/cpp/item.py b/test/pylib/cpp/item.py new file mode 100644 index 0000000000..c38f600e6b --- /dev/null +++ b/test/pylib/cpp/item.py @@ -0,0 +1,155 @@ +# +# Copyright (c) 2014 Bruno Oliveira +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Copyright (C) 2025-present ScyllaDB +# +# SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 +# +from __future__ import annotations + +from pathlib import Path +from typing import Sequence, Any, Iterator + +import pytest +from _pytest._code.code import TerminalRepr, ReprFileLocation +from _pytest._io import TerminalWriter + +from test.pylib.cpp.facade import CppTestFailure, CppTestFailureList, CppTestFacade + + +class CppFailureRepr(object): + failure_sep = "---" + + def __init__(self, failures: Sequence[CppTestFailure]) -> None: + self.failures = failures + + def __str__(self) -> str: + reprs = [] + for failure in self.failures: + pure_lines = "\n".join(x[0] for x in failure.get_lines()) + repr_loc = self._get_repr_file_location(failure) + reprs.append("%s\n%s" % (pure_lines, repr_loc)) + return self.failure_sep.join(reprs) + + def _get_repr_file_location(self, failure: CppTestFailure) -> ReprFileLocation: + filename, line_num = failure.get_file_reference() + return ReprFileLocation(filename, line_num, "C++ failure") + + def toterminal(self, tw: TerminalWriter) -> None: + for index, failure in enumerate(self.failures): + for line, markup in failure.get_lines(): + markup_params = {m: True for m in markup} + tw.line(line, **markup_params) + + location = self._get_repr_file_location(failure) + location.toterminal(tw) + + if index != len(self.failures) - 1: + tw.line(self.failure_sep, cyan=True) + + +class CppTestFunction(pytest.Item): + """ + Represents a single test function in the file. + """ + facade = None + + def __init__(self, *, executable: Path, facade: CppTestFacade, mode: str, test_unique_name: str, arguments: Sequence[str], + file_name: Path, run_id:int = None, **kwargs: Any) -> None: + super().__init__(**kwargs) + self.facade = facade + self.executable = executable + self.mode = mode + self.file_name = file_name + self.originalname = kwargs['name'] + self.test_unique_name = test_unique_name + self._arguments = arguments + self.run_id = run_id + self.fixturenames = [] + self.own_markers = [] + self.add_marker(pytest.mark.cpp) + + @property + def nodeid(self) -> str: + return self._nodeid + + @nodeid.setter + def nodeid(self, nodeid: str) -> None: + self._nodeid = nodeid + + def runtest(self) -> None: + + failures, output = self.facade.run_test(self.executable, self.originalname, self.test_unique_name, self.mode, + self.file_name, self._arguments) + # Report the c++ output in its own sections + self.add_report_section("call", "c++", output) + + if failures: + raise CppTestFailureList(failures) + + def repr_failure( # type:ignore[override] + self, excinfo: pytest.ExceptionInfo[BaseException], **kwargs: Any) -> str | TerminalRepr | CppFailureRepr: + if isinstance(excinfo.value, CppTestFailureList): + return CppFailureRepr(excinfo.value.failures) + return pytest.Item.repr_failure(self, excinfo) + + def reportinfo(self) -> tuple[Any, int, str]: + return self.path, 0, self.originalname + + +class CppFile(pytest.File): + """ + Represents the C++ test file with all necessary information for test execution + """ + def __init__(self, *, no_parallel_run: bool = False, modes: list[str], disabled_tests: dict[str, set[str]], + run_id=None, facade: CppTestFacade, arguments: Sequence[str], parameters: list[str] = None, project_root: Path, + **kwargs: Any) -> None: + super().__init__(**kwargs) + self.facade = facade + self.modes = modes + self.run_id = run_id + self.disabled_tests = disabled_tests + self.no_parallel_run = no_parallel_run + self.parameters = parameters + self.project_root = project_root + self._arguments = arguments + + def collect(self) -> Iterator[CppTestFunction]: + for mode in self.modes: + test_name = self.path.stem + if test_name in self.disabled_tests[mode]: + continue + executable = Path(f'{self.project_root}/build/{mode}/test/{self.path.parent.name}/{test_name}') + combined, tests = self.facade.list_tests(executable, self.no_parallel_run) + if combined: + executable = executable.parent / 'combined_tests' + for test_name in tests: + if '/' in test_name: + test_name = test_name.replace('/', '_') + if self.parameters: + for index, parameter in enumerate(self.parameters): + yield CppTestFunction.from_parent(self, name=test_name, executable=executable, + facade=self.facade, mode=mode, test_unique_name=f'{test_name}.{index + 1}', + file_name=self.path, run_id=self.run_id, + arguments=[*self._arguments, parameter]) + else: + yield CppTestFunction.from_parent(self, name=test_name, executable=executable, facade=self.facade, mode=mode, + file_name=self.path, test_unique_name=test_name, run_id=self.run_id, arguments=self._arguments) diff --git a/test/pylib/report_plugin.py b/test/pylib/report_plugin.py index 864f7eef36..7f6985116f 100644 --- a/test/pylib/report_plugin.py +++ b/test/pylib/report_plugin.py @@ -17,7 +17,10 @@ class ReportPlugin: # Pytest hook to modify test name to include mode and run_id def pytest_configure(self, config): - self.build_mode = config.getoption('mode') + # getting build_mode is needed for the cases when there will be no mode provided + self.build_mode = config.getoption("modes") + if self.build_mode: + self.build_mode = self.build_mode[0] self.config = config self.run_id = config.getoption("run_id") @@ -25,8 +28,6 @@ class ReportPlugin: def pytest_runtest_makereport(self): outcome = yield report = outcome.get_result() - if self.build_mode is not None or self.run_id is not None: - report.nodeid = f"{report.nodeid}.{self.build_mode}.{self.run_id}" status = get_pytest_report_status(report) # skip attaching logs for passed tests # attach_capture is a destination for "--allure-no-capture" option from allure-plugin @@ -44,7 +45,7 @@ class ReportPlugin: Add mode tag to be able to search by it. Add parameters to make allure distinguish them and not put them to retries. """ - request.node.name = f"{request.node.name}.{self.build_mode}.{self.run_id}" - allure.dynamic.tag(self.build_mode) - allure.dynamic.parameter('mode', self.build_mode) - allure.dynamic.parameter('run_id', self.run_id) + if self.build_mode is not None or self.run_id is not None: + allure.dynamic.tag(self.build_mode) + allure.dynamic.parameter('mode', self.build_mode) + allure.dynamic.parameter('run_id', self.run_id) diff --git a/test/pylib/util.py b/test/pylib/util.py index 61191d1b3f..7824ff4140 100644 --- a/test/pylib/util.py +++ b/test/pylib/util.py @@ -276,3 +276,11 @@ def get_configured_modes(): # debug release dev return re.sub(r'.* List configured modes\n(.*)\n', r'\1', out, count=1, flags=re.DOTALL).split('\n')[-1].split(' ') + +def get_modes_to_run(session) -> list[str]: + modes = session.config.getoption('modes') + if not modes: + modes = get_configured_modes(root_dir=pathlib.Path(session.config.rootpath).parent) + if not modes: + raise RuntimeError('No modes configured. Please run ./configure.py first') + return modes \ No newline at end of file diff --git a/test/topology/conftest.py b/test/topology/conftest.py index 97b69e8179..a22db33e98 100644 --- a/test/topology/conftest.py +++ b/test/topology/conftest.py @@ -265,9 +265,3 @@ def skip_mode_fixture(request, build_mode): for reason, platform_key in skipped_funcs.get((request.function, build_mode), []): if platform_key is None or platform_key in platform.platform(): pytest.skip(f'{request.node.name} skipped, reason: {reason}') - - -def pytest_collection_modifyitems(items, config): - run_id = config.getoption('run_id') - for item in items: - item.name = f"{item.name}.{run_id}"