From 9c8a2fcae79f543e4d003e437eebf1442e98ef09 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 15:26:59 +0900 Subject: [PATCH 01/14] [AIENG-294] add options for split subset --- 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 bf5357ea2..8807e350b 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -29,7 +29,7 @@ from ..utils.fail_fast_mode import (FailFastModeValidateParams, fail_fast_mode_validate, set_fail_fast_mode, warn_and_exit_if_fail_fast_mode) from ..utils.smart_tests_client import SmartTestsClient -from ..utils.typer_types import Duration, Percentage, parse_duration, parse_percentage +from ..utils.typer_types import Duration, Fraction, Percentage, parse_duration, parse_fraction, parse_percentage from .test_path_writer import TestPathWriter @@ -174,6 +174,18 @@ def __init__( type=fileText(mode="r"), metavar="FILE" )] = None, + bin_target: Annotated[Fraction | None, typer.Option( + "--bin", + help="Split subset into bins, e.g. --bin 1/4", + metavar="INDEX/COUNT", + type=parse_fraction + )] = None, + same_bin_files: Annotated[List[str], typer.Option( + "--same-bin", + help="Keep all tests listed in the file together when splitting; one test per line", + metavar="FILE", + multiple=True + )] = [], is_get_tests_from_guess: Annotated[bool, typer.Option( "--get-tests-from-guess", help="Get subset list from guessed tests" @@ -255,6 +267,8 @@ def warn(msg: str): self.ignore_flaky_tests_above = ignore_flaky_tests_above self.prioritize_tests_failed_within_hours = prioritize_tests_failed_within_hours self.prioritized_tests_mapping_file = prioritized_tests_mapping_file + self.bin_target = bin_target + self.same_bin_files = list(same_bin_files) self.is_get_tests_from_guess = is_get_tests_from_guess self.use_case = use_case From 26d834ae4668a77d9f73fe016942247ab3133777 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 15:28:00 +0900 Subject: [PATCH 02/14] [AIENG-294] update samebin formatter used by TestPath --- smart_tests/commands/subset.py | 90 ++++++++++++++++++++++++ smart_tests/commands/test_path_writer.py | 8 +-- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 8807e350b..51be08148 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -418,8 +418,98 @@ def get_payload(self) -> dict[str, Any]: if self.use_case: payload['changesUnderTest'] = self.use_case.value + split_subset = self._build_split_subset_payload() + if split_subset: + payload['splitSubset'] = split_subset + return payload + def _build_split_subset_payload(self) -> dict[str, Any] | None: + if self.bin_target is None: + if self.same_bin_files: + print_error_and_die( + "--same-bin requires --bin", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + return None + + slice_index = self.bin_target.numerator + slice_count = self.bin_target.denominator + + if slice_index <= 0 or slice_count <= 0: + print_error_and_die( + "Invalid --bin value. Both index and count must be positive integers.", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + + if slice_count < slice_index: + print_error_and_die( + "Invalid --bin value. The numerator cannot exceed the denominator.", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + + same_bins = self._read_same_bin_files() + + return { + "sliceIndex": slice_index, + "sliceCount": slice_count, + "sameBins": same_bins, + } + + def _read_same_bin_files(self) -> list[list[TestPath]]: + if not self.same_bin_files: + return [] + + formatter = self.same_bin_formatter + if formatter is None: + print_error_and_die( + "--same-bin is not supported for this test runner.", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + + same_bins: list[list[TestPath]] = [] + seen_tests: set[str] = set() + + for same_bin_file in self.same_bin_files: + try: + with open(same_bin_file, "r", encoding="utf-8") as fp: + tests = [line.strip() for line in fp if line.strip()] + except OSError as exc: + print_error_and_die( + f"Failed to read --same-bin file '{same_bin_file}': {exc}", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + + unique_tests = list(dict.fromkeys(tests)) + + group: list[TestPath] = [] + for test in unique_tests: + if test in seen_tests: + print_error_and_die( + f"Error: test '{test}' is listed in multiple --same-bin files.", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + seen_tests.add(test) + + formatted = formatter(test) + if not formatted: + print_error_and_die( + f"Failed to parse test '{test}' from --same-bin file {same_bin_file}", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + group.append(formatted) + + same_bins.append(group) + + return same_bins + def _collect_potential_test_files(self): LOOSE_TEST_FILE_PATTERN = r'(\.(test|spec)\.|_test\.|Test\.|Spec\.|test/|tests/|__tests__/|src/test/)' EXCLUDE_PATTERN = r'(BUILD|Makefile|Dockerfile|LICENSE|.gitignore|.gitkeep|.keep|id_rsa|rsa|blank|taglib)|\.(xml|json|jsonl|txt|yml|yaml|toml|md|png|jpg|jpeg|gif|svg|sql|html|css|graphql|proto|gz|zip|rz|bzl|conf|config|snap|pem|crt|key|lock|jpi|hpi|jelly|properties|jar|ini|mod|sum|bmp|env|envrc|sh)$' # noqa E501 diff --git a/smart_tests/commands/test_path_writer.py b/smart_tests/commands/test_path_writer.py index f45865aca..6dbcae05b 100644 --- a/smart_tests/commands/test_path_writer.py +++ b/smart_tests/commands/test_path_writer.py @@ -1,5 +1,5 @@ from os.path import join -from typing import Callable, Dict, List +from typing import Callable, List import click @@ -19,7 +19,7 @@ class TestPathWriter(object): def __init__(self, app: Application): self.formatter = self.default_formatter - self._same_bin_formatter: Callable[[str], Dict[str, str]] | None = None + self._same_bin_formatter: Callable[[str], TestPath] | None = None self.separator = "\n" self.app = app @@ -43,9 +43,9 @@ def print(self, test_paths: List[TestPath]): for t in test_paths)) @property - def same_bin_formatter(self) -> Callable[[str], Dict[str, str]] | None: + def same_bin_formatter(self) -> Callable[[str], TestPath] | None: return self._same_bin_formatter @same_bin_formatter.setter - def same_bin_formatter(self, v: Callable[[str], Dict[str, str]]): + def same_bin_formatter(self, v: Callable[[str], TestPath]): self._same_bin_formatter = v From f03c7a6c7ff68dcb570a3f856f42bd19eaf0331a Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 15:28:31 +0900 Subject: [PATCH 03/14] [AIENG-294] set same bin formatter --- smart_tests/test_runners/go_test.py | 1 + smart_tests/test_runners/gradle.py | 2 ++ smart_tests/test_runners/maven.py | 2 ++ 3 files changed, 5 insertions(+) diff --git a/smart_tests/test_runners/go_test.py b/smart_tests/test_runners/go_test.py index 880ea8c9e..4a52fbd8c 100644 --- a/smart_tests/test_runners/go_test.py +++ b/smart_tests/test_runners/go_test.py @@ -41,6 +41,7 @@ def subset(client: Subset): test_cases = [] client.formatter = lambda x: f"^{x[1]['name']}$" client.separator = '|' + client.same_bin_formatter = format_same_bin client.run() diff --git a/smart_tests/test_runners/gradle.py b/smart_tests/test_runners/gradle.py index 1ef70c2f3..ff79ceacf 100644 --- a/smart_tests/test_runners/gradle.py +++ b/smart_tests/test_runners/gradle.py @@ -77,6 +77,8 @@ def exclusion_output_handler(subset_tests, rest_tests): client.formatter = lambda x: f"--tests {x[0]['name']}" client.separator = ' ' + client.same_bin_formatter = lambda s: [{"type": "class", "name": s}] + client.run() diff --git a/smart_tests/test_runners/maven.py b/smart_tests/test_runners/maven.py index 90dd968d5..ead1ab44a 100644 --- a/smart_tests/test_runners/maven.py +++ b/smart_tests/test_runners/maven.py @@ -126,6 +126,8 @@ def file2test(f: str) -> List | None: for root in source_roots: client.scan(root, '**/*', file2test) + client.same_bin_formatter = lambda s: [{"type": "class", "name": s}] + client.run() From 68849913bccae0050df629d2050332b52e6c8a0a Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 15:28:42 +0900 Subject: [PATCH 04/14] [AIENG-294] add tests --- tests/commands/test_subset.py | 128 ++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/tests/commands/test_subset.py b/tests/commands/test_subset.py index afc5d75ee..7127fc7b7 100644 --- a/tests/commands/test_subset.py +++ b/tests/commands/test_subset.py @@ -522,3 +522,131 @@ def test_subset_with_get_tests_from_guess(self): """ payload = self.decode_request_body(self.find_request('/subset').request.body) self.assertIn([{"type": "file", "name": "tests/commands/test_subset.py"}], payload.get("testPaths", [])) + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_subset_with_bin_option(self): + pipe = "test_1.py\ntest_2.py\n" + mock_json_response = { + "testPaths": [[{"type": "file", "name": "test_1.py"}]], + "rest": [[{"type": "file", "name": "test_2.py"}]], + "subsettingId": 999, + "summary": {"subset": {"duration": 1, "candidates": 1, "rate": 50}, + "rest": {"duration": 1, "candidates": 1, "rate": 50}}, + "isObservation": False, + } + responses.replace( + responses.POST, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset", + json=mock_json_response, + status=200, + ) + + result = self.cli( + "subset", + "file", + "--session", + self.session, + "--target", + "10%", + "--bin", + "1/4", + mix_stderr=False, + input=pipe, + ) + self.assert_success(result) + + payload = self.decode_request_body(self.find_request('/subset').request.body) + self.assertEqual( + payload.get('splitSubset'), + {"sliceIndex": 1, "sliceCount": 4, "sameBins": []}, + ) + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_subset_with_same_bin_file(self): + pipe = "TestExample1\nok github.com/example/project 0.1s\n" + mock_json_response = { + "testPaths": [[ + {"type": "class", "name": "rocket-car-gotest"}, + {"type": "testcase", "name": "TestExample1"}, + ]], + "rest": [], + "subsettingId": 123, + "summary": { + "subset": {"duration": 1, "candidates": 1, "rate": 50}, + "rest": {"duration": 1, "candidates": 0, "rate": 50}, + }, + "isObservation": False, + } + responses.replace( + responses.POST, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset", + json=mock_json_response, + status=200, + ) + + same_bin_file = tempfile.NamedTemporaryFile(delete=False) + try: + same_bin_file.write(b"example.AddTest\nexample.DivTest\n") + same_bin_file.close() + + result = self.cli( + "subset", + "go-test", + "--session", + self.session, + "--target", + "20%", + "--bin", + "2/5", + "--same-bin", + same_bin_file.name, + mix_stderr=False, + input=pipe, + ) + self.assert_success(result) + + payload = self.decode_request_body(self.find_request('/subset').request.body) + split_subset = payload.get('splitSubset') + self.assertEqual(split_subset.get('sliceIndex'), 2) + self.assertEqual(split_subset.get('sliceCount'), 5) + self.assertEqual( + split_subset.get('sameBins'), + [[ + [ + {"type": "class", "name": "example"}, + {"type": "testcase", "name": "AddTest"}, + ], + [ + {"type": "class", "name": "example"}, + {"type": "testcase", "name": "DivTest"}, + ], + ]], + ) + finally: + os.unlink(same_bin_file.name) + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_same_bin_requires_bin(self): + pipe = "TestExample\nok github.com/example/project 0.1s\n" + same_bin_file = tempfile.NamedTemporaryFile(delete=False) + try: + same_bin_file.write(b"example.AddTest\n") + same_bin_file.close() + + result = self.cli( + "subset", + "go-test", + "--session", + self.session, + "--same-bin", + same_bin_file.name, + mix_stderr=False, + input=pipe, + ) + self.assert_exit_code(result, 1) + self.assertIn("--same-bin requires --bin", result.stderr) + finally: + os.unlink(same_bin_file.name) From 685907e582051b301653a0fb7aa2d8226f1349da Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 15:54:07 +0900 Subject: [PATCH 05/14] [AIENG-249] add --subset-id option to be able to reuse existing subsetting result --- smart_tests/commands/subset.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 51be08148..7eb48f28c 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -174,6 +174,12 @@ def __init__( type=fileText(mode="r"), metavar="FILE" )] = None, + subset_id: Annotated[int | None, typer.Option( + "--subset-id", + help="Reuse reorder results from an existing subset ID", + metavar="ID", + type=intType(min=1) + )] = None, bin_target: Annotated[Fraction | None, typer.Option( "--bin", help="Split subset into bins, e.g. --bin 1/4", @@ -267,6 +273,7 @@ def warn(msg: str): self.ignore_flaky_tests_above = ignore_flaky_tests_above self.prioritize_tests_failed_within_hours = prioritize_tests_failed_within_hours self.prioritized_tests_mapping_file = prioritized_tests_mapping_file + self.subset_id = subset_id self.bin_target = bin_target self.same_bin_files = list(same_bin_files) self.is_get_tests_from_guess = is_get_tests_from_guess @@ -418,6 +425,9 @@ def get_payload(self) -> dict[str, Any]: if self.use_case: payload['changesUnderTest'] = self.use_case.value + if self.subset_id is not None: + payload['subsettingId'] = self.subset_id + split_subset = self._build_split_subset_payload() if split_subset: payload['splitSubset'] = split_subset From b6b9e31f5f7d0c9cc1faf5c139af533f7ce9c4d3 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 15:59:26 +0900 Subject: [PATCH 06/14] [AIENG-294] define private method and use it --- smart_tests/commands/subset.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 7eb48f28c..f3521e62f 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -577,13 +577,20 @@ def request_subset(self) -> SubsetResult: e, "Warning: the service failed to subset. Falling back to running all tests") return SubsetResult.from_test_paths(self.test_paths) + def _requires_test_input(self) -> bool: + return ( + self.subset_id is None + and not self.is_get_tests_from_previous_sessions # noqa: W503 + and len(self.test_paths) == 0 # noqa: W503 + ) + def run(self): """called after tests are scanned to compute the optimized order""" if self.is_get_tests_from_guess: self._collect_potential_test_files() - if not self.is_get_tests_from_previous_sessions and len(self.test_paths) == 0: + if self._requires_test_input(): if self.input_given: print_error_and_die("ERROR: Given arguments did not match any tests. They appear to be incorrect/non-existent.", tracking_client, Tracking.ErrorEvent.USER_ERROR) # noqa E501 else: From ff764d120e22419987888d56ea54b3e14a73347c Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 16:44:08 +0900 Subject: [PATCH 07/14] [AIENG-294] add tests --- tests/commands/test_subset.py | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/commands/test_subset.py b/tests/commands/test_subset.py index 7127fc7b7..f3083916f 100644 --- a/tests/commands/test_subset.py +++ b/tests/commands/test_subset.py @@ -551,12 +551,15 @@ def test_subset_with_bin_option(self): "10%", "--bin", "1/4", + "--subset-id", + "222", mix_stderr=False, input=pipe, ) self.assert_success(result) payload = self.decode_request_body(self.find_request('/subset').request.body) + self.assertEqual(payload.get('subsettingId'), 222) self.assertEqual( payload.get('splitSubset'), {"sliceIndex": 1, "sliceCount": 4, "sameBins": []}, @@ -650,3 +653,42 @@ def test_same_bin_requires_bin(self): self.assertIn("--same-bin requires --bin", result.stderr) finally: os.unlink(same_bin_file.name) + + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_subset_without_tests_but_with_subset_id(self): + responses.replace( + responses.POST, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset", + json={ + "testPaths": [], + "rest": [], + "subsettingId": 321, + "summary": { + "subset": {"duration": 0, "candidates": 0, "rate": 0}, + "rest": {"duration": 0, "candidates": 0, "rate": 0}, + }, + "isObservation": False, + }, + status=200, + ) + + result = self.cli( + "subset", + "file", + "--session", + self.session, + "--subset-id", + "321", + "--bin", + "1/2", + "--target", + "10%", + mix_stderr=False, + ) + + self.assert_success(result) + self.assertNotIn("Warning: this command reads from stdin", result.stderr) + + payload = self.decode_request_body(self.find_request('/subset').request.body) + self.assertEqual(payload.get('subsettingId'), 321) From 2276a1240e34acb7310d13c927a1ecba954b3b40 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 17:35:50 +0900 Subject: [PATCH 08/14] [AIENG-294] for type checking --- 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 f3521e62f..d0ba3f5c9 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -507,6 +507,8 @@ def _read_same_bin_files(self) -> list[list[TestPath]]: ) seen_tests.add(test) + # For type check + assert formatter is not None, "--same -bin is not supported for this test runner" formatted = formatter(test) if not formatted: print_error_and_die( From 23ac0a08498b1f6642f444ecb0d3b464394abccb Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 17:38:36 +0900 Subject: [PATCH 09/14] [AIENG-294] update error message --- smart_tests/commands/subset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index d0ba3f5c9..39c618f1e 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -438,7 +438,7 @@ def _build_split_subset_payload(self) -> dict[str, Any] | None: if self.bin_target is None: if self.same_bin_files: print_error_and_die( - "--same-bin requires --bin", + "--same-bin option requires --bin option.\nPlease set --bin option to use --same-bin", self.tracking_client, Tracking.ErrorEvent.USER_ERROR, ) From b9bfe6cb1d1ae5b2d462ca77f3c88cb0c5003404 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 17:56:16 +0900 Subject: [PATCH 10/14] [AIENG-294] define private method and use it --- smart_tests/commands/subset.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 39c618f1e..81f7f441f 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -326,7 +326,7 @@ def stdin(self) -> Iterable[str]: """ # To avoid the cli continue to wait from stdin - if self.is_get_tests_from_previous_sessions or self.is_get_tests_from_guess: + if self._should_skip_stdin(): return [] if sys.stdin.isatty(): @@ -586,6 +586,22 @@ def _requires_test_input(self) -> bool: and len(self.test_paths) == 0 # noqa: W503 ) + def _should_skip_stdin(self) -> bool: + if self.is_get_tests_from_previous_sessions or self.is_get_tests_from_guess: + return True + + if self.subset_id is not None: + if self._stdin_is_tty(): + warn_and_exit_if_fail_fast_mode( + "Warning: --subset-id is set so stdin will be ignored." + ) + return True + return False + + # Note(Konboi): Helper to be able to patch stdin easily in tests + def _stdin_is_tty(self) -> bool: + return sys.stdin.isatty() + def run(self): """called after tests are scanned to compute the optimized order""" From 2d9befc701d2be8f833b2b1e3e634349bac420a4 Mon Sep 17 00:00:00 2001 From: Konboi Date: Mon, 15 Dec 2025 17:56:30 +0900 Subject: [PATCH 11/14] [AIENG-294] refactor tests of subset command --- smart_tests/commands/subset.py | 6 +- tests/commands/test_subset.py | 128 +++++++-------------------------- 2 files changed, 28 insertions(+), 106 deletions(-) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 81f7f441f..3b9d03ef2 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -591,17 +591,13 @@ def _should_skip_stdin(self) -> bool: return True if self.subset_id is not None: - if self._stdin_is_tty(): + if sys.stdin.isatty(): warn_and_exit_if_fail_fast_mode( "Warning: --subset-id is set so stdin will be ignored." ) return True return False - # Note(Konboi): Helper to be able to patch stdin easily in tests - def _stdin_is_tty(self) -> bool: - return sys.stdin.isatty() - def run(self): """called after tests are scanned to compute the optimized order""" diff --git a/tests/commands/test_subset.py b/tests/commands/test_subset.py index f3083916f..93e1a90c3 100644 --- a/tests/commands/test_subset.py +++ b/tests/commands/test_subset.py @@ -542,22 +542,10 @@ def test_subset_with_bin_option(self): status=200, ) - result = self.cli( - "subset", - "file", - "--session", - self.session, - "--target", - "10%", - "--bin", - "1/4", - "--subset-id", - "222", - mix_stderr=False, - input=pipe, - ) - self.assert_success(result) + result = self.cli("subset", "file", "--session", self.session, "--target", "10%", "--bin", "1/4", + "--subset-id", "222", mix_stderr=False, input=pipe) + self.assert_success(result) payload = self.decode_request_body(self.find_request('/subset').request.body) self.assertEqual(payload.get('subsettingId'), 222) self.assertEqual( @@ -568,7 +556,25 @@ def test_subset_with_bin_option(self): @responses.activate @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) def test_subset_with_same_bin_file(self): - pipe = "TestExample1\nok github.com/example/project 0.1s\n" + # Test invalid case + # --same-bin requires --bin options + with tempfile.NamedTemporaryFile("w+", delete=False) as same_bin_file: + same_bin_file.write("example.AddTest\n") + same_bin_file.flush() + result = self.cli( + "subset", + "go-test", + "--session", + self.session, + "--subset-id", + 123, + "--same-bin", + same_bin_file.name, + mix_stderr=False) + self.assert_exit_code(result, 1) + self.assertIn("--same-bin option requires --bin option", result.stderr) + + # Test valid case mock_json_response = { "testPaths": [[ {"type": "class", "name": "rocket-car-gotest"}, @@ -589,27 +595,12 @@ def test_subset_with_same_bin_file(self): status=200, ) - same_bin_file = tempfile.NamedTemporaryFile(delete=False) - try: - same_bin_file.write(b"example.AddTest\nexample.DivTest\n") - same_bin_file.close() - - result = self.cli( - "subset", - "go-test", - "--session", - self.session, - "--target", - "20%", - "--bin", - "2/5", - "--same-bin", - same_bin_file.name, - mix_stderr=False, - input=pipe, - ) + with tempfile.NamedTemporaryFile("w+", delete=False) as same_bin_file: + same_bin_file.write("example.AddTest\nexample.DivTest\n") + same_bin_file.flush() + result = self.cli("subset", "go-test", "--session", self.session, "--target", "20%", "--subset-id", 123, + "--bin", "2/5", "--same-bin", same_bin_file.name, mix_stderr=False) self.assert_success(result) - payload = self.decode_request_body(self.find_request('/subset').request.body) split_subset = payload.get('splitSubset') self.assertEqual(split_subset.get('sliceIndex'), 2) @@ -627,68 +618,3 @@ def test_subset_with_same_bin_file(self): ], ]], ) - finally: - os.unlink(same_bin_file.name) - - @responses.activate - @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) - def test_same_bin_requires_bin(self): - pipe = "TestExample\nok github.com/example/project 0.1s\n" - same_bin_file = tempfile.NamedTemporaryFile(delete=False) - try: - same_bin_file.write(b"example.AddTest\n") - same_bin_file.close() - - result = self.cli( - "subset", - "go-test", - "--session", - self.session, - "--same-bin", - same_bin_file.name, - mix_stderr=False, - input=pipe, - ) - self.assert_exit_code(result, 1) - self.assertIn("--same-bin requires --bin", result.stderr) - finally: - os.unlink(same_bin_file.name) - - @responses.activate - @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) - def test_subset_without_tests_but_with_subset_id(self): - responses.replace( - responses.POST, - f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset", - json={ - "testPaths": [], - "rest": [], - "subsettingId": 321, - "summary": { - "subset": {"duration": 0, "candidates": 0, "rate": 0}, - "rest": {"duration": 0, "candidates": 0, "rate": 0}, - }, - "isObservation": False, - }, - status=200, - ) - - result = self.cli( - "subset", - "file", - "--session", - self.session, - "--subset-id", - "321", - "--bin", - "1/2", - "--target", - "10%", - mix_stderr=False, - ) - - self.assert_success(result) - self.assertNotIn("Warning: this command reads from stdin", result.stderr) - - payload = self.decode_request_body(self.find_request('/subset').request.body) - self.assertEqual(payload.get('subsettingId'), 321) From f6aacd3c994312b7c5ec2c62804c02789d076037 Mon Sep 17 00:00:00 2001 From: Konboi Date: Thu, 18 Dec 2025 17:58:07 +0900 Subject: [PATCH 12/14] [AIENG-294] rename the option name from -subset-id to -input-snapshot-id --- smart_tests/commands/subset.py | 37 ++++++++++++++++++++------- tests/commands/test_subset.py | 46 +++++++++++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 3b9d03ef2..f9666b768 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -174,12 +174,16 @@ def __init__( type=fileText(mode="r"), metavar="FILE" )] = None, - subset_id: Annotated[int | None, typer.Option( - "--subset-id", - help="Reuse reorder results from an existing subset ID", + input_snapshot_id: Annotated[int | None, typer.Option( + "--input-snapshot-id", + help="Reuse reorder results from an existing input snapshot ID", metavar="ID", type=intType(min=1) )] = None, + print_input_snapshot_id: Annotated[bool, typer.Option( + "--print-input-snapshot-id", + help="Print the input snapshot ID returned from the server instead of the subset results" + )] = False, bin_target: Annotated[Fraction | None, typer.Option( "--bin", help="Split subset into bins, e.g. --bin 1/4", @@ -273,7 +277,8 @@ def warn(msg: str): self.ignore_flaky_tests_above = ignore_flaky_tests_above self.prioritize_tests_failed_within_hours = prioritize_tests_failed_within_hours self.prioritized_tests_mapping_file = prioritized_tests_mapping_file - self.subset_id = subset_id + self.input_snapshot_id = input_snapshot_id + self.print_input_snapshot_id = print_input_snapshot_id self.bin_target = bin_target self.same_bin_files = list(same_bin_files) self.is_get_tests_from_guess = is_get_tests_from_guess @@ -425,8 +430,8 @@ def get_payload(self) -> dict[str, Any]: if self.use_case: payload['changesUnderTest'] = self.use_case.value - if self.subset_id is not None: - payload['subsettingId'] = self.subset_id + if self.input_snapshot_id is not None: + payload['subsettingId'] = self.input_snapshot_id split_subset = self._build_split_subset_payload() if split_subset: @@ -581,7 +586,7 @@ def request_subset(self) -> SubsetResult: def _requires_test_input(self) -> bool: return ( - self.subset_id is None + self.input_snapshot_id is None and not self.is_get_tests_from_previous_sessions # noqa: W503 and len(self.test_paths) == 0 # noqa: W503 ) @@ -590,14 +595,22 @@ def _should_skip_stdin(self) -> bool: if self.is_get_tests_from_previous_sessions or self.is_get_tests_from_guess: return True - if self.subset_id is not None: + if self.input_snapshot_id is not None: if sys.stdin.isatty(): warn_and_exit_if_fail_fast_mode( - "Warning: --subset-id is set so stdin will be ignored." + "Warning: --input-snapshot-id is set so stdin will be ignored." ) return True return False + def _print_input_snapshot_id_value(self, subset_result: SubsetResult): + if not subset_result.subset_id: + raise click.ClickException( + "This request did not return an input snapshot ID. Please re-run the command." + ) + + click.echo(subset_result.subset_id) + def run(self): """called after tests are scanned to compute the optimized order""" @@ -623,6 +636,12 @@ def run(self): if len(subset_result.subset) == 0: warn_and_exit_if_fail_fast_mode("Error: no tests found matching the path.") + if self.print_input_snapshot_id: + self._print_input_snapshot_id_value(subset_result) + return + + if self.print_input_snapshot_id: + self._print_input_snapshot_id_value(subset_result) return # TODO(Konboi): split subset isn't provided for smart-tests initial release diff --git a/tests/commands/test_subset.py b/tests/commands/test_subset.py index 93e1a90c3..33a24f631 100644 --- a/tests/commands/test_subset.py +++ b/tests/commands/test_subset.py @@ -95,6 +95,46 @@ def test_subset(self): rest.close() os.unlink(rest.name) + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_subset_print_input_snapshot_id(self): + pipe = "test_1.py\ntest_2.py" + mock_json_response = { + "testPaths": [ + [{"type": "file", "name": "test_1.py"}], + [{"type": "file", "name": "test_2.py"}], + ], + "testRunner": "file", + "rest": [], + "subsettingId": 456, + "summary": { + "subset": {"duration": 10, "candidates": 2, "rate": 50}, + "rest": {"duration": 10, "candidates": 0, "rate": 50}, + }, + "isObservation": False, + } + responses.replace( + responses.POST, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset", + json=mock_json_response, + status=200, + ) + + result = self.cli( + "subset", + "file", + "--target", + "30%", + "--session", + self.session, + "--print-input-snapshot-id", + mix_stderr=False, + input=pipe, + ) + + self.assert_success(result) + self.assertEqual(result.stdout, "456\n") + @responses.activate @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) def test_subset_with_observation_session(self): @@ -543,7 +583,7 @@ def test_subset_with_bin_option(self): ) result = self.cli("subset", "file", "--session", self.session, "--target", "10%", "--bin", "1/4", - "--subset-id", "222", mix_stderr=False, input=pipe) + "--input-snapshot-id", "222", mix_stderr=False, input=pipe) self.assert_success(result) payload = self.decode_request_body(self.find_request('/subset').request.body) @@ -566,7 +606,7 @@ def test_subset_with_same_bin_file(self): "go-test", "--session", self.session, - "--subset-id", + "--input-snapshot-id", 123, "--same-bin", same_bin_file.name, @@ -598,7 +638,7 @@ def test_subset_with_same_bin_file(self): with tempfile.NamedTemporaryFile("w+", delete=False) as same_bin_file: same_bin_file.write("example.AddTest\nexample.DivTest\n") same_bin_file.flush() - result = self.cli("subset", "go-test", "--session", self.session, "--target", "20%", "--subset-id", 123, + result = self.cli("subset", "go-test", "--session", self.session, "--target", "20%", "--input-snapshot-id", 123, "--bin", "2/5", "--same-bin", same_bin_file.name, mix_stderr=False) self.assert_success(result) payload = self.decode_request_body(self.find_request('/subset').request.body) From ea69f9ed08198f64ff1040da8cc0b179f89f3482 Mon Sep 17 00:00:00 2001 From: Konboi Date: Thu, 18 Dec 2025 18:21:05 +0900 Subject: [PATCH 13/14] [AIENG-294] define InputSnapshotId class --- smart_tests/commands/subset.py | 14 +++----- smart_tests/utils/input_snapshot.py | 50 +++++++++++++++++++++++++++++ tests/commands/test_subset.py | 40 +++++++++++++++++++++++ 3 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 smart_tests/utils/input_snapshot.py diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index f9666b768..870d6ccb5 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -28,6 +28,7 @@ from ..utils.env_keys import REPORT_ERROR_KEY from ..utils.fail_fast_mode import (FailFastModeValidateParams, fail_fast_mode_validate, set_fail_fast_mode, warn_and_exit_if_fail_fast_mode) +from ..utils.input_snapshot import InputSnapshotId from ..utils.smart_tests_client import SmartTestsClient from ..utils.typer_types import Duration, Fraction, Percentage, parse_duration, parse_fraction, parse_percentage from .test_path_writer import TestPathWriter @@ -174,12 +175,7 @@ def __init__( type=fileText(mode="r"), metavar="FILE" )] = None, - input_snapshot_id: Annotated[int | None, typer.Option( - "--input-snapshot-id", - help="Reuse reorder results from an existing input snapshot ID", - metavar="ID", - type=intType(min=1) - )] = None, + input_snapshot_id: Annotated[InputSnapshotId | None, InputSnapshotId.as_option()] = None, print_input_snapshot_id: Annotated[bool, typer.Option( "--print-input-snapshot-id", help="Print the input snapshot ID returned from the server instead of the subset results" @@ -277,7 +273,7 @@ def warn(msg: str): self.ignore_flaky_tests_above = ignore_flaky_tests_above self.prioritize_tests_failed_within_hours = prioritize_tests_failed_within_hours self.prioritized_tests_mapping_file = prioritized_tests_mapping_file - self.input_snapshot_id = input_snapshot_id + self.input_snapshot_id = input_snapshot_id.value if input_snapshot_id else None self.print_input_snapshot_id = print_input_snapshot_id self.bin_target = bin_target self.same_bin_files = list(same_bin_files) @@ -596,7 +592,7 @@ def _should_skip_stdin(self) -> bool: return True if self.input_snapshot_id is not None: - if sys.stdin.isatty(): + if not sys.stdin.isatty(): warn_and_exit_if_fail_fast_mode( "Warning: --input-snapshot-id is set so stdin will be ignored." ) @@ -606,7 +602,7 @@ def _should_skip_stdin(self) -> bool: def _print_input_snapshot_id_value(self, subset_result: SubsetResult): if not subset_result.subset_id: raise click.ClickException( - "This request did not return an input snapshot ID. Please re-run the command." + "Subset request did not return an input snapshot ID. Please re-run the command." ) click.echo(subset_result.subset_id) diff --git a/smart_tests/utils/input_snapshot.py b/smart_tests/utils/input_snapshot.py new file mode 100644 index 000000000..5a2b19490 --- /dev/null +++ b/smart_tests/utils/input_snapshot.py @@ -0,0 +1,50 @@ +"""Utility type for --input-snapshot-id option.""" + +import click + +from smart_tests.args4p import typer + + +class InputSnapshotId: + """Parses either a numeric snapshot ID or @path reference.""" + + def __init__(self, raw: str): + value = raw + if value.startswith('@'): + file_path = value[1:] + try: + with open(file_path, 'r', encoding='utf-8') as fp: + value = fp.read().strip() + except OSError as exc: + raise click.BadParameter( + f"Failed to read input snapshot ID file '{file_path}': {exc}" + ) + + try: + parsed = int(value) + except ValueError: + raise click.BadParameter( + f"Invalid input snapshot ID '{value}'. Expected a positive integer." + ) + + if parsed < 1: + raise click.BadParameter( + "Invalid input snapshot ID. Expected a positive integer." + ) + + self.value = parsed + + def __int__(self) -> int: + return self.value + + def __str__(self) -> str: + return str(self.value) + + @staticmethod + def as_option() -> typer.Option: + return typer.Option( + "--input-snapshot-id", + help="Reuse reorder results from an existing input snapshot ID or specify @path/to/file to load it", + metavar="ID|@FILE", + type=InputSnapshotId, + ) diff --git a/tests/commands/test_subset.py b/tests/commands/test_subset.py index 33a24f631..25ec762d7 100644 --- a/tests/commands/test_subset.py +++ b/tests/commands/test_subset.py @@ -593,6 +593,46 @@ def test_subset_with_bin_option(self): {"sliceIndex": 1, "sliceCount": 4, "sameBins": []}, ) + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_subset_input_snapshot_id_from_file(self): + pipe = "test_1.py\ntest_2.py\n" + mock_json_response = { + "testPaths": [[{"type": "file", "name": "test_1.py"}]], + "rest": [[{"type": "file", "name": "test_2.py"}]], + "subsettingId": 999, + "summary": {"subset": {"duration": 1, "candidates": 1, "rate": 50}, + "rest": {"duration": 1, "candidates": 1, "rate": 50}}, + "isObservation": False, + } + responses.replace( + responses.POST, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset", + json=mock_json_response, + status=200, + ) + + with tempfile.NamedTemporaryFile("w+", delete=False) as snapshot_file: + snapshot_file.write("777\n") + snapshot_file.flush() + result = self.cli( + "subset", + "file", + "--session", + self.session, + "--target", + "10%", + "--input-snapshot-id", + f"@{snapshot_file.name}", + mix_stderr=False, + input=pipe, + ) + os.unlink(snapshot_file.name) + + self.assert_success(result) + payload = self.decode_request_body(self.find_request('/subset').request.body) + self.assertEqual(payload.get('subsettingId'), 777) + @responses.activate @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) def test_subset_with_same_bin_file(self): From f59bf30cd5c600e13d35c6f0a46a2170b180c136 Mon Sep 17 00:00:00 2001 From: Konboi Date: Fri, 19 Dec 2025 11:10:29 +0900 Subject: [PATCH 14/14] [AIENG-294] add validation --- smart_tests/commands/subset.py | 37 ++++++++++++++++++++++++++++++++++ tests/commands/test_subset.py | 20 ++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/smart_tests/commands/subset.py b/smart_tests/commands/subset.py index 870d6ccb5..885dcc66b 100644 --- a/smart_tests/commands/subset.py +++ b/smart_tests/commands/subset.py @@ -280,6 +280,8 @@ def warn(msg: str): self.is_get_tests_from_guess = is_get_tests_from_guess self.use_case = use_case + self._validate_print_input_snapshot_option() + self.file_path_normalizer = FilePathNormalizer(base_path, no_base_path_inference=no_base_path_inference) self.test_paths: list[list[dict[str, str]]] = [] @@ -599,6 +601,41 @@ def _should_skip_stdin(self) -> bool: return True return False + def _validate_print_input_snapshot_option(self): + if not self.print_input_snapshot_id: + return + + conflicts: list[str] = [] + option_checks = [ + ("--target", self.target is not None), + ("--time", self.time is not None), + ("--confidence", self.confidence is not None), + ("--goal-spec", self.goal_spec is not None), + ("--rest", self.rest is not None), + ("--bin", self.bin_target is not None), + ("--same-bin", bool(self.same_bin_files)), + ("--ignore-new-tests", self.ignore_new_tests), + ("--ignore-flaky-tests-above", self.ignore_flaky_tests_above is not None), + ("--prioritize-tests-failed-within-hours", self.prioritize_tests_failed_within_hours is not None), + ("--prioritized-tests-mapping", self.prioritized_tests_mapping_file is not None), + ("--get-tests-from-previous-sessions", self.is_get_tests_from_previous_sessions), + ("--get-tests-from-guess", self.is_get_tests_from_guess), + ("--output-exclusion-rules", self.is_output_exclusion_rules), + ("--non-blocking", self.is_non_blocking), + ] + + for option_name, is_set in option_checks: + if is_set: + conflicts.append(option_name) + + if conflicts: + conflict_list = ", ".join(conflicts) + print_error_and_die( + f"--print-input-snapshot-id cannot be used with {conflict_list}.", + self.tracking_client, + Tracking.ErrorEvent.USER_ERROR, + ) + def _print_input_snapshot_id_value(self, subset_result: SubsetResult): if not subset_result.subset_id: raise click.ClickException( diff --git a/tests/commands/test_subset.py b/tests/commands/test_subset.py index 25ec762d7..53e753005 100644 --- a/tests/commands/test_subset.py +++ b/tests/commands/test_subset.py @@ -135,6 +135,26 @@ def test_subset_print_input_snapshot_id(self): self.assert_success(result) self.assertEqual(result.stdout, "456\n") + @responses.activate + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + def test_subset_print_input_snapshot_id_disallows_subset_options(self): + pipe = "test_1.py\n" + + result = self.cli( + "subset", + "file", + "--target", + "30%", + "--session", + self.session, + "--print-input-snapshot-id", + mix_stderr=False, + input=pipe, + ) + + self.assert_exit_code(result, 1) + self.assertIn("--print-input-snapshot-id cannot be used with --target", result.stderr) + @responses.activate @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) def test_subset_with_observation_session(self):