feat(scanner): add --soft-fail CLI flag and integrate in GHA workflow

Author: chalmerlowe

.github/workflows/version_scanner.yml

@@ -33,7 +33,7 @@ jobs:
       - name: Run Version Scanner
         run: |
           # Use -o to output the raw CSV to a file, and --stdout to print the summary to the GitHub Actions UI
-          python scripts/version_scanner/version_scanner.py -d python -v 3.7 --stdout -o version_scanner_output.csv
+          python scripts/version_scanner/version_scanner.py -d python -v 3.7 --stdout -o version_scanner_output.csv --soft-fail
           
       - name: Upload CSV Results
         if: always()

scripts/version_scanner/tests/unit/test_version_scanner.py

@@ -19,7 +19,17 @@
 from unittest.mock import patch
 import pytest
 import yaml
-from version_scanner import ConfigManager, scan_file, write_csv_report
+from version_scanner import (
+    ConfigManager, 
+    scan_file, 
+    write_csv_report,
+    _truncate_context,
+    _wrap_sheet_hyperlink,
+    _wrap_sheet_string,
+    format_for_raw_csv,
+    format_for_spreadsheet,
+    format_for_console
+)
 
 # Test ConfigManager
 @pytest.mark.parametrize("dependency, version, expected", [
@@ -156,7 +166,7 @@ def test_write_csv_report(tmp_path):
     assert rows[0]["file_path"] == "./setup.py"
     assert rows[0]["rule_name"] == "python_requires_check"
     assert rows[0]["line_number"] == "1"
-    assert rows[0]["matched_string"] == '="python_requires = \'>=3.7\'"'
+    assert rows[0]["matched_string"] == "python_requires = '>=3.7'"
     assert rows[0]["context_line"] == "python_requires = '>=3.7'"
 
 
@@ -246,43 +256,7 @@ def test_main_package_file_not_found(capsys):
     assert excinfo.value.code == 1
     captured = capsys.readouterr()
     assert "Error: Package file not found" in captured.err
-def test_format_match_for_csv():
-    from version_scanner import format_match_for_csv
-    match = {
-        "file_path": "google-cloud-python/main/packages/pkg_a/setup.py",
-        "repo_path": "packages/pkg_a/setup.py",
-        "line_number": 123,
-        "rule_name": "test_rule"
-    }
-    
-    # Test without github_repo
-    formatted = format_match_for_csv(match)
-    assert formatted["line_number"] == 123
-    
-    # Test with github_repo
-    formatted = format_match_for_csv(match, github_repo="https://github.com/user/repo", branch="main")
-    expected_url = "https://github.com/user/repo/blob/main/packages/pkg_a/setup.py#L123"
-    assert formatted["line_number"] == f'=HYPERLINK("{expected_url}", "123")'
-
 
-def test_format_match_for_csv_truncates_long_line():
-    from version_scanner import format_match_for_csv
-    
-    long_line = "a" * 1000 + "PY37" + "b" * 1000
-    match = {
-        "file_path": "test.py",
-        "line_number": 1,
-        "rule_name": "test_rule",
-        "matched_string": "PY37",
-        "context_line": long_line
-    }
-    
-    formatted = format_match_for_csv(match)
-    context = formatted["context_line"]
-    
-    assert len(context) <= 600
-    assert "PY37" in context
-    assert "..." in context
 
 
 def test_get_match_counts():
@@ -315,30 +289,7 @@ def test_scan_file_removes_newline_from_match(tmp_path):
     assert "\n" not in results[0]["matched_string"]
 
 
-def test_write_csv_report_with_links(tmp_path):
-    output_file = tmp_path / "report.csv"
-    matches = [
-        {
-            "file_path": "google-cloud-python/main/packages/pkg_a/setup.py",
-            "repo_path": "packages/pkg_a/setup.py",
-            "line_number": 1,
-            "rule_name": "python_requires_check",
-            "matched_string": "python_requires = '>=3.7'",
-            "context_line": "python_requires = '>=3.7'"
-        }
-    ]
-    
-    from version_scanner import write_csv_report
-    write_csv_report(str(output_file), matches, github_repo="https://github.com/user/repo", branch="main")
-    
-    assert output_file.exists()
-    
-    with open(output_file, 'r', encoding='utf-8', newline='') as f:
-        reader = csv.DictReader(f)
-        rows = list(reader)
-        
-    assert len(rows) == 1
-    assert "HYPERLINK" in rows[0]["line_number"]
+
 def test_scan_repository_ignores_version_scanner(tmp_path):
     vs_dir = tmp_path / "version_scanner"
     vs_dir.mkdir()
@@ -500,6 +451,18 @@ def test_main_exit_code_1():
                 main()
             assert excinfo.value.code == 1
 
+
+def test_main_soft_fail_exit_code_0():
+    """Test that main() calls sys.exit(0) when matches are found but --soft-fail is set."""
+    test_args = ['version_scanner.py', '-d', 'python', '-v', '3.7', '--soft-fail']
+    with mock.patch('sys.argv', test_args):
+        from version_scanner import main
+        with mock.patch('version_scanner.scan_repository', return_value=[{'file_path': 'test', 'line_number': 1, 'matched_string': '3.7', 'rule_name': 'test'}]):
+            with pytest.raises(SystemExit) as excinfo:
+                main()
+            assert excinfo.value.code == 0
+
+
 def test_main_stdout(capsys):
     """Test that --stdout prints the CSV output to stdout."""
     test_args = ['version_scanner.py', '-d', 'python', '-v', '3.7', '--stdout']
@@ -510,10 +473,8 @@ def test_main_stdout(capsys):
                 main()
 
     captured = capsys.readouterr()
-    assert "=== CSV Output ===" in captured.out
-    assert "test.py," in captured.out
-    assert "test," in captured.out
-    assert '"=""3.7"""' in captured.out
+    assert "=== Scan Results ===" in captured.out
+    assert "test.py:1 [test] 3.7" in captured.out
 
 def test_scan_file_truncation_bug(tmp_path):
     """Test that searching for 3.1 does NOT match 3.10 (truncation bug)."""
@@ -579,3 +540,90 @@ def test_scan_repository_package_name_roots(tmp_path):
     assert len(results) == 1
     assert results[0]["package_name"] == "pkg_third"
     assert "third_party/pkg_third/setup.py" in results[0]["file_path"]
+
+
+# --- Decoupled Formatters Tests (TDD) ---
+
+def test_truncate_context():
+    # Context shorter than 500 characters shouldn't be truncated
+    assert _truncate_context("short context", "short") == "short context"
+    
+    # Context longer than 500 characters should be truncated around the matched string
+    matched = "TARGET_VERSION"
+    long_prefix = "a" * 300
+    long_suffix = "b" * 300
+    long_context = long_prefix + matched + long_suffix
+    
+    truncated = _truncate_context(long_context, matched)
+    assert len(truncated) <= 500
+    assert matched in truncated
+    assert truncated.startswith("...")
+    assert truncated.endswith("...")
+
+def test_wrap_sheet_hyperlink():
+    assert _wrap_sheet_hyperlink("https://github.com/foo", "12") == '=HYPERLINK("https://github.com/foo", "12")'
+
+def test_wrap_sheet_string():
+    assert _wrap_sheet_string("3.10") == '="3.10"'
+    assert _wrap_sheet_string("") == ""
+    assert _wrap_sheet_string(None) == ""
+
+def test_format_for_raw_csv():
+    match = {
+        "file_path": "google-cloud-python/main/packages/pkg_a/setup.py",
+        "repo_path": "packages/pkg_a/setup.py",
+        "package_name": "pkg_a",
+        "rule_name": "python_requires_check",
+        "line_number": "123",
+        "matched_string": "3.7",
+        "context_line": "python_requires = '>=3.7'"
+    }
+    
+    formatted = format_for_raw_csv(match)
+    
+    assert formatted["file_path"] == "google-cloud-python/main/packages/pkg_a/setup.py"
+    assert formatted["package_name"] == "pkg_a"
+    assert formatted["rule_name"] == "python_requires_check"
+    assert formatted["line_number"] == 123  # Int conversion
+    assert formatted["matched_string"] == "3.7"  # No formula wrapping
+    assert formatted["context_line"] == "python_requires = '>=3.7'"
+
+def test_format_for_spreadsheet():
+    match = {
+        "file_path": "google-cloud-python/main/packages/pkg_a/setup.py",
+        "repo_path": "packages/pkg_a/setup.py",
+        "package_name": "pkg_a",
+        "rule_name": "python_requires_check",
+        "line_number": 123,
+        "matched_string": "3.7",
+        "context_line": "python_requires = '>=3.7'"
+    }
+    
+    # Without github_repo
+    formatted_no_repo = format_for_spreadsheet(match)
+    assert formatted_no_repo["line_number"] == 123
+    assert formatted_no_repo["matched_string"] == '="3.7"'  # Decimal protection formula
+    
+    # With github_repo
+    formatted_repo = format_for_spreadsheet(match, github_repo="https://github.com/user/repo", branch="main")
+    expected_url = "https://github.com/user/repo/blob/main/packages/pkg_a/setup.py#L123"
+    assert formatted_repo["line_number"] == f'=HYPERLINK("{expected_url}", "123")'
+    assert formatted_repo["matched_string"] == '="3.7"'
+
+def test_format_for_console():
+    match = {
+        "file_path": "google-cloud-python/main/packages/pkg_a/setup.py",
+        "repo_path": "packages/pkg_a/setup.py",
+        "package_name": "pkg_a",
+        "rule_name": "python_requires_check",
+        "line_number": 123,
+        "matched_string": "3.7",
+        "context_line": "python_requires = '>=3.7'"
+    }
+    
+    log_str = format_for_console(match)
+    assert "google-cloud-python/main/packages/pkg_a/setup.py:123" in log_str
+    assert "[python_requires_check]" in log_str
+    assert "3.7" in log_str
+    assert "python_requires = " not in log_str  # Slim format doesn't print context line
+