chore(version-scanner): configure GHA to use targets file for multi-version scanning

.github/workflows/version_scanner.yml

@@ -35,7 +35,7 @@ jobs:
           # Uses -o to output a detailed, raw CSV to a file
           # Uses --stdout to print a slim, easier to parse summary to the GitHub Actions UI
           # Uses --soft-fail to temporarily limit causing CI/CD failures during the migration to full operation. 
-          python scripts/version_scanner/version_scanner.py -d python -v 3.7 --stdout -o version_scanner_output.csv --soft-fail
+          python scripts/version_scanner/version_scanner.py --matrix-file scripts/version_scanner/matrix.yaml --package-file scripts/version_scanner/example-list-non-generated-packages.txt --stdout -o version_scanner_output.csv --soft-fail
 
       - name: Upload CSV Results
         if: always()

scripts/version_scanner/example-list-non-generated-packages.txt

@@ -0,0 +1,31 @@
+packages/bigframes
+packages/bigquery-magics
+packages/db-dtypes
+packages/django-google-spanner
+packages/gapic-generator
+packages/google-api-core
+# packages/google-api-python-client # non-monorepo, ignore for now.
+packages/google-auth
+packages/google-auth-httplib2
+packages/google-auth-oauthlib
+packages/google-cloud-bigquery
+packages/pandas-gbq
+packages/google-cloud-bigtable
+packages/google-cloud-core
+packages/google-crc32c
+packages/google-cloud-datastore
+packages/google-cloud-dns
+packages/google-cloud-documentai-toolbox
+packages/google-cloud-error-reporting
+packages/google-cloud-firestore
+packages/google-cloud-logging
+packages/google-cloud-ndb
+packages/google-cloud-pubsub
+packages/google-cloud-runtimeconfig
+packages/google-cloud-spanner
+packages/google-cloud-storage
+packages/google-cloud-testutils
+packages/google-resumable-media
+packages/proto-plus
+packages/sqlalchemy-bigquery
+packages/sqlalchemy-spanner

scripts/version_scanner/matrix.yaml

@@ -0,0 +1,4 @@
+python:
+  - "3.7"
+  - "3.8"
+  - "3.9"

scripts/version_scanner/tests/unit/test_version_scanner.py

@@ -59,6 +59,8 @@ def sample_match():
         (PermissionError(), False, False, False, "Warning: Permission denied reading test_desc", None),  # Optional PermissionError
         (IOError("disk full"), True, False, True, "Error reading test_desc", None),  # Required IOError
         (IOError("disk full"), False, False, False, "Warning: Error reading test_desc", None),  # Optional IOError
+        (ValueError("invalid bytes"), True, False, True, "Error reading test_desc", None),  # Required ValueError
+        (ValueError("invalid bytes"), False, False, False, "Warning: Error reading test_desc", None),  # Optional ValueError
     ]
 )
 def test_safe_read_file_scenarios(
@@ -782,16 +784,16 @@ def test_format_for_console(sample_match):
     assert "python_requires = " not in log_str  # Slim format doesn't print context line
 
 
-def test_parse_targets_file(tmp_path):
-    from version_scanner import parse_targets_file
-    yaml_file = tmp_path / "targets.yaml"
+def test_parse_matrix_file(tmp_path):
+    from version_scanner import parse_matrix_file
+    yaml_file = tmp_path / "matrix.yaml"
     yaml_file.write_text("""
 python:
   - "3.7"
   - "3.8"
 protobuf: "4.25.8"
 """)
-    targets = parse_targets_file(str(yaml_file))
+    targets = parse_matrix_file(str(yaml_file))
     assert targets == [("python", "3.7"), ("python", "3.8"), ("protobuf", "4.25.8")]
 
 @pytest.mark.parametrize(
@@ -801,20 +803,22 @@ def test_parse_targets_file(tmp_path):
         ("invalid: {", True),           # Invalid YAML
         ("- not_a_mapping", True),      # Invalid structure (list instead of map)
         ("python:\n  - null", True),    # Invalid version type (null/None value)
+        ("python:\n  - 3.10", True),    # Invalid version type (float instead of string in list)
+        ("python: 3.10", True),         # Invalid version type (float instead of string)
     ]
 )
-def test_parse_targets_file_failures(tmp_path, file_content, file_exists):
-    from version_scanner import parse_targets_file
+def test_parse_matrix_file_failures(tmp_path, file_content, file_exists):
+    from version_scanner import parse_matrix_file
 
     if file_exists:
-        yaml_file = tmp_path / "targets_failures.yaml"
+        yaml_file = tmp_path / "matrix_failures.yaml"
         yaml_file.write_text(file_content)
         path = str(yaml_file)
     else:
         path = "nonexistent_file.yaml"
 
     with pytest.raises(SystemExit) as excinfo:
-        parse_targets_file(path)
+        parse_matrix_file(path)
     assert excinfo.value.code == 1
 
 def test_scan_repository_multi_targets(tmp_path):
@@ -868,3 +872,26 @@ def test_scan_repository_multi_targets(tmp_path):
     assert protobuf_match[0]["version"] == "4.25.8"
     assert protobuf_match[0]["rule_name"] == "protobuf_check"
 
+
+@pytest.mark.parametrize(
+    "args, expected_error_msg",
+    [
+        # Mixing -m/--matrix-file with -d or -v
+        (['version_scanner.py', '-m', 'matrix.yaml', '-d', 'python'], "Cannot specify -d/--dependency or -v/--version when using -m/--matrix-file"),
+        (['version_scanner.py', '-m', 'matrix.yaml', '-v', '3.7'], "Cannot specify -d/--dependency or -v/--version when using -m/--matrix-file"),
+        (['version_scanner.py', '-m', 'matrix.yaml', '-d', 'python', '-v', '3.7'], "Cannot specify -d/--dependency or -v/--version when using -m/--matrix-file"),
+        # Missing either -d or -v when not using -m
+        (['version_scanner.py', '-d', 'python'], "Must specify both -d/--dependency and -v/--version when not using -m/--matrix-file"),
+        (['version_scanner.py', '-v', '3.7'], "Must specify both -d/--dependency and -v/--version when not using -m/--matrix-file"),
+        (['version_scanner.py'], "Must specify both -d/--dependency and -v/--version when not using -m/--matrix-file"),
+    ]
+)
+def test_main_cli_validation(capsys, args, expected_error_msg):
+    from version_scanner import main
+    with mock.patch('sys.argv', args):
+        with pytest.raises(SystemExit) as excinfo:
+            main()
+        assert excinfo.value.code == 2
+        captured = capsys.readouterr()
+        assert expected_error_msg in captured.err
+

scripts/version_scanner/version_scanner.py

@@ -65,7 +65,7 @@ def _safe_read_file(
         else:
             print(f"Warning: Permission denied reading {description}: {file_path}", file=sys.stderr)
             return None
-    except IOError as e:
+    except (IOError, ValueError) as e:
         if required:
             print(f"Error reading {description} {file_path}: {e}", file=sys.stderr)
             sys.exit(1)
@@ -624,33 +624,36 @@ def scan_repository(
     return results
 
 
-def parse_targets_file(file_path: str) -> List[Tuple[str, str]]:
+def parse_matrix_file(file_path: str) -> List[Tuple[str, str]]:
     """
-    Parses a YAML targets file into a list of (dependency, version) tuples.
+    Parses a YAML matrix file into a list of (dependency, version) tuples.
     """
-    content = _safe_read_file(file_path, required=True, description="targets file")
+    content = _safe_read_file(file_path, required=True, description="matrix file")
     try:
-        raw_targets = yaml.safe_load(content)
+        raw_matrix = yaml.safe_load(content)
     except Exception as e:
-        print(f"Error parsing targets YAML mapping: {e}", file=sys.stderr)
+        print(f"Error parsing matrix YAML mapping: {e}", file=sys.stderr)
         sys.exit(1)
 
-    if not isinstance(raw_targets, dict):
-        print("Error: Targets file content must resolve to a YAML mapping", file=sys.stderr)
+    if not isinstance(raw_matrix, dict):
+        print("Error: Matrix file content must resolve to a YAML mapping", file=sys.stderr)
         sys.exit(1)
 
     targets = []
-    for dep, versions in raw_targets.items():
+    for dep, versions in raw_matrix.items():
         if isinstance(versions, list):
             for v in versions:
                 if v is None or isinstance(v, (dict, list)):
                     print(f"Error: Invalid version '{v}' for dependency '{dep}'", file=sys.stderr)
                     sys.exit(1)
-                targets.append((str(dep), str(v)))
-        elif versions is not None and not isinstance(versions, dict):
-            targets.append((str(dep), str(versions)))
+                if not isinstance(v, str):
+                    print(f"Error: Version '{v}' for dependency '{dep}' must be specified as a quoted string to prevent YAML parsing issues (e.g., 3.10 parsed as 3.1).", file=sys.stderr)
+                    sys.exit(1)
+                targets.append((str(dep), v))
+        elif isinstance(versions, str):
+            targets.append((str(dep), versions))
         else:
-            print(f"Error: Invalid version '{versions}' for dependency '{dep}'", file=sys.stderr)
+            print(f"Error: Invalid version '{versions}' for dependency '{dep}'. Versions must be specified as quoted strings.", file=sys.stderr)
             sys.exit(1)
 
     return targets
@@ -675,7 +678,7 @@ def main():
     )
 
     parser.add_argument(
-        "--targets-file",
+        "-m", "--matrix-file",
         help="Path to a YAML file containing target dependencies and versions."
     )
 
@@ -743,17 +746,17 @@ def main():
     args = parser.parse_args()
 
     # Validation of required inputs
-    has_single_target = bool(args.dependency and args.version)
-    has_targets_file = bool(args.targets_file)
-
-    if not (has_single_target or has_targets_file):
-        parser.error("Must specify either (-d/--dependency AND -v/--version) OR (--targets-file)")
-    if has_single_target and has_targets_file:
-        parser.error("Cannot specify both single target (-d/-v) and targets file (--targets-file)")
+    has_matrix_file = bool(args.matrix_file)
+    if has_matrix_file:
+        if args.dependency or args.version:
+            parser.error("Cannot specify -d/--dependency or -v/--version when using -m/--matrix-file")
+    else:
+        if not (args.dependency and args.version):
+            parser.error("Must specify both -d/--dependency and -v/--version when not using -m/--matrix-file")
 
     targets = []
-    if has_targets_file:
-        targets = parse_targets_file(args.targets_file)
+    if has_matrix_file:
+        targets = parse_matrix_file(args.matrix_file)
     else:
         targets = [(args.dependency, args.version)]
 
@@ -772,7 +775,7 @@ def main():
     elif args.package_file:
         target_packages = read_package_file(args.package_file)
 
-    if has_targets_file:
+    if has_matrix_file:
         print("Starting scan for multiple targets:")
         for dep, ver in targets:
             print(f"  - {dep}: {ver}")
@@ -809,7 +812,7 @@ def main():
         rules,
         target_packages,
         ignore_dirs,
-        version_string=(None if has_targets_file else args.version),
+        version_string=(None if has_matrix_file else args.version),
         targets=targets
     )
 
@@ -833,8 +836,8 @@ def main():
         script_dir = os.path.dirname(os.path.abspath(__file__))
         results_dir = os.path.join(script_dir, "results")
         os.makedirs(results_dir, exist_ok=True)
-        if has_targets_file:
-            base_name = os.path.splitext(os.path.basename(args.targets_file))[0]
+        if has_matrix_file:
+            base_name = os.path.splitext(os.path.basename(args.matrix_file))[0]
             output_path = os.path.join(results_dir, f"{base_name}-{timestamp}.csv")
         else:
             output_path = os.path.join(results_dir, f"{args.dependency}-{args.version}-{timestamp}.csv")