Improve sync-back automation with automatic action detection, comment preservation, and tests

Co-authored-by: henrymercer <14129055+henrymercer@users.noreply.github.com>

pr-checks/.gitignore

@@ -1 +1,3 @@
 env
+__pycache__/
+*.pyc

pr-checks/readme.md

@@ -18,21 +18,32 @@ Manually run each step in the `justfile`.
 When Dependabot updates action versions in the generated workflow files (`.github/workflows/__*.yml`), 
 the sync-back automation ensures those changes are properly reflected in the source templates.
 
+The sync-back script automatically detects all actions used in generated workflows and preserves
+version comments (e.g., `# v1.2.3`) when syncing versions between files.
+
 ### Running sync-back manually
 
 To sync action versions from generated workflows back to source templates:
 
 ```bash
 # Dry run to see what would be changed
-./pr-checks/sync-back.sh --dry-run --verbose
+python3 pr-checks/sync-back.py --dry-run --verbose
 
 # Actually apply the changes
-./pr-checks/sync-back.sh
+python3 pr-checks/sync-back.py
 ```
 
-The sync-back script (`sync-back.py`) automatically updates:
+The sync-back script automatically updates:
 - Hardcoded action versions in `pr-checks/sync.py`
 - Action version references in template files in `pr-checks/checks/`
 - Action version references in regular workflow files
 
 This ensures that the `verify-pr-checks.sh` test always passes after Dependabot PRs.
+
+### Testing
+
+The sync-back script includes comprehensive tests that can be run with:
+
+```bash
+python3 pr-checks/test_sync_back.py -v
+```

pr-checks/sync-back.py

@@ -4,11 +4,14 @@ Sync-back script to automatically update action versions in source templates
 from the generated workflow files after Dependabot updates.
 
 This script scans the generated workflow files (.github/workflows/__*.yml) to find
-the latest action versions used, then updates:
+all external action versions used, then updates:
 1. Hardcoded action versions in pr-checks/sync.py
 2. Action version references in template files in pr-checks/checks/
 3. Action version references in regular workflow files
 
+The script automatically detects all actions used in generated workflows and
+preserves version comments (e.g., # v1.2.3) when syncing versions.
+
 This ensures that when Dependabot updates action versions in generated workflows,
 those changes are properly synced back to the source templates.
 """
@@ -30,31 +33,25 @@ def scan_generated_workflows(workflow_dir: str) -> Dict[str, str]:
         workflow_dir: Path to .github/workflows directory
         
     Returns:
-        Dictionary mapping action names to their latest versions
+        Dictionary mapping action names to their latest versions (including comments)
     """
     action_versions = {}
     generated_files = glob.glob(os.path.join(workflow_dir, "__*.yml"))
     
-    # Actions we care about syncing
-    target_actions = {
-        'actions/setup-go',
-        'actions/setup-node', 
-        'actions/setup-python',
-        'actions/github-script'
-    }
-    
     for file_path in generated_files:
         with open(file_path, 'r') as f:
             content = f.read()
             
-        # Find all action uses in the file
-        pattern = r'uses:\s+(actions/[^@\s]+)@([^@\s]+)'
+        # Find all action uses in the file, including potential comments
+        # This pattern captures: action_name@version_with_possible_comment
+        pattern = r'uses:\s+([^/\s]+/[^@\s]+)@([^@\n]+)'
         matches = re.findall(pattern, content)
         
-        for action_name, version in matches:
-            if action_name in target_actions:
+        for action_name, version_with_comment in matches:
+            # Only track non-local actions (those with / but not starting with ./)
+            if '/' in action_name and not action_name.startswith('./'):
                 # Take the latest version seen (they should all be the same after Dependabot)
-                action_versions[action_name] = version
+                action_versions[action_name] = version_with_comment.rstrip()
                 
     return action_versions
 
@@ -65,7 +62,7 @@ def update_sync_py(sync_py_path: str, action_versions: Dict[str, str]) -> bool:
     
     Args:
         sync_py_path: Path to sync.py file
-        action_versions: Dictionary of action names to versions
+        action_versions: Dictionary of action names to versions (may include comments)
         
     Returns:
         True if file was modified, False otherwise
@@ -80,9 +77,12 @@ def update_sync_py(sync_py_path: str, action_versions: Dict[str, str]) -> bool:
     original_content = content
     
     # Update hardcoded action versions 
-    for action_name, version in action_versions.items():
+    for action_name, version_with_comment in action_versions.items():
+        # Extract just the version part (before any comment) for sync.py
+        version = version_with_comment.split('#')[0].strip() if '#' in version_with_comment else version_with_comment.strip()
+        
         # Look for patterns like 'uses': 'actions/setup-node@v4'
-        pattern = rf"('uses':\s*')(actions/{action_name.split('/')[-1]})@([^']+)(')"
+        pattern = rf"('uses':\s*')(actions/{re.escape(action_name.split('/')[-1])})@([^']+)(')"
         replacement = rf"\1\2@{version}\4"
         content = re.sub(pattern, replacement, content)
         
@@ -102,7 +102,7 @@ def update_template_files(checks_dir: str, action_versions: Dict[str, str]) -> L
     
     Args:
         checks_dir: Path to pr-checks/checks directory
-        action_versions: Dictionary of action names to versions
+        action_versions: Dictionary of action names to versions (may include comments)
         
     Returns:
         List of files that were modified
@@ -117,10 +117,10 @@ def update_template_files(checks_dir: str, action_versions: Dict[str, str]) -> L
         original_content = content
         
         # Update action versions
-        for action_name, version in action_versions.items():
-            # Look for patterns like 'uses: actions/setup-node@v4'
-            pattern = rf"(uses:\s+{re.escape(action_name)})@([^@\s]+)"
-            replacement = rf"\1@{version}"
+        for action_name, version_with_comment in action_versions.items():
+            # Look for patterns like 'uses: actions/setup-node@v4' or 'uses: actions/setup-node@sha # comment'
+            pattern = rf"(uses:\s+{re.escape(action_name)})@([^@\n]+)"
+            replacement = rf"\1@{version_with_comment}"
             content = re.sub(pattern, replacement, content)
             
         if content != original_content:
@@ -138,7 +138,7 @@ def update_regular_workflows(workflow_dir: str, action_versions: Dict[str, str])
     
     Args:
         workflow_dir: Path to .github/workflows directory
-        action_versions: Dictionary of action names to versions
+        action_versions: Dictionary of action names to versions (may include comments)
         
     Returns:
         List of files that were modified
@@ -156,10 +156,10 @@ def update_regular_workflows(workflow_dir: str, action_versions: Dict[str, str])
         original_content = content
         
         # Update action versions
-        for action_name, version in action_versions.items():
-            # Look for patterns like 'uses: actions/setup-node@v4'
-            pattern = rf"(uses:\s+{re.escape(action_name)})@([^@\s]+)"
-            replacement = rf"\1@{version}"
+        for action_name, version_with_comment in action_versions.items():
+            # Look for patterns like 'uses: actions/setup-node@v4' or 'uses: actions/setup-node@sha # comment'
+            pattern = rf"(uses:\s+{re.escape(action_name)})@([^@\n]+)"
+            replacement = rf"\1@{version_with_comment}"
             content = re.sub(pattern, replacement, content)
             
         if content != original_content:

pr-checks/sync-back.sh

@@ -1,25 +0,0 @@
-#!/bin/bash
-
-# Sync-back wrapper script
-# This script runs the sync-back.py Python script to automatically sync 
-# Dependabot action version updates back to the source templates.
-
-set -euo pipefail
-
-SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-SYNC_BACK_PY="${SCRIPT_DIR}/sync-back.py"
-
-# Check if Python script exists
-if [[ ! -f "$SYNC_BACK_PY" ]]; then
-    echo "Error: sync-back.py not found at $SYNC_BACK_PY" >&2
-    exit 1
-fi
-
-# Make sure the Python script is executable
-chmod +x "$SYNC_BACK_PY"
-
-# Run the sync-back script with all provided arguments
-echo "Running sync-back automation..."
-python3 "$SYNC_BACK_PY" "$@"
-
-echo "Sync-back completed successfully."

pr-checks/test_sync_back.py

@@ -0,0 +1,339 @@
+#!/usr/bin/env python3
+"""
+Tests for the sync-back.py script
+"""
+
+import os
+import tempfile
+import shutil
+import unittest
+from pathlib import Path
+from unittest.mock import patch
+import sys
+
+# Add the current directory to sys.path and import the sync_back module
+sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
+
+# Import the sync-back module
+import importlib.util
+spec = importlib.util.spec_from_file_location("sync_back", os.path.join(os.path.dirname(__file__), "sync-back.py"))
+sync_back = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(sync_back)
+
+
+class TestSyncBack(unittest.TestCase):
+    
+    def setUp(self):
+        """Set up temporary directories and files for testing"""
+        self.test_dir = tempfile.mkdtemp()
+        self.workflow_dir = os.path.join(self.test_dir, ".github", "workflows")
+        self.checks_dir = os.path.join(self.test_dir, "pr-checks", "checks")
+        os.makedirs(self.workflow_dir)
+        os.makedirs(self.checks_dir)
+        
+        # Create sync.py file
+        self.sync_py_path = os.path.join(self.test_dir, "pr-checks", "sync.py")
+        
+    def tearDown(self):
+        """Clean up temporary directories"""
+        shutil.rmtree(self.test_dir)
+        
+    def test_scan_generated_workflows_basic(self):
+        """Test basic workflow scanning functionality"""
+        # Create a test generated workflow file
+        workflow_content = """
+name: Test Workflow
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v5
+      - uses: actions/setup-go@v6
+        """
+        
+        with open(os.path.join(self.workflow_dir, "__test.yml"), 'w') as f:
+            f.write(workflow_content)
+            
+        result = sync_back.scan_generated_workflows(self.workflow_dir)
+        
+        self.assertEqual(result['actions/checkout'], 'v4')
+        self.assertEqual(result['actions/setup-node'], 'v5')
+        self.assertEqual(result['actions/setup-go'], 'v6')
+        
+    def test_scan_generated_workflows_with_comments(self):
+        """Test scanning workflows with version comments"""
+        workflow_content = """
+name: Test Workflow
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ruby/setup-ruby@44511735964dcb71245e7e55f72539531f7bc0eb # v1.257.0
+      - uses: actions/setup-python@v6 # Latest Python
+        """
+        
+        with open(os.path.join(self.workflow_dir, "__test.yml"), 'w') as f:
+            f.write(workflow_content)
+            
+        result = sync_back.scan_generated_workflows(self.workflow_dir)
+        
+        self.assertEqual(result['actions/checkout'], 'v4')
+        self.assertEqual(result['ruby/setup-ruby'], '44511735964dcb71245e7e55f72539531f7bc0eb # v1.257.0')
+        self.assertEqual(result['actions/setup-python'], 'v6 # Latest Python')
+        
+    def test_scan_generated_workflows_ignores_local_actions(self):
+        """Test that local actions (starting with ./) are ignored"""
+        workflow_content = """
+name: Test Workflow
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ./.github/actions/local-action
+      - uses: ./another-local-action@v1
+        """
+        
+        with open(os.path.join(self.workflow_dir, "__test.yml"), 'w') as f:
+            f.write(workflow_content)
+            
+        result = sync_back.scan_generated_workflows(self.workflow_dir)
+        
+        self.assertEqual(result['actions/checkout'], 'v4')
+        self.assertNotIn('./.github/actions/local-action', result)
+        self.assertNotIn('./another-local-action', result)
+        
+    def test_scan_generated_workflows_skips_non_generated(self):
+        """Test that non-generated files are ignored"""
+        # Create generated file
+        generated_content = """
+name: Generated
+jobs:
+  test:
+    steps:
+      - uses: actions/checkout@v4
+        """
+        with open(os.path.join(self.workflow_dir, "__generated.yml"), 'w') as f:
+            f.write(generated_content)
+            
+        # Create regular file
+        regular_content = """
+name: Regular
+jobs:
+  test:
+    steps:
+      - uses: actions/checkout@v3
+        """
+        with open(os.path.join(self.workflow_dir, "regular.yml"), 'w') as f:
+            f.write(regular_content)
+            
+        result = sync_back.scan_generated_workflows(self.workflow_dir)
+        
+        # Should only see the version from the generated file
+        self.assertEqual(result['actions/checkout'], 'v4')
+        
+    def test_update_sync_py(self):
+        """Test updating sync.py file"""
+        sync_py_content = """
+steps = [
+    {
+        'uses': 'actions/setup-node@v4',
+        'with': {'node-version': '16'}
+    },
+    {
+        'uses': 'actions/setup-go@v5', 
+        'with': {'go-version': '1.19'}
+    }
+]
+        """
+        
+        with open(self.sync_py_path, 'w') as f:
+            f.write(sync_py_content)
+            
+        action_versions = {
+            'actions/setup-node': 'v5',
+            'actions/setup-go': 'v6'
+        }
+        
+        result = sync_back.update_sync_py(self.sync_py_path, action_versions)
+        self.assertTrue(result)
+        
+        with open(self.sync_py_path, 'r') as f:
+            updated_content = f.read()
+            
+        self.assertIn("'uses': 'actions/setup-node@v5'", updated_content)
+        self.assertIn("'uses': 'actions/setup-go@v6'", updated_content)
+        
+    def test_update_sync_py_with_comments(self):
+        """Test updating sync.py file when versions have comments"""
+        sync_py_content = """
+steps = [
+    {
+        'uses': 'actions/setup-node@v4',
+        'with': {'node-version': '16'}
+    }
+]
+        """
+        
+        with open(self.sync_py_path, 'w') as f:
+            f.write(sync_py_content)
+            
+        action_versions = {
+            'actions/setup-node': 'v5 # Latest version'
+        }
+        
+        result = sync_back.update_sync_py(self.sync_py_path, action_versions)
+        self.assertTrue(result)
+        
+        with open(self.sync_py_path, 'r') as f:
+            updated_content = f.read()
+            
+        # sync.py should get the version without comment
+        self.assertIn("'uses': 'actions/setup-node@v5'", updated_content)
+        self.assertNotIn("# Latest version", updated_content)
+        
+    def test_update_template_files(self):
+        """Test updating template files"""
+        template_content = """
+name: Test Template
+steps:
+  - uses: actions/checkout@v3
+  - uses: actions/setup-node@v4
+    with:
+      node-version: 16
+        """
+        
+        template_path = os.path.join(self.checks_dir, "test.yml")
+        with open(template_path, 'w') as f:
+            f.write(template_content)
+            
+        action_versions = {
+            'actions/checkout': 'v4',
+            'actions/setup-node': 'v5 # Latest'
+        }
+        
+        result = sync_back.update_template_files(self.checks_dir, action_versions)
+        self.assertEqual(len(result), 1)
+        self.assertIn(template_path, result)
+        
+        with open(template_path, 'r') as f:
+            updated_content = f.read()
+            
+        self.assertIn("uses: actions/checkout@v4", updated_content)
+        self.assertIn("uses: actions/setup-node@v5 # Latest", updated_content)
+        
+    def test_update_template_files_preserves_comments(self):
+        """Test that updating template files preserves version comments"""
+        template_content = """
+name: Test Template
+steps:
+  - uses: ruby/setup-ruby@44511735964dcb71245e7e55f72539531f7bc0eb # v1.256.0
+        """
+        
+        template_path = os.path.join(self.checks_dir, "test.yml")
+        with open(template_path, 'w') as f:
+            f.write(template_content)
+            
+        action_versions = {
+            'ruby/setup-ruby': '55511735964dcb71245e7e55f72539531f7bc0eb # v1.257.0'
+        }
+        
+        result = sync_back.update_template_files(self.checks_dir, action_versions)
+        self.assertEqual(len(result), 1)
+        
+        with open(template_path, 'r') as f:
+            updated_content = f.read()
+            
+        self.assertIn("uses: ruby/setup-ruby@55511735964dcb71245e7e55f72539531f7bc0eb # v1.257.0", updated_content)
+        
+    def test_update_regular_workflows(self):
+        """Test updating regular workflow files"""
+        # Create a regular workflow file
+        workflow_content = """
+name: Regular Workflow
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v4
+        """
+        
+        workflow_path = os.path.join(self.workflow_dir, "regular.yml")
+        with open(workflow_path, 'w') as f:
+            f.write(workflow_content)
+            
+        # Create a generated workflow file (should be ignored)
+        generated_path = os.path.join(self.workflow_dir, "__generated.yml")
+        with open(generated_path, 'w') as f:
+            f.write(workflow_content)
+            
+        action_versions = {
+            'actions/checkout': 'v4',
+            'actions/setup-node': 'v5'
+        }
+        
+        result = sync_back.update_regular_workflows(self.workflow_dir, action_versions)
+        
+        # Should only update the regular file, not the generated one
+        self.assertEqual(len(result), 1)
+        self.assertIn(workflow_path, result)
+        self.assertNotIn(generated_path, result)
+        
+        with open(workflow_path, 'r') as f:
+            updated_content = f.read()
+            
+        self.assertIn("uses: actions/checkout@v4", updated_content)
+        self.assertIn("uses: actions/setup-node@v5", updated_content)
+        
+    def test_no_changes_needed(self):
+        """Test that functions return False/empty when no changes are needed"""
+        # Test sync.py with no changes needed
+        sync_py_content = """
+steps = [
+    {
+        'uses': 'actions/setup-node@v5',
+        'with': {'node-version': '16'}
+    }
+]
+        """
+        
+        with open(self.sync_py_path, 'w') as f:
+            f.write(sync_py_content)
+            
+        action_versions = {
+            'actions/setup-node': 'v5'
+        }
+        
+        result = sync_back.update_sync_py(self.sync_py_path, action_versions)
+        self.assertFalse(result)
+        
+    def test_missing_sync_py_file(self):
+        """Test handling of missing sync.py file"""
+        result = sync_back.update_sync_py("/nonexistent/sync.py", {})
+        self.assertFalse(result)
+        
+    def test_main_dry_run(self):
+        """Test that dry-run functionality works"""
+        # Create a test workflow
+        workflow_content = """
+name: Test
+jobs:
+  test:
+    steps:
+      - uses: actions/checkout@v4
+        """
+        
+        with open(os.path.join(self.workflow_dir, "__test.yml"), 'w') as f:
+            f.write(workflow_content)
+            
+        # Test the scanning function directly since mocking main() is complex
+        result = sync_back.scan_generated_workflows(self.workflow_dir)
+        self.assertIn('actions/checkout', result)
+        self.assertEqual(result['actions/checkout'], 'v4')
+
+
+if __name__ == '__main__':
+    unittest.main()