diff --git a/smart_tests/commands/compare/subsets.py b/smart_tests/commands/compare/subsets.py index b42e199d9..0a9f65359 100644 --- a/smart_tests/commands/compare/subsets.py +++ b/smart_tests/commands/compare/subsets.py @@ -1,3 +1,6 @@ +import re +import shutil +import textwrap from dataclasses import dataclass from http import HTTPStatus from pathlib import Path @@ -192,13 +195,54 @@ def _from_subset_ids(client: SmartTestsClient, subset_id_before: int, subset_id_ # Display results in a tabular format headers = ["Δ Rank", "Subset Rank", "Test Name", "Reason", "Density"] + column_width = get_column_width() tabular_data = [ - (rank, after, test_name, reason, density) + (rank, after, wrap_data(test_name, width=column_width), wrap_data(reason, width=column_width), density) for rank, after, test_name, reason, density in rows ] click.echo_via_pager(summary + "\n" + tabulate(tabular_data, headers=headers, tablefmt="simple")) +def wrap_data(data: str, width: int = 30) -> str: + + if not data: + return data + + # Add space after / and \ to allow wrapping at these points + formatted_data = re.sub(r'([/\\])', r'\1 ', data) + wrapped = textwrap.fill(formatted_data, width=width) + # Remove the added spaces + wrapped = wrapped.replace("/ ", "/").replace("\\ ", "\\") + return wrapped + + +def get_column_width() -> int: + + try: + # Get terminal size, fallback to 80x24 if not detectable + terminal_size = shutil.get_terminal_size(fallback=(80, 24)) + terminal_width = terminal_size.columns + + # Estimate space needed for fixed columns and table formatting: + # - "Δ Rank" column: ~10 chars + # - "Subset Rank" column: ~12 chars + # - "Density" column: ~8 chars + # - Table separators and padding: ~6 chars + fixed_width = 36 + + # Calculate available width for the two wrappable columns + available_width = terminal_width - fixed_width + + # Split equally between Test Name and Reason columns + column_width = available_width // 2 + + # Ensure minimum width of 30 characters + return max(30, column_width) + except Exception: + # If anything goes wrong, fall back to default + return 30 + + def _from_files(file_before: Path, file_after: Path): before_subset = SubsetResultBases.from_file(file_before) after_subset = SubsetResultBases.from_file(file_after) diff --git a/tests/commands/compare/test_subsets.py b/tests/commands/compare/test_subsets.py index 7f0d38a0e..e8f022181 100644 --- a/tests/commands/compare/test_subsets.py +++ b/tests/commands/compare/test_subsets.py @@ -214,3 +214,136 @@ def test_subsets_subset_ids(self): ]: for cell in expected_row: self.assertIn(cell, output) + + def test_wrap_data(self): + """Test that wrap_data breaks paths at separators.""" + from smart_tests.commands.compare.subsets import wrap_data + + # Short path - no wrapping needed + short = "Changed file: aaa.py" + self.assertEqual(wrap_data(short, width=30), short) + + # Empty string + self.assertEqual(wrap_data("", width=30), "") + + # Long path - should wrap at / + long_path = "Changed file: src/mongo/db/telemetry/telemetry_thread_base.cpp" + wrapped = wrap_data(long_path, width=30) + + # Verify it contains newlines + self.assertIn("\n", wrapped) + + # Verify no unwanted spaces around path separators + self.assertNotIn(" /", wrapped) + + # Verify content is preserved (just reformatted) + unwrapped = wrapped.replace("\n", "") + self.assertEqual(unwrapped, long_path) + + # Windows path test + windows_path = "Changed file: C:\\Users\\test\\very\\long\\windows\\path\\file.cpp" + wrapped_win = wrap_data(windows_path, width=30) + self.assertIn("\n", wrapped_win) + self.assertNotIn(" \\", wrapped_win) + + # Verify content preservation + self.assertEqual(wrapped_win.replace("\n", ""), windows_path) + + def test_get_column_width(self): + """Test that get_column_width() calculates widths based on terminal size.""" + from smart_tests.commands.compare.subsets import get_column_width + + # Test with a wide terminal (e.g., 160 columns) + # Available width = 160 - 36 (fixed columns) = 124 + # Per column = 124 / 2 = 62 + with mock.patch("shutil.get_terminal_size") as mock_terminal: + mock_terminal.return_value = os.terminal_size((160, 24)) + width = get_column_width() + self.assertEqual(width, 62) + + # Test with standard terminal (80 columns) + # Available width = 80 - 36 = 44 + # Per column = 44 / 2 = 22, but minimum is 30 + with mock.patch("shutil.get_terminal_size") as mock_terminal: + mock_terminal.return_value = os.terminal_size((80, 24)) + width = get_column_width() + self.assertEqual(width, 30) # Should hit minimum + + # Test with narrow terminal (60 columns) + # Should still return minimum of 30 + with mock.patch("shutil.get_terminal_size") as mock_terminal: + mock_terminal.return_value = os.terminal_size((60, 24)) + width = get_column_width() + self.assertEqual(width, 30) # Should hit minimum + + # Test with very wide terminal (200 columns) + # Available width = 200 - 36 = 164 + # Per column = 164 / 2 = 82 + with mock.patch("shutil.get_terminal_size") as mock_terminal: + mock_terminal.return_value = os.terminal_size((200, 24)) + width = get_column_width() + self.assertEqual(width, 82) + + # Test fallback when get_terminal_size raises exception + with mock.patch("shutil.get_terminal_size") as mock_terminal: + mock_terminal.side_effect = Exception("Terminal size unavailable") + width = get_column_width() + self.assertEqual(width, 30) # Should fall back to default + + @mock.patch.dict(os.environ, {"SMART_TESTS_TOKEN": CliTestCase.smart_tests_token}) + @responses.activate + def test_subsets_with_long_paths_wrapped(self): + """Test that long file paths in Reason column are wrapped properly.""" + long_path1 = "src/mongo/db/telemetry/telemetry_thread_base.cpp" + long_path2 = "jstests/concurrency/fsm_workloads/timeseries/timeseries_raw_data_operations.js" + + responses.add( + responses.GET, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset/200", + json={ + "subsetting": {"id": 200}, + "testPaths": [ + {"testPath": [{"type": "file", "name": long_path1}], + "duration": 10, "density": 0.9, + "reason": f"Changed file: {long_path1}"} + ], + "rest": [] + }, + status=200 + ) + + responses.add( + responses.GET, + f"{get_base_url()}/intake/organizations/{self.organization}/workspaces/{self.workspace}/subset/201", + json={ + "subsetting": {"id": 201}, + "testPaths": [ + {"testPath": [{"type": "file", "name": long_path2}], + "duration": 10, "density": 0.85, + "reason": f"Changed file: {long_path2}"} + ], + "rest": [] + }, + status=200 + ) + + result = self.cli('compare', 'subsets', + '--subset-id-before', '200', + '--subset-id-after', '201', + mix_stderr=False) + + self.assert_success(result) + + # Verify output contains the summary + self.assertIn("PTS subset change summary:", result.stdout) + self.assertIn("Changed file:", result.stdout) + + # Verify the long paths are present (even if wrapped) + self.assertIn("telemetry_thread_base.cpp", result.stdout) + self.assertIn("timeseries_raw_data_operations.js", result.stdout) + + # Verify no line has excessive length (wrapped properly) + for line in result.stdout.split('\n'): + # Allow some buffer for separator lines + if line.strip() and not line.startswith('─'): + self.assertLessEqual(len(line), 150)