From b1e246cf58f7457964c821e1427ca4f41b967a08 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 22 Sep 2025 10:24:11 +0900 Subject: [PATCH 1/6] [forward-port] #1126 https://github.com/cloudbees-oss/smart-tests-cli/pull/1126 --- smart_tests/commands/subset.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 2228a724e..c5edbe330 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -5,6 +5,7 @@ import re import subprocess import sys +from enum import Enum from multiprocessing import Process from os.path import join from typing import Annotated, Any, Callable, Dict, List, TextIO @@ -34,6 +35,12 @@ app = typer.Typer(name="subset", help="Subsetting tests") +class SubsetUseCase(str, Enum): + ONE_COMMIT = "one-commit" + FEATURE_BRANCH = "feature-branch" + RECURRING = "recurring" + + @app.callback() def subset( ctx: typer.Context, @@ -111,7 +118,11 @@ def subset( is_get_tests_from_guess: Annotated[bool, typer.Option( "--get-tests-from-guess", help="Get subset list from guessed tests" - )] = False + )] = False, + use_case: Annotated[SubsetUseCase | None, typer.Option( + "--use-case", + hidden=True + )] = None, ): app = ctx.obj tracking_client = TrackingClient(Command.SUBSET, app=app) @@ -352,6 +363,9 @@ def get_payload( if prioritized_tests_mapping_file: payload['prioritizedTestsMapping'] = json.load(prioritized_tests_mapping_file) + if use_case: + payload['changesUnderTest'] = use_case.value + return payload def _collect_potential_test_files(self): From e8f1ddeb3cc07454adc065d6cb0bcd3f59f18601 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 22 Sep 2025 10:45:05 +0900 Subject: [PATCH 2/6] [forward-port] #1127 ref: https://github.com/cloudbees-oss/smart-tests-cli/pull/1127 --- smart_tests/commands/subset.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index c5edbe330..2a4d3e642 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -415,6 +415,8 @@ def request_subset(self) -> SubsetResult: if res.status_code == 422: print_error_and_die("Error: {}".format(res.reason), tracking_client, Tracking.ErrorEvent.USER_ERROR) + res.raise_for_status() + return SubsetResult.from_response(res.json()) except Exception as e: tracking_client.send_error_event( From b09d721f343ffbb60825e2267f8b2a58c32e4cf6 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 22 Sep 2025 10:47:09 +0900 Subject: [PATCH 3/6] [forward-port] #1129 https://github.com/cloudbees-oss/smart-tests-cli/pull/1129 --- smart_tests/commands/compare/subsets.py | 2 +- smart_tests/commands/inspect/subset.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/smart_tests/commands/compare/subsets.py b/smart_tests/commands/compare/subsets.py index 351e393a9..1be312320 100644 --- a/smart_tests/commands/compare/subsets.py +++ b/smart_tests/commands/compare/subsets.py @@ -52,4 +52,4 @@ def subsets( (before, after, f"{diff:+}" if isinstance(diff, int) else diff, test) for before, after, diff, test in rows ] - typer.echo(tabulate(tabular_data, headers=headers, tablefmt="github")) + typer.echo_via_pager(tabulate(tabular_data, headers=headers, tablefmt="github")) diff --git a/smart_tests/commands/inspect/subset.py b/smart_tests/commands/inspect/subset.py index 097fd8191..74fcc7755 100644 --- a/smart_tests/commands/inspect/subset.py +++ b/smart_tests/commands/inspect/subset.py @@ -65,7 +65,7 @@ def display(self): result._estimated_duration_sec, ] ) - typer.echo(tabulate(rows, header, tablefmt="github", floatfmt=".2f")) + typer.echo_via_pager(tabulate(rows, header, tablefmt="github", floatfmt=".2f")) class SubsetResultJSONDisplay(SubsetResultAbstractDisplay): From e7c72fc585afd3bdb96d98ce84df0d008389724a Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 22 Sep 2025 12:30:53 +0900 Subject: [PATCH 4/6] [forward port] #1130, #1134 https://github.com/cloudbees-oss/smart-tests-cli/pull/1130 https://github.com/cloudbees-oss/smart-tests-cli/pull/1134 --- smart_tests/__main__.py | 3 + smart_tests/commands/detect_flakes.py | 97 ++++++++++++++++++++++ smart_tests/test_runners/bazel.py | 3 + smart_tests/test_runners/file.py | 3 + smart_tests/test_runners/rspec.py | 1 + smart_tests/test_runners/smart_tests.py | 31 +++++++ smart_tests/utils/commands.py | 1 + smart_tests/utils/test_runner_registry.py | 6 ++ tests/commands/test_detect_flakes.py | 99 +++++++++++++++++++++++ 9 files changed, 244 insertions(+) create mode 100644 smart_tests/commands/detect_flakes.py create mode 100644 tests/commands/test_detect_flakes.py diff --git a/smart_tests/__main__.py b/smart_tests/__main__.py index 9f1904a1a..153b1604e 100644 --- a/smart_tests/__main__.py +++ b/smart_tests/__main__.py @@ -9,6 +9,7 @@ import typer from smart_tests.app import Application +from smart_tests.commands import detect_flakes from smart_tests.commands.record.tests import create_nested_commands as create_record_target_commands from smart_tests.commands.subset import create_nested_commands as create_subset_target_commands from smart_tests.utils.test_runner_registry import get_registry @@ -150,6 +151,7 @@ def main( app.add_typer(inspect.app, name="inspect") app.add_typer(stats.app, name="stats") app.add_typer(compare.app, name="compare") + app.add_typer(detect_flakes.app, name="detect-flakes") # Add record-target as a sub-app to record command record.app.add_typer(record_target_app, name="test") # Use NestedCommand version @@ -162,6 +164,7 @@ def main( app.add_typer(verify.app, name="verify") app.add_typer(inspect.app, name="inspect") app.add_typer(stats.app, name="stats") + app.add_typer(detect_flakes.app, name="detect-flakes") app.callback()(main) diff --git a/smart_tests/commands/detect_flakes.py b/smart_tests/commands/detect_flakes.py new file mode 100644 index 000000000..93dece59a --- /dev/null +++ b/smart_tests/commands/detect_flakes.py @@ -0,0 +1,97 @@ +from enum import Enum +import os +from typing import Annotated +import typer + +from smart_tests.app import Application +from smart_tests.commands.test_path_writer import TestPathWriter +from smart_tests.testpath import unparse_test_path +from smart_tests.utils.commands import Command +from smart_tests.utils.dynamic_commands import DynamicCommandBuilder, extract_callback_options +from smart_tests.utils.env_keys import REPORT_ERROR_KEY +from smart_tests.utils.exceptions import print_error_and_die +from smart_tests.utils.session import get_session +from smart_tests.utils.smart_tests_client import SmartTestsClient +from smart_tests.utils.tracking import Tracking, TrackingClient +from smart_tests.utils.typer_types import ignorable_error + + +class DetectFlakesRetryThreshold(str, Enum): + LOW = "LOW" + MEDIUM = "MEDIUM" + HIGH = "HIGH" + + +app = typer.Typer(name="detect-flakes", help="Detect flaky tests") + + +@app.callback() +def detect_flakes( + ctx: typer.Context, + session: Annotated[str, typer.Option( + "--session", + help="In the format builds//test_sessions/", + metavar="SESSION" + )], + retry_threshold: Annotated[DetectFlakesRetryThreshold, typer.Option( + "--retry-threshold", + help="Thoroughness of how \"flake\" is detected", + case_sensitive=False, + show_default=True, + )] = DetectFlakesRetryThreshold.MEDIUM, +): + app = ctx.obj + tracking_client = TrackingClient(Command.DETECT_FLAKE, app=app) + test_runner = getattr(ctx, 'test_runner', None) + client = SmartTestsClient(app=app, tracking_client=tracking_client, test_runner=test_runner) + + test_session = None + try: + test_session = get_session(client=client, session=session) + except ValueError as e: + print_error_and_die(msg=str(e), tracking_client=tracking_client, event=Tracking.ErrorEvent.USER_ERROR) + except Exception as e: + if os.getenv(REPORT_ERROR_KEY): + raise e + else: + typer.echo(ignorable_error(e), err=True) + + if test_session is None: + return + + class FlakeDetection(TestPathWriter): + def __init__(self, app: Application): + super(FlakeDetection, self).__init__(app) + + def run(self): + test_paths = [] + try: + res = client.request( + "get", + "detect-flake", + params={ + "confidence": retry_threshold.value.upper(), + "session-id": os.path.basename(session), + "test-runner": test_runner, + }) + + res.raise_for_status() + test_paths = res.json().get("testPaths", []) + if test_paths: + self.print(test_paths) + typer.echo("Trying to retry the following tests:", err=True) + for detail in res.json().get("testDetails", []): + typer.echo(f"{detail.get('reason'): {unparse_test_path(detail.get('fullTestPath'))}}", err=True) + except Exception as e: + tracking_client.send_error_event( + event_name=Tracking.ErrorEvent.INTERNAL_CLI_ERROR, + stack_trace=str(e), + ) + if os.getenv(REPORT_ERROR_KEY): + raise e + else: + typer.echo(ignorable_error(e), err=True) + + ctx.obj = FlakeDetection(app=ctx.obj) + + diff --git a/smart_tests/test_runners/bazel.py b/smart_tests/test_runners/bazel.py index defa3f9d4..49630e5a5 100644 --- a/smart_tests/test_runners/bazel.py +++ b/smart_tests/test_runners/bazel.py @@ -30,6 +30,9 @@ def subset(client): client.run() +smart_tests.CommonDetectFlakesImpls(__name__).detect_flakes() + + @smart_tests.record.tests def record_tests( client, diff --git a/smart_tests/test_runners/file.py b/smart_tests/test_runners/file.py index d9856f746..f8da60ef1 100644 --- a/smart_tests/test_runners/file.py +++ b/smart_tests/test_runners/file.py @@ -55,3 +55,6 @@ def find_filename(): for r in reports: client.report(r) client.run() + + +smart_tests.CommonDetectFlakesImpls(__name__).detect_flakes() \ No newline at end of file diff --git a/smart_tests/test_runners/rspec.py b/smart_tests/test_runners/rspec.py index ee2c1850e..55b73a5e7 100644 --- a/smart_tests/test_runners/rspec.py +++ b/smart_tests/test_runners/rspec.py @@ -2,3 +2,4 @@ subset = smart_tests.CommonSubsetImpls(__name__).scan_files('*_spec.rb') record_tests = smart_tests.CommonRecordTestImpls(__name__).report_files() +detect_flakes = smart_tests.CommonDetectFlakesImpls(__name__).detect_flakes() diff --git a/smart_tests/test_runners/smart_tests.py b/smart_tests/test_runners/smart_tests.py index 70bf959a5..56f168bc9 100644 --- a/smart_tests/test_runners/smart_tests.py +++ b/smart_tests/test_runners/smart_tests.py @@ -8,6 +8,7 @@ from smart_tests.commands.record.tests import app as record_tests_cmd from smart_tests.commands.subset import app as subset_cmd +from smart_tests.commands.detect_flakes import app as detect_flakes_cmd from smart_tests.utils.test_runner_registry import cmdname, create_test_runner_wrapper, get_registry @@ -34,6 +35,13 @@ def subset(f): return f +def detect_flakes(f): + test_runner_name = cmdname(f.__module__) + registry = get_registry() + registry.register_detect_flakes(test_runner_name, f) + return f + + record = types.SimpleNamespace() @@ -152,3 +160,26 @@ def load_report_files(cls, client, source_roots, file_mask="*.xml"): return client.run() + + +class CommonDetectFlakesImpls: + def __init__( + self, + module_name, + formatter=None, + separator="\n", + ): + self.cmdname = cmdname(module_name) + self._formatter = formatter + self._separator = separator + + def detect_flakes(self): + def detect_flakes(client): + if self._formatter: + client.formatter = self._formatter + if self._separator: + client.separator = self._separator + + client.run() + + return wrap(detect_flakes, detect_flakes_cmd, self.cmdname) \ No newline at end of file diff --git a/smart_tests/utils/commands.py b/smart_tests/utils/commands.py index 6a8e357b6..dde29c98d 100644 --- a/smart_tests/utils/commands.py +++ b/smart_tests/utils/commands.py @@ -8,6 +8,7 @@ class Command(Enum): RECORD_SESSION = 'RECORD_SESSION' SUBSET = 'SUBSET' COMMIT = 'COMMIT' + DETECT_FLAKE = 'DETECT_FLAKE' def display_name(self): return self.value.lower().replace('_', ' ') diff --git a/smart_tests/utils/test_runner_registry.py b/smart_tests/utils/test_runner_registry.py index 52928d8c8..b972013ff 100644 --- a/smart_tests/utils/test_runner_registry.py +++ b/smart_tests/utils/test_runner_registry.py @@ -22,6 +22,7 @@ def __init__(self): self._subset_functions: Dict[str, Callable] = {} self._record_test_functions: Dict[str, Callable] = {} self._split_subset_functions: Dict[str, Callable] = {} + self._detect_flakes_functions: Dict[str, Callable] = {} # Callback to trigger when new test runners are registered self._on_register_callback: Callable[[], None] | None = None @@ -47,6 +48,11 @@ def register_split_subset(self, test_runner_name: str, func: Callable) -> None: if self._on_register_callback: self._on_register_callback() + def register_detect_flakes(self, test_runner_name: str, func: Callable) -> None: + self._detect_flakes_functions[test_runner_name] = func + if self._on_register_callback: + self._on_register_callback() + def get_subset_functions(self) -> Dict[str, Callable]: """Get all registered subset functions.""" return self._subset_functions.copy() diff --git a/tests/commands/test_detect_flakes.py b/tests/commands/test_detect_flakes.py new file mode 100644 index 000000000..72d3fca5e --- /dev/null +++ b/tests/commands/test_detect_flakes.py @@ -0,0 +1,99 @@ +import os +from unittest import mock + +import responses # type: ignore + +from smart_tests.utils.http_client import get_base_url +from tests.cli_test_case import CliTestCase + + +class DetectFlakeTest(CliTestCase): + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_detect_flakes_success(self): + mock_json_response = { + "testPaths": [ + [{"type": "file", "name": "test_flaky_1.py"}], + [{"type": "file", "name": "test_flaky_2.py"}], + ] + } + responses.add( + responses.GET, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/detect-flake", + json=mock_json_response, + status=200, + ) + result = self.cli( + "detect-flakes", + "file", + "--session", self.session, + "--retry-threshold", "high", + mix_stderr=False, + ) + self.assert_success(result) + self.assertIn("test_flaky_1.py", result.stdout) + self.assertIn("test_flaky_2.py", result.stdout) + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_detect_flakes_without_retry_threshold_success(self): + mock_json_response = { + "testPaths": [ + [{"type": "file", "name": "test_flaky_1.py"}], + [{"type": "file", "name": "test_flaky_2.py"}], + ] + } + responses.add( + responses.GET, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/detect-flake", + json=mock_json_response, + status=200, + ) + result = self.cli( + "detect-flakes", + "file", + "--session", self.session, + mix_stderr=False, + ) + self.assert_success(result) + self.assertIn("test_flaky_1.py", result.stdout) + self.assertIn("test_flaky_2.py", result.stdout) + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_detect_flakes_no_flakes(self): + mock_json_response = {"testPaths": []} + responses.add( + responses.GET, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/detect-flake", + json=mock_json_response, + status=200, + ) + result = self.cli( + "detect-flakes", + "file", + "--session", self.session, + "--retry-threshold", "low", + mix_stderr=False, + ) + self.assert_success(result) + self.assertEqual(result.stdout, "") + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_flake_detection_api_error(self): + responses.add( + responses.GET, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/detect-flake", + status=500, + ) + result = self.cli( + "detect-flakes", + "file", + "--session", self.session, + "--retry-threshold", "medium", + mix_stderr=False, + ) + self.assert_exit_code(result, 0) + self.assertIn("Error", result.stderr) + self.assertEqual(result.stdout, "") From 103c981fe9b0fb571dc205723c4ea77808a8ba61 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 22 Sep 2025 12:32:56 +0900 Subject: [PATCH 5/6] follow up forward port #1130 and #1134 to align v2 command option formats --- smart_tests/__main__.py | 11 +- smart_tests/commands/detect_flakes.py | 11 +- smart_tests/test_runners/file.py | 2 +- smart_tests/test_runners/smart_tests.py | 7 +- smart_tests/utils/dynamic_commands.py | 125 ++++++++++++++++++++++ smart_tests/utils/test_runner_registry.py | 5 + 6 files changed, 153 insertions(+), 8 deletions(-) diff --git a/smart_tests/__main__.py b/smart_tests/__main__.py index 153b1604e..1b46ca5c5 100644 --- a/smart_tests/__main__.py +++ b/smart_tests/__main__.py @@ -9,12 +9,12 @@ import typer from smart_tests.app import Application -from smart_tests.commands import detect_flakes +from smart_tests.commands.detect_flakes import create_nested_command_app as create_detect_flakes_commands from smart_tests.commands.record.tests import create_nested_commands as create_record_target_commands from smart_tests.commands.subset import create_nested_commands as create_subset_target_commands from smart_tests.utils.test_runner_registry import get_registry -from .commands import compare, inspect, record, stats, subset, verify +from .commands import compare, detect_flakes, inspect, record, stats, subset, verify from .utils import logger from .utils.env_keys import SKIP_CERT_VERIFICATION from .version import __version__ @@ -30,6 +30,7 @@ try: create_subset_target_commands() create_record_target_commands() + create_detect_flakes_commands() except Exception as e: # If NestedCommand creation fails, continue with legacy commands # This ensures backward compatibility @@ -48,7 +49,8 @@ def _rebuild_nested_commands_with_plugins(): try: # Clear existing commands from nested apps and rebuild - for module_name in ['smart_tests.commands.subset', 'smart_tests.commands.record.tests']: + for module_name in ['smart_tests.commands.subset', 'smart_tests.commands.record.tests', + 'smart_tests.commands.detect_flakes']: module = importlib.import_module(module_name) if hasattr(module, 'nested_command_app'): nested_app = module.nested_command_app @@ -142,6 +144,7 @@ def main( # Use NestedCommand apps if available, otherwise fall back to legacy try: + from smart_tests.commands.detect_flakes import nested_command_app as detect_flakes_target_app from smart_tests.commands.record.tests import nested_command_app as record_target_app from smart_tests.commands.subset import nested_command_app as subset_target_app @@ -151,7 +154,7 @@ def main( app.add_typer(inspect.app, name="inspect") app.add_typer(stats.app, name="stats") app.add_typer(compare.app, name="compare") - app.add_typer(detect_flakes.app, name="detect-flakes") + app.add_typer(detect_flakes_target_app, name="detect-flakes") # Add record-target as a sub-app to record command record.app.add_typer(record_target_app, name="test") # Use NestedCommand version diff --git a/smart_tests/commands/detect_flakes.py b/smart_tests/commands/detect_flakes.py index 93dece59a..8cd5efd10 100644 --- a/smart_tests/commands/detect_flakes.py +++ b/smart_tests/commands/detect_flakes.py @@ -1,6 +1,7 @@ -from enum import Enum import os +from enum import Enum from typing import Annotated + import typer from smart_tests.app import Application @@ -95,3 +96,11 @@ def run(self): ctx.obj = FlakeDetection(app=ctx.obj) +nested_command_app = typer.Typer(name="detect-flakes", help="Detect flaky tests from test files (NestedCommand)") + + +def create_nested_command_app(): + builder = DynamicCommandBuilder() + + callback_options = extract_callback_options(detect_flakes) + builder.create_detect_flakes_commands(nested_command_app, detect_flakes, callback_options) diff --git a/smart_tests/test_runners/file.py b/smart_tests/test_runners/file.py index f8da60ef1..ad84a0baf 100644 --- a/smart_tests/test_runners/file.py +++ b/smart_tests/test_runners/file.py @@ -57,4 +57,4 @@ def find_filename(): client.run() -smart_tests.CommonDetectFlakesImpls(__name__).detect_flakes() \ No newline at end of file +smart_tests.CommonDetectFlakesImpls(__name__).detect_flakes() diff --git a/smart_tests/test_runners/smart_tests.py b/smart_tests/test_runners/smart_tests.py index 56f168bc9..9455f3e6c 100644 --- a/smart_tests/test_runners/smart_tests.py +++ b/smart_tests/test_runners/smart_tests.py @@ -6,9 +6,9 @@ import typer +from smart_tests.commands.detect_flakes import app as detect_flakes_cmd from smart_tests.commands.record.tests import app as record_tests_cmd from smart_tests.commands.subset import app as subset_cmd -from smart_tests.commands.detect_flakes import app as detect_flakes_cmd from smart_tests.utils.test_runner_registry import cmdname, create_test_runner_wrapper, get_registry @@ -182,4 +182,7 @@ def detect_flakes(client): client.run() - return wrap(detect_flakes, detect_flakes_cmd, self.cmdname) \ No newline at end of file + # Register with new registry system for NestedCommand + registry = get_registry() + registry.register_detect_flakes(self.cmdname, detect_flakes) + return wrap(detect_flakes, detect_flakes_cmd, self.cmdname) diff --git a/smart_tests/utils/dynamic_commands.py b/smart_tests/utils/dynamic_commands.py index 758eee9a8..fdebafaf8 100644 --- a/smart_tests/utils/dynamic_commands.py +++ b/smart_tests/utils/dynamic_commands.py @@ -70,6 +70,20 @@ def create_record_test_commands(self, base_app: typer.Typer, # Register the command with the app base_app.command(name=test_runner_name, help=f"Record test results using {test_runner_name}")(combined_command) + def create_detect_flakes_commands(self, base_app: typer.Typer, + base_callback_func: Callable, + base_callback_options: Dict[str, Any]) -> None: + + detect_flakes_functions = self.registry.get_detect_flakes_functions() + for test_runner_name, test_runner_func in detect_flakes_functions.items(): + combined_command = self._create_combined_detects_flakes_command( + test_runner_name, + test_runner_func, + base_callback_func, + base_callback_options + ) + base_app.command(name=test_runner_name, help=f"Detect flaky tests using {test_runner_name}")(combined_command) + def _create_combined_subset_command(self, test_runner_name: str, test_runner_func: Callable, base_callback_func: Callable, @@ -269,6 +283,117 @@ def combined_function(*args, **kwargs): return combined_function + # Note(Konboi): copy from _create_combined_subset_command + def _create_combined_detects_flakes_command(self, test_runner_name: str, test_runner_func: Callable, + base_callback_func: Callable, base_callback_options: Dict[str, Any]) -> Callable: + """Create a combined detect flakes command for a specific test runner.""" + + # Get signatures from both functions + base_sig = inspect.signature(base_callback_func) + test_runner_sig = inspect.signature(test_runner_func) + + # Combine parameters from both functions + combined_params = [] + + # Add ctx parameter first + combined_params.append( + inspect.Parameter('ctx', inspect.Parameter.POSITIONAL_OR_KEYWORD, annotation=typer.Context) + ) + + # Add parameters from base callback (excluding ctx) + for param_name, param in base_sig.parameters.items(): + if param_name != 'ctx': + combined_params.append(param) + + # Add parameters from test runner function (excluding client) + test_runner_params = list(test_runner_sig.parameters.values())[1:] # Skip 'client' + for param in test_runner_params: + # Avoid duplicate parameter names + if param.name not in [p.name for p in combined_params]: + # Ensure parameter has a default value to avoid "non-default follows default" error + if param.default == inspect.Parameter.empty: + # Add a default value for parameters without one + param = param.replace(default=None) + combined_params.append(param) + + # Create the combined function + def combined_function(*args, **kwargs): + # Extract ctx from args/kwargs + ctx = kwargs.get('ctx') or (args[0] if args else None) + + if not ctx: + raise ValueError("Context not found in function arguments") + + # Store test runner name as context attribute for direct access + ctx.test_runner = test_runner_name + + # Prepare arguments for base callback + base_args = {} + # Unused variable removed + + for i, (param_name, param) in enumerate(base_sig.parameters.items()): + if param_name == 'ctx': + base_args[param_name] = ctx + elif param_name in kwargs: + base_args[param_name] = kwargs[param_name] + elif i < len(args): + base_args[param_name] = args[i] + elif param.default != inspect.Parameter.empty: + base_args[param_name] = param.default + + # Call base callback to set up context + base_callback_func(**base_args) + + # Get client from context + client = ctx.obj + + # Store test runner name in client if possible + if hasattr(client, 'set_test_runner'): + client.set_test_runner(test_runner_name) + + # Auto-infer base path if not explicitly provided for all test runners + # This ensures all test runners have access to base_path when needed + has_base_path_attr = hasattr(client, 'base_path') + base_path_is_none = client.base_path is None if has_base_path_attr else False + no_inference_disabled = not kwargs.get('no_base_path_inference', False) + + if has_base_path_attr and base_path_is_none and no_inference_disabled: + + # Attempt to infer base path from current working directory + try: + import pathlib + + from smart_tests.commands.test_path_writer import TestPathWriter + from smart_tests.testpath import FilePathNormalizer + + file_path_normalizer = FilePathNormalizer() + inferred_base_path = file_path_normalizer._auto_infer_base_path(pathlib.Path.cwd().resolve()) + if inferred_base_path: + TestPathWriter.base_path = inferred_base_path + except (ImportError, OSError) as e: + import logging + logging.error(f"Failed to infer base path: {e}") + # If inference fails, continue with None + + # Prepare arguments for test runner function + test_runner_args = [client] # First argument is always client + test_runner_kwargs = {} + + test_runner_param_names = list(test_runner_sig.parameters.keys())[1:] # Skip 'client' + + for param_name in test_runner_param_names: + if param_name in kwargs: + test_runner_kwargs[param_name] = kwargs[param_name] + + # Call test runner function + return test_runner_func(*test_runner_args, **test_runner_kwargs) + + # Set the signature for the combined function + setattr(combined_function, '__signature__', inspect.Signature(combined_params)) + combined_function.__name__ = f"detect_flakes_{test_runner_name.replace('-', '_')}" + + return combined_function + def extract_callback_options(callback_func: Callable) -> Dict[str, Any]: """ diff --git a/smart_tests/utils/test_runner_registry.py b/smart_tests/utils/test_runner_registry.py index b972013ff..1f0ea3f2d 100644 --- a/smart_tests/utils/test_runner_registry.py +++ b/smart_tests/utils/test_runner_registry.py @@ -65,12 +65,17 @@ def get_split_subset_functions(self) -> Dict[str, Callable]: """Get all registered split subset functions.""" return self._split_subset_functions.copy() + def get_detect_flakes_functions(self) -> Dict[str, Callable]: + """Get all registered detect flakes functions.""" + return self._detect_flakes_functions.copy() + def get_all_test_runner_names(self) -> List[str]: """Get all unique test runner names across all command types.""" all_names: set[str] = set() all_names.update(self._subset_functions.keys()) all_names.update(self._record_test_functions.keys()) all_names.update(self._split_subset_functions.keys()) + all_names.update(self._detect_flakes_functions.keys()) return sorted(list(all_names)) From 8fb9d19449846d7a91db068576767e11f6e3646e Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 22 Sep 2025 14:20:28 +0900 Subject: [PATCH 6/6] [forward port] #1128 https://github.com/cloudbees-oss/smart-tests-cli/pull/1128 --- smart_tests/commands/record/build.py | 13 +---- smart_tests/commands/record/session.py | 28 ++++----- smart_tests/utils/link.py | 81 +++++++++++++++++++++++++- tests/commands/record/test_build.py | 44 ++++++++++++++ 4 files changed, 137 insertions(+), 29 deletions(-) diff --git a/smart_tests/commands/record/build.py b/smart_tests/commands/record/build.py index 0364f6920..feffb2790 100644 --- a/smart_tests/commands/record/build.py +++ b/smart_tests/commands/record/build.py @@ -6,8 +6,8 @@ import typer from tabulate import tabulate -from smart_tests.commands.record.session import KeyValue, LinkKind, parse_key_value -from smart_tests.utils.link import CIRCLECI_KEY, GITHUB_ACTIONS_KEY, JENKINS_URL_KEY, capture_link +from smart_tests.commands.record.session import KeyValue, parse_key_value +from smart_tests.utils.link import CIRCLECI_KEY, GITHUB_ACTIONS_KEY, JENKINS_URL_KEY, capture_links from smart_tests.utils.tracking import Tracking, TrackingClient from ...utils import subprocess @@ -292,14 +292,7 @@ def send(ws: List[Workspace]) -> str | None: # TODO(Konboi): port forward #1128 # figure out all the CI links to capture def compute_links(): - _links = capture_link(os.environ) - for k, v in links: - _links.append({ - "title": k, - "url": v, - "kind": LinkKind.CUSTOM_LINK.name, - }) - return _links + return capture_links(link_options=links, env=os.environ) try: lineage = branch or ws[0].branch diff --git a/smart_tests/commands/record/session.py b/smart_tests/commands/record/session.py index ef148c0fb..5b6772284 100644 --- a/smart_tests/commands/record/session.py +++ b/smart_tests/commands/record/session.py @@ -9,7 +9,7 @@ from smart_tests.utils.commands import Command from smart_tests.utils.exceptions import print_error_and_die from smart_tests.utils.fail_fast_mode import set_fail_fast_mode -from smart_tests.utils.link import LinkKind, capture_link +from smart_tests.utils.link import capture_links from smart_tests.utils.no_build import NO_BUILD_BUILD_NAME from smart_tests.utils.smart_tests_client import SmartTestsClient from smart_tests.utils.tracking import Tracking, TrackingClient @@ -78,24 +78,16 @@ def session( if is_no_build: build_name = NO_BUILD_BUILD_NAME - payload = { - "flavors": dict([(f.key, f.value) for f in flavors]), - "isObservation": is_observation, - "noBuild": is_no_build, - "testSuite": test_suite, - "timestamp": parsed_timestamp.isoformat() if parsed_timestamp else None, - } - - _links = capture_link(os.environ) - for link in links: - _links.append({ - "title": link.key, - "url": link.value, - "kind": LinkKind.CUSTOM_LINK.name, - }) - payload["links"] = _links - try: + payload = { + "flavors": dict([(f.key, f.value) for f in flavors]), + "isObservation": is_observation, + "noBuild": is_no_build, + "testSuite": test_suite, + "timestamp": parsed_timestamp.isoformat() if parsed_timestamp else None, + "links": capture_links(link_options=[(link.key, link.value) for link in links], env=os.environ) + } + sub_path = f"builds/{build_name}/test_sessions" res = client.request("post", sub_path, payload=payload) diff --git a/smart_tests/utils/link.py b/smart_tests/utils/link.py index a61be7afc..c5236ac59 100644 --- a/smart_tests/utils/link.py +++ b/smart_tests/utils/link.py @@ -1,5 +1,8 @@ +import re from enum import Enum -from typing import Dict, List, Mapping +from typing import Dict, List, Mapping, Sequence, Tuple + +import typer JENKINS_URL_KEY = 'JENKINS_URL' JENKINS_BUILD_URL_KEY = 'BUILD_URL' @@ -18,6 +21,8 @@ CIRCLECI_BUILD_NUM_KEY = 'CIRCLE_BUILD_NUM' CIRCLECI_JOB_KEY = 'CIRCLE_JOB' +GITHUB_PR_REGEX = re.compile(r"^https://github\.com/[^/]+/[^/]+/pull/\d+$") + class LinkKind(Enum): @@ -66,3 +71,77 @@ def capture_link(env: Mapping[str, str]) -> List[Dict[str, str]]: }) return links + + +def capture_links_from_options(link_options: Sequence[Tuple[str, str]]) -> List[Dict[str, str]]: + """ + Validate user-provided link options, inferring the kind when not explicitly specified. + + Each link option is expected in the format "kind|title=url" or "title=url". + If the kind is not provided, it infers the kind based on the URL pattern. + + Returns: + A list of dictionaries, where each dictionary contains the validated title, URL, and kind for each link. + + Raises: + click.UsageError: If an invalid kind is provided or URL doesn't match with the specified kind. + """ + links = [] + for k, url in link_options: + url = url.strip() + + # if k,v in format "kind|title=url" + if '|' in k: + kind, title = (part.strip() for part in k.split('|', 1)) + if kind not in _valid_kinds(): + msg = f"Invalid kind '{kind}' passed to --link option.\nSupported kinds are {_valid_kinds()}" + raise typer.BadParameter(typer.style(msg, fg=typer.colors.RED)) + + if not _url_matches_kind(url, kind): + msg = f"Invalid url '{url}' passed to --link option.\nURL doesn't match with the specified kind '{kind}'" + raise typer.BadParameter(typer.style(msg, fg=typer.colors.RED)) + # if k,v in format "title=url" + else: + kind = _infer_kinds(url) + title = k.strip() + + links.append({ + "title": title, + "url": url, + "kind": kind, + }) + + return links + + +def capture_links(link_options: Sequence[Tuple[str, str]], env: Mapping[str, str]) -> List[Dict[str, str]]: + links = capture_links_from_options(link_options) + + env_links = capture_link(env) + for env_link in env_links: + if not _has_kind(links, env_link['kind']): + links.append(env_link) + + return links + + +def _infer_kinds(url: str) -> str: + if GITHUB_PR_REGEX.match(url): + return LinkKind.GITHUB_PULL_REQUEST.name + + return LinkKind.CUSTOM_LINK.name + + +def _has_kind(input_links: List[Dict[str, str]], kind: str) -> bool: + return any(link for link in input_links if link['kind'] == kind) + + +def _valid_kinds() -> List[str]: + return [kind.name for kind in LinkKind if kind != LinkKind.LINK_KIND_UNSPECIFIED] + + +def _url_matches_kind(url: str, kind: str) -> bool: + if kind == LinkKind.GITHUB_PULL_REQUEST.name: + return bool(GITHUB_PR_REGEX.match(url)) + + return True diff --git a/tests/commands/record/test_build.py b/tests/commands/record/test_build.py index 2131a4e6f..4c229e8c9 100644 --- a/tests/commands/record/test_build.py +++ b/tests/commands/record/test_build.py @@ -414,3 +414,47 @@ def test_with_link(self): ], "timestamp": None }, payload) + + # with invalid kind + result = self.cli( + "record", "build", "--build", self.build_name, + '--link', 'UNKNOWN_KIND|PR=https://github.com/cloudbees-oss/smart-tests/pull/1', + # set these options for easy to check payload + "--no-commit-collection", + "--branch", "main", + "--commit", "app=abc12", + ) + self.assertIn("Invalid kind 'UNKNOWN_KIND' passed to --link option.", result.output) + + # with invalid URL + result = self.cli( + "record", "build", "--build", self.build_name, + '--link', 'GITHUB_PULL_REQUEST|PR=https://smart-tests.test/pull/1/files', + # set these options for easy to check payload + "--no-commit-collection", + "--branch", "main", + "--commit", "app=abc12", + ) + self.assertIn("Invalid url 'https://smart-tests.test/pull/1/files' passed to --link option.", result.output) + + # with infer kind + result = self.cli( + "record", "build", "--build", self.build_name, + '--link', 'PR=https://smart-tests.test/pull/1', + # set these options for easy to check payload + "--no-commit-collection", + "--branch", "main", + "--commit", "app=abc12", + ) + self.assert_success(result) + + # with explicit kind + result = self.cli( + "record", "build", "--build", self.build_name, + '--link', 'GITHUB_PULL_REQUEST|PR=https://smart-tests.test/pull/1', + # set these options for easy to check payload + "--no-commit-collection", + "--branch", "main", + "--commit", "app=abc12", + ) + self.assert_success(result)