chore(IDX): replace changed files step with gh action (#155) (#156)

cgundy · basvandijk · web-flow · commit 474ef14dc7f0 · 2025-04-30T10:07:28.000+02:00
* chore(IDX): replace changed files step with gh action (#155) * chore(IDX): update README, introduce git flow (#131) * refactor(IDX): change triggers (#130) * refactor(IDX): change triggers * Update .github/workflows/internal_vs_external.yml Co-authored-by: Bas van Dijk <bas@van.dijk.ch> --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * chore(IDX): combine workflow files (#129) * refactor(IDX): rename workflows (#133) * refactor(IDX): update bot policies workflow (#134) * Merge Develop Into Main (#132) (#136) * Merge main into develop (#140) * Merge Develop Into Main (#132) * chore(IDX): update README, introduce git flow (#131) * refactor(IDX): change triggers (#130) * refactor(IDX): change triggers * Update .github/workflows/internal_vs_external.yml Co-authored-by: Bas van Dijk <bas@van.dijk.ch> --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * chore(IDX): combine workflow files (#129) * refactor(IDX): rename workflows (#133) --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * chore(IDX): refactor bot policies (#138) --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * chore(IDX): update cla workflow (#139) * Merge Develop Into Main (#132) * chore(IDX): update README, introduce git flow (#131) * refactor(IDX): change triggers (#130) * refactor(IDX): change triggers * Update .github/workflows/internal_vs_external.yml Co-authored-by: Bas van Dijk <bas@van.dijk.ch> --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * chore(IDX): combine workflow files (#129) * refactor(IDX): rename workflows (#133) --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * chore(IDX): refactor bot policies (#138) * chore(IDX): update cla workflow --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * refactor(IDX): replace checkout with github action * remove test * add mock * fix test * update * update * mock gh login * update test * remove * set use_rest_api --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch> * Update repo_policies.yml * Update check_is_bot.yml * split by whitespace * update test * remove pin to develop * remove pin to develop --------- Co-authored-by: Bas van Dijk <bas@van.dijk.ch>
diff --git a/.github/workflows/check_cla_ruleset.yml b/.github/workflows/check_cla_ruleset.yml
@@ -76,12 +76,6 @@ jobs:
               labels: ["external-contributor"]
             })
 
-      - name: Checkout
-        uses: actions/checkout@v4
-        if: ${{ steps.accepts_external_contrib.outputs.accepts_contrib != 'false' }}
-        with:
-          repository: 'dfinity/public-workflows'
-
       - name: Check CLA
         id:  check-cla
         run: |
diff --git a/.github/workflows/repo_policies.yml b/.github/workflows/repo_policies.yml
@@ -22,20 +22,17 @@ jobs:
           repository: dfinity/public-workflows
           path: public-workflows
 
-        # Then switch back to this repository to make sure it's run from current
-      - name: Checkout Original Repository
-        uses: actions/checkout@v4
-        with:
-          path: current-repo # need to specify another path to avoid overwriting the first checkout
-          repository: ${{ github.event.pull_request.head.repo.full_name }}
-          ref: ${{ github.head_ref }}
-          fetch-depth: 100
-
       - name: Python Setup
         uses: ./public-workflows/.github/workflows/python-setup
         with:
           working-directory: public-workflows
 
+      - name: Get changed files
+        id: changed-files
+        uses: tj-actions/changed-files@ed68ef82c095e0d48ec87eccea555d944a631a4c # v46.0.5
+        with:
+          use_rest_api: 'true'
+
       - name: Bot Checks
         id:  bot-checks
         run: |
@@ -44,10 +41,8 @@ jobs:
           python public-workflows/reusable_workflows/repo_policies/check_bot_approved_files.py
         shell: bash
         env:
+          CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} # no actual token stored, read-only permissions
           GH_ORG: ${{ github.repository_owner }}
           USER: ${{ github.event.pull_request.user.login }}
           REPO: ${{ github.event.repository.name }}
-          MERGE_BASE_SHA: ${{ github.event.pull_request.base.sha }}
-          BRANCH_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
-          REPO_PATH: current-repo
diff --git a/reusable_workflows/repo_policies/check_bot_approved_files.py b/reusable_workflows/repo_policies/check_bot_approved_files.py
@@ -1,42 +1,11 @@
 import fnmatch
-import subprocess
-from typing import Optional
 
 import github3
 
 from shared.utils import download_gh_file, load_env_vars
 
 BOT_APPROVED_FILES_PATH = ".github/repo_policies/BOT_APPROVED_FILES"
-REQUIRED_ENV_VARS = [
-    "USER",
-    "GH_TOKEN",
-    "GH_ORG",
-    "REPO",
-    "REPO_PATH",
-    "MERGE_BASE_SHA",
-    "BRANCH_HEAD_SHA",
-]
-
-
-def get_changed_files(
-    merge_base_sha: str, branch_head_sha: str, repo_path: Optional[str] = None
-) -> list[str]:
-    """
-    Compares the files changed in the current branch to the merge base.
-    """
-    commit_range = f"{merge_base_sha}..{branch_head_sha}"
-    result = subprocess.run(
-        ["git", "diff", "--name-only", commit_range],
-        capture_output=True,
-        text=True,
-        cwd=repo_path,
-    )
-    if result.returncode != 0:
-        raise RuntimeError(
-            f"git diff failed with exit code {result.returncode}: {result.stderr}"
-        )
-    changed_files = result.stdout.strip().split("\n")
-    return changed_files
+REQUIRED_ENV_VARS = ["CHANGED_FILES", "USER", "GH_TOKEN", "GH_ORG", "REPO"]
 
 
 def get_approved_files_config(repo: github3.github.repo) -> str:
@@ -51,11 +20,11 @@ def get_approved_files_config(repo: github3.github.repo) -> str:
             f"No config file found. Make sure you have a file saved at {BOT_APPROVED_FILES_PATH} in the default branch"
         )
 
-
-def get_approved_files(config_file: str) -> list[str]:
+def get_approved_files(repo: github3.github.repo) -> list[str]:
     """
     Extracts the list of approved files from the config file.
     """
+    config_file = get_approved_files_config(repo)
     approved_files = [
         line for line in config_file.splitlines() if line.strip() and not line.strip().startswith("#")
     ]
@@ -77,12 +46,8 @@ def check_if_pr_is_blocked(env_vars: dict) -> None:
     """
     gh = github3.login(token=env_vars["GH_TOKEN"])
     repo = gh.repository(owner=env_vars["GH_ORG"], repository=env_vars["REPO"])
-    repo_path = env_vars["REPO_PATH"]
-    changed_files = get_changed_files(
-        env_vars["MERGE_BASE_SHA"], env_vars["BRANCH_HEAD_SHA"], repo_path
-    )
-    config = get_approved_files_config(repo)
-    approved_files = get_approved_files(config)
+    approved_files = get_approved_files(repo)
+    changed_files = env_vars["CHANGED_FILES"].split()
     block_pr = not check_files_in_approved_list(changed_files, approved_files)
     print(f"changed_files: {changed_files}")
     print(f"approved_files: {approved_files}")
diff --git a/reusable_workflows/tests/test_repo_policies.py b/reusable_workflows/tests/test_repo_policies.py
@@ -9,27 +9,9 @@
     check_if_pr_is_blocked,
     get_approved_files,
     get_approved_files_config,
-    get_changed_files,
 )
 
 
-@mock.patch("repo_policies.check_bot_approved_files.subprocess.run")
-def test_get_changed_files(mock_subprocess_run):
-    mock_subprocess_run.return_value = mock.Mock(
-        stdout="file1.py\nfile2.py\n", returncode=0, stderr=""
-    )
-
-    changed_files = get_changed_files("merge_base_sha", "branch_head_sha")
-
-    assert changed_files == ["file1.py", "file2.py"]
-    mock_subprocess_run.assert_called_once_with(
-        ["git", "diff", "--name-only", "merge_base_sha..branch_head_sha"],
-        capture_output=True,
-        text=True,
-        cwd=None,
-    )
-
-
 @mock.patch("repo_policies.check_bot_approved_files.download_gh_file")
 def test_get_approved_files_config(download_gh_file):
     repo = mock.Mock()
@@ -56,20 +38,26 @@ def test_get_approved_files_config_fails(download_gh_file):
     )
 
 
-def test_get_approved_files():
+@mock.patch("repo_policies.check_bot_approved_files.get_approved_files_config")
+def test_get_approved_files(get_approved_files_config):
     config_file = open(
         "reusable_workflows/tests/test_data/BOT_APPROVED_FILES", "r"
     ).read()
-    approved_files = get_approved_files(config_file)
+    get_approved_files_config.return_value = config_file
+    repo = mock.Mock()
+    approved_files = get_approved_files(repo)
 
     assert approved_files == ["file1", "file2", "folder/*.txt"]
 
 
-def get_test_approved_files():
+@mock.patch("repo_policies.check_bot_approved_files.get_approved_files_config")
+def get_test_approved_files(get_approved_files_config):
     config_file = open(
         "reusable_workflows/tests/test_data/BOT_APPROVED_FILES", "r"
     ).read()
-    approved_files = get_approved_files(config_file)
+    get_approved_files_config.return_value = config_file
+    repo = mock.Mock()
+    approved_files = get_approved_files(repo)
     return approved_files
 
 
@@ -95,62 +83,36 @@ def test_check_files_in_approved_list_fails():
     assert not check_files_in_approved_list(changed_files, approved_files)
 
 
-@mock.patch("repo_policies.check_bot_approved_files.get_changed_files")
-@mock.patch(
-    "repo_policies.check_bot_approved_files.get_approved_files_config"
-)
+@mock.patch("repo_policies.check_bot_approved_files.get_approved_files")
 @mock.patch("github3.login")
-def test_pr_is_blocked_false(gh_login, get_approved_files_config, get_changed_files):
+def test_pr_is_blocked_false(gh_login, get_approved_files):
     env_vars = {
+        "CHANGED_FILES": "file1 file2",
         "GH_TOKEN": "token",
         "GH_ORG": "org",
         "REPO": "repo",
-        "REPO_PATH": "path",
-        "MERGE_BASE_SHA": "base",
-        "BRANCH_HEAD_SHA": "head",
     }
     gh = mock.Mock()
     gh_login.return_value = gh
-    repo = mock.Mock()
-    gh.repository.return_value = repo
-    get_changed_files.return_value = ["file1", "file2"]
-    config_file = open(
-        "reusable_workflows/tests/test_data/BOT_APPROVED_FILES", "r"
-    ).read()
-    get_approved_files_config.return_value = config_file
+    approved_files = get_test_approved_files()
+    get_approved_files.return_value = approved_files
 
     check_if_pr_is_blocked(env_vars)
 
-    get_changed_files.assert_called_once_with("base", "head", "path")
-    get_approved_files_config.assert_called_once_with(repo)
 
-
-@mock.patch("repo_policies.check_bot_approved_files.get_changed_files")
-@mock.patch(
-    "repo_policies.check_bot_approved_files.get_approved_files_config"
-)
+@mock.patch("repo_policies.check_bot_approved_files.get_approved_files")
 @mock.patch("github3.login")
-def test_pr_is_blocked_true(gh_login, get_approved_files_config, get_changed_files):
+def test_pr_is_blocked_true(gh_login, get_approved_files):
     env_vars = {
+        "CHANGED_FILES": "file1 file2 file3",
         "GH_TOKEN": "token",
         "GH_ORG": "org",
         "REPO": "repo",
-        "REPO_PATH": "path",
-        "MERGE_BASE_SHA": "base",
-        "BRANCH_HEAD_SHA": "head",
     }
     gh = mock.Mock()
     gh_login.return_value = gh
-    repo = mock.Mock()
-    gh.repository.return_value = repo
-    get_changed_files.return_value = ["file1", "file2", "file3"]
-    config_file = open(
-        "reusable_workflows/tests/test_data/BOT_APPROVED_FILES", "r"
-    ).read()
-    get_approved_files_config.return_value = config_file
+    approved_files = get_test_approved_files()
+    get_approved_files.return_value = approved_files
 
     with pytest.raises(SystemExit):
         check_if_pr_is_blocked(env_vars)
-
-    get_changed_files.assert_called_once_with("base", "head", "path")
-    get_approved_files_config.assert_called_once_with(repo)
