Repository: cpp-linter/cpp-linter (language: python)

Issue:
Switch to unidiff
This migrates diff-parsing fallback mechanism (previously using regex patterns) to new dependecy [unidiff](https://pypi.org/project/unidiff).

This allows for a few new tactics:
- able to parse partial patches provided by REST API endpoints that list files with individual patch for each file (addresses cpp-linter/cpp-linter-action#249)
- able to move PR review comment generation from `rest_api/github_api.py` module into new `clang_tools.patcher.py` module because we don't need to keep data (previously used by `pygit2.Patch`) alive for the entire process (which avoided data corruption when the string of changes dropped out of scope).

   This change employs a mixin class to generate PR suggestions (for better/clearer code reuse).

I originally intended to use this to merge suggestions from both tools, but that isn't as easy as it sounds. Perhaps later, I can experiment with diff merging using these changes.


Discussion / hints:
It seems that our pr-review test wasn't generating suggestions with clang tools v7 and v8. I'm inclined to skip those versions when running tests, but It might be better to adjust the checks so that those versions generate suggestions.

Also, there seems to be a problem in CI where the coverage data (`.coverage*` files) isn't generated into repo root. This doesn't happen locally, so I'll have to investigate more in depth.

Fix patch:
diff --git a/cpp_linter/rest_api/github_api.py b/cpp_linter/rest_api/github_api.py
index 142b3da..9273122 100644
--- a/cpp_linter/rest_api/github_api.py
+++ b/cpp_linter/rest_api/github_api.py
@@ -111,11 +111,52 @@ def get_list_of_changed_files(
                 files_link += f"commits/{self.sha}"
             logger.info("Fetching files list from url: %s", files_link)
             response = self.api_request(
-                url=files_link, headers=self.make_headers(use_diff=True)
+                url=files_link, headers=self.make_headers(use_diff=True), strict=False
             )
-            files = parse_diff(response.text, file_filter, lines_changed_only)
-        else:
-            files = parse_diff(get_diff(), file_filter, lines_changed_only)
+            if response.status_code != 200:
+                return self._get_changed_files_paginated(
+                    files_link, lines_changed_only, file_filter
+                )
+            return parse_diff(response.text, file_filter, lines_changed_only)
+        return parse_diff(get_diff(), file_filter, lines_changed_only)
+
+    def _get_changed_files_paginated(
+        self, url: Optional[str], lines_changed_only: int, file_filter: FileFilter
+    ) -> List[FileObj]:
+        """A fallback implementation of getting file changes using a paginated
+        REST API endpoint."""
+        logger.info(
+            "Could not get raw diff of the %s event. "
+            "Perhaps there are too many changes?",
+            self.event_name,
+        )
+        assert url is not None
+        if self.event_name == "pull_request":
+            url += "/files"
+        files = []
+        while url is not None:
+            response = self.api_request(url)
+            url = RestApiClient.has_more_pages(response)
+            file_list: List[Dict[str, Any]]
+            if self.event_name == "pull_request":
+                file_list = response.json()
+            else:
+                file_list = response.json()["files"]
+            for file in file_list:
+                assert "filename" in file
+                file_name = file["filename"]
+                if not file_filter.is_source_or_ignored(file_name):
+                    continue
+                old_name = file_name
+                if "previous_filename" in file:
+                    old_name = file["previous_filename"]
+                assert "patch" in file
+                file_diff = (
+                    f"diff --git a/{file_name} b/{old_name}\n"
+                    + f"--- a/{file_name}\n+++ b/{old_name}\n"
+                    + file["patch"]
+                )
+                files.extend(parse_diff(file_diff, file_filter, lines_changed_only))
         return files
 
     def verify_files_are_present(self, files: List[FileObj]) -> None:


Test patch:
diff --git a/.github/workflows/run-dev-tests.yml b/.github/workflows/run-dev-tests.yml
index 2dbedc8..0704ae9 100644
--- a/.github/workflows/run-dev-tests.yml
+++ b/.github/workflows/run-dev-tests.yml
@@ -102,6 +102,7 @@ jobs:
         with:
           name: coverage-data-${{ runner.os }}-py${{ matrix.py }}-${{ matrix.version }}
           path: .coverage*
+          include-hidden-files: true
 
   coverage-report:
     needs: [test]
diff --git a/tests/list_changes/patch.diff b/tests/list_changes/patch.diff
new file mode 100644
index 0000000..7bda2e1
--- /dev/null
+++ b/tests/list_changes/patch.diff
@@ -0,0 +1,142 @@
+diff --git a/.github/workflows/cpp-lint-package.yml b/.github/workflows/cpp-lint-package.yml
+index 0418957..3b8c454 100644
+--- a/.github/workflows/cpp-lint-package.yml
++++ b/.github/workflows/cpp-lint-package.yml
+@@ -7,6 +7,7 @@ on:
+         description: 'which branch to test'
+         default: 'main'
+         required: true
++  pull_request:
+ 
+ jobs:
+   cpp-linter:
+@@ -14,9 +15,9 @@ jobs:
+ 
+     strategy:
+       matrix:
+-        clang-version: ['7', '8', '9','10', '11', '12', '13', '14', '15', '16', '17']
++        clang-version: ['10', '11', '12', '13', '14', '15', '16', '17']
+         repo: ['cpp-linter/cpp-linter']
+-        branch: ['${{ inputs.branch }}']
++        branch: ['pr-review-suggestions']
+       fail-fast: false
+ 
+     steps:
+@@ -62,10 +63,12 @@ jobs:
+           -i=build 
+           -p=build 
+           -V=${{ runner.temp }}/llvm 
+-          -f=false 
+           --extra-arg="-std=c++14 -Wall" 
+-          --thread-comments=${{ matrix.clang-version == '12' }} 
+-          -a=${{ matrix.clang-version == '12' }}
++          --file-annotations=false
++          --lines-changed-only=true
++          --thread-comments=${{ matrix.clang-version == '16' }} 
++          --tidy-review=${{ matrix.clang-version == '16' }}
++          --format-review=${{ matrix.clang-version == '16' }}
+ 
+       - name: Fail fast?!
+         if: steps.linter.outputs.checks-failed > 0
+diff --git a/src/demo.cpp b/src/demo.cpp
+index 0c1db60..1bf553e 100644
+--- a/src/demo.cpp
++++ b/src/demo.cpp
+@@ -1,17 +1,18 @@
+ /** This is a very ugly test code (doomed to fail linting) */
+ #include "demo.hpp"
+-#include <cstdio>
+-#include <cstddef>
++#include <stdio.h>
+ 
+-// using size_t from cstddef
+-size_t dummyFunc(size_t i) { return i; }
+ 
+-int main()
+-{
+-    for (;;)
+-        break;
++
++
++int main(){
++
++    for (;;) break;
++
+ 
+     printf("Hello world!\n");
+ 
+-    return 0;
+-}
++
++
++
++    return 0;}
+diff --git a/src/demo.hpp b/src/demo.hpp
+index 2695731..f93d012 100644
+--- a/src/demo.hpp
++++ b/src/demo.hpp
+@@ -5,12 +5,10 @@
+ class Dummy {
+     char* useless;
+     int numb;
++    Dummy() :numb(0), useless("\0"){}
+ 
+     public:
+-    void *not_usefull(char *str){
+-        useless = str;
+-        return 0;
+-    }
++    void *not_useful(char *str){useless = str;}
+ };
+ 
+ 
+@@ -28,14 +26,11 @@ class Dummy {
+ 
+ 
+ 
+-
+-
+-
+-
+ 
+ 
+ struct LongDiff
+ {
++
+     long diff;
+ 
+ };
+
+diff --git a/src/demo.c b/src/demo.c
+index 0c1db60..1bf553e 100644
+--- a/src/demo.c
++++ b/src/demo.c
+@@ -1,17 +1,18 @@
+ /** This is a very ugly test code (doomed to fail linting) */
+ #include "demo.hpp"
+-#include <cstdio>
+-#include <cstddef>
++#include <stdio.h>
+ 
+-// using size_t from cstddef
+-size_t dummyFunc(size_t i) { return i; }
+ 
+-int main()
+-{
+-    for (;;)
+-        break;
++
++
++int main(){
++
++    for (;;) break;
++
+ 
+     printf("Hello world!\n");
+ 
+-    return 0;
+-}
++
++
++
++    return 0;}
diff --git a/tests/list_changes/pull_request_files_pg1.json b/tests/list_changes/pull_request_files_pg1.json
new file mode 100644
index 0000000..5a70fd9
--- /dev/null
+++ b/tests/list_changes/pull_request_files_pg1.json
@@ -0,0 +1,27 @@
+[
+  {
+    "sha": "52501fa1dc96d6bc6f8a155816df041b1de975d9",
+    "filename": ".github/workflows/cpp-lint-package.yml",
+    "status": "modified",
+    "additions": 9,
+    "deletions": 5,
+    "changes": 14,
+    "blob_url": "https://github.com/cpp-linter/test-cpp-linter-action/blob/635a9c57bdcca07b99ddef52c2640337c50280b1/.github%2Fworkflows%2Fcpp-lint-package.yml",
+    "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/.github%2Fworkflows%2Fcpp-lint-package.yml",
+    "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/.github%2Fworkflows%2Fcpp-lint-package.yml?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
+    "patch": "@@ -7,16 +7,17 @@ on:\n         description: 'which branch to test'\n         default: 'main'\n         required: true\n+  pull_request:\n \n jobs:\n   cpp-linter:\n     runs-on: windows-latest\n \n     strategy:\n       matrix:\n-        clang-version: ['7', '8', '9','10', '11', '12', '13', '14', '15', '16', '17']\n+        clang-version: ['10', '11', '12', '13', '14', '15', '16', '17']\n         repo: ['cpp-linter/cpp-linter']\n-        branch: ['${{ inputs.branch }}']\n+        branch: ['pr-review-suggestions']\n       fail-fast: false\n \n     steps:\n@@ -62,10 +63,13 @@ jobs:\n           -i=build \n           -p=build \n           -V=${{ runner.temp }}/llvm \n-          -f=false \n           --extra-arg=\"-std=c++14 -Wall\" \n-          --thread-comments=${{ matrix.clang-version == '12' }} \n-          -a=${{ matrix.clang-version == '12' }}\n+          --file-annotations=false\n+          --lines-changed-only=false\n+          --extension=h,c\n+          --thread-comments=${{ matrix.clang-version == '16' }} \n+          --tidy-review=${{ matrix.clang-version == '16' }}\n+          --format-review=${{ matrix.clang-version == '16' }}\n \n       - name: Fail fast?!\n         if: steps.linter.outputs.checks-failed > 0"
+  },
+  {
+    "sha": "1bf553e06e4b7c6c9a9be5da4845acbdeb04f6a5",
+    "filename": "src/demo.cpp",
+    "previous_filename": "src/demo.c",
+    "status": "modified",
+    "additions": 11,
+    "deletions": 10,
+    "changes": 21,
+    "blob_url": "https://github.com/cpp-linter/test-cpp-linter-action/blob/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.cpp",
+    "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.cpp",
+    "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/src%2Fdemo.cpp?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
+    "patch": "@@ -1,17 +1,18 @@\n /** This is a very ugly test code (doomed to fail linting) */\n #include \"demo.hpp\"\n-#include <cstdio>\n-#include <cstddef>\n+#include <stdio.h>\n \n-// using size_t from cstddef\n-size_t dummyFunc(size_t i) { return i; }\n \n-int main()\n-{\n-    for (;;)\n-        break;\n+\n+\n+int main(){\n+\n+    for (;;) break;\n+\n \n     printf(\"Hello world!\\n\");\n \n-    return 0;\n-}\n+\n+\n+\n+    return 0;}"
+  }
+]
diff --git a/tests/list_changes/pull_request_files_pg2.json b/tests/list_changes/pull_request_files_pg2.json
new file mode 100644
index 0000000..987a239
--- /dev/null
+++ b/tests/list_changes/pull_request_files_pg2.json
@@ -0,0 +1,14 @@
+[
+    {
+        "sha": "f93d0122ae2e3c1952c795837d71c432036b55eb",
+        "filename": "src/demo.hpp",
+        "status": "modified",
+        "additions": 3,
+        "deletions": 8,
+        "changes": 11,
+        "blob_url": "https://github.com/cpp-linter/test-cpp-linter-action/blob/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.hpp",
+        "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.hpp",
+        "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/src%2Fdemo.hpp?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
+        "patch": "@@ -5,12 +5,10 @@\n class Dummy {\n     char* useless;\n     int numb;\n+    Dummy() :numb(0), useless(\"\\0\"){}\n \n     public:\n-    void *not_usefull(char *str){\n-        useless = str;\n-        return 0;\n-    }\n+    void *not_useful(char *str){useless = str;}\n };\n \n \n@@ -28,14 +26,11 @@ class Dummy {\n \n \n \n-\n-\n-\n-\n \n \n struct LongDiff\n {\n+\n     long diff;\n \n };"
+    }
+]
diff --git a/tests/list_changes/push_files_pg1.json b/tests/list_changes/push_files_pg1.json
new file mode 100644
index 0000000..8022a1e
--- /dev/null
+++ b/tests/list_changes/push_files_pg1.json
@@ -0,0 +1,29 @@
+{
+  "files": [
+    {
+      "sha": "52501fa1dc96d6bc6f8a155816df041b1de975d9",
+      "filename": ".github/workflows/cpp-lint-package.yml",
+      "status": "modified",
+      "additions": 9,
+      "deletions": 5,
+      "changes": 14,
+      "blob_url": "https://github.com/cpp-linter/test-cpp-linter-action/blob/635a9c57bdcca07b99ddef52c2640337c50280b1/.github%2Fworkflows%2Fcpp-lint-package.yml",
+      "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/.github%2Fworkflows%2Fcpp-lint-package.yml",
+      "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/.github%2Fworkflows%2Fcpp-lint-package.yml?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
+      "patch": "@@ -7,16 +7,17 @@ on:\n         description: 'which branch to test'\n         default: 'main'\n         required: true\n+  pull_request:\n \n jobs:\n   cpp-linter:\n     runs-on: windows-latest\n \n     strategy:\n       matrix:\n-        clang-version: ['7', '8', '9','10', '11', '12', '13', '14', '15', '16', '17']\n+        clang-version: ['10', '11', '12', '13', '14', '15', '16', '17']\n         repo: ['cpp-linter/cpp-linter']\n-        branch: ['${{ inputs.branch }}']\n+        branch: ['pr-review-suggestions']\n       fail-fast: false\n \n     steps:\n@@ -62,10 +63,13 @@ jobs:\n           -i=build \n           -p=build \n           -V=${{ runner.temp }}/llvm \n-          -f=false \n           --extra-arg=\"-std=c++14 -Wall\" \n-          --thread-comments=${{ matrix.clang-version == '12' }} \n-          -a=${{ matrix.clang-version == '12' }}\n+          --file-annotations=false\n+          --lines-changed-only=false\n+          --extension=h,c\n+          --thread-comments=${{ matrix.clang-version == '16' }} \n+          --tidy-review=${{ matrix.clang-version == '16' }}\n+          --format-review=${{ matrix.clang-version == '16' }}\n \n       - name: Fail fast?!\n         if: steps.linter.outputs.checks-failed > 0"
+    },
+    {
+      "sha": "1bf553e06e4b7c6c9a9be5da4845acbdeb04f6a5",
+      "filename": "src/demo.cpp",
+      "previous_filename": "src/demo.c",
+      "status": "modified",
+      "additions": 11,
+      "deletions": 10,
+      "changes": 21,
+      "blob_url": "https://github.com/cpp-linter/test-cpp-linter-action/blob/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.cpp",
+      "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.cpp",
+      "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/src%2Fdemo.cpp?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
+      "patch": "@@ -1,17 +1,18 @@\n /** This is a very ugly test code (doomed to fail linting) */\n #include \"demo.hpp\"\n-#include <cstdio>\n-#include <cstddef>\n+#include <stdio.h>\n \n-// using size_t from cstddef\n-size_t dummyFunc(size_t i) { return i; }\n \n-int main()\n-{\n-    for (;;)\n-        break;\n+\n+\n+int main(){\n+\n+    for (;;) break;\n+\n \n     printf(\"Hello world!\\n\");\n \n-    return 0;\n-}\n+\n+\n+\n+    return 0;}"
+    }
+  ]
+}
diff --git a/tests/list_changes/push_files_pg2.json b/tests/list_changes/push_files_pg2.json
new file mode 100644
index 0000000..573f532
--- /dev/null
+++ b/tests/list_changes/push_files_pg2.json
@@ -0,0 +1,16 @@
+{
+  "files": [
+    {
+      "sha": "f93d0122ae2e3c1952c795837d71c432036b55eb",
+      "filename": "src/demo.hpp",
+      "status": "modified",
+      "additions": 3,
+      "deletions": 8,
+      "changes": 11,
+      "blob_url": "https://github.com/cpp-linter/test-cpp-linter-action/blob/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.hpp",
+      "raw_url": "https://github.com/cpp-linter/test-cpp-linter-action/raw/635a9c57bdcca07b99ddef52c2640337c50280b1/src%2Fdemo.hpp",
+      "contents_url": "https://api.github.com/repos/cpp-linter/test-cpp-linter-action/contents/src%2Fdemo.hpp?ref=635a9c57bdcca07b99ddef52c2640337c50280b1",
+      "patch": "@@ -5,12 +5,10 @@\n class Dummy {\n     char* useless;\n     int numb;\n+    Dummy() :numb(0), useless(\"\\0\"){}\n \n     public:\n-    void *not_usefull(char *str){\n-        useless = str;\n-        return 0;\n-    }\n+    void *not_useful(char *str){useless = str;}\n };\n \n \n@@ -28,14 +26,11 @@ class Dummy {\n \n \n \n-\n-\n-\n-\n \n \n struct LongDiff\n {\n+\n     long diff;\n \n };"
+    }
+  ]
+}
diff --git a/tests/list_changes/test_get_file_changes.py b/tests/list_changes/test_get_file_changes.py
new file mode 100644
index 0000000..5e15d8b
--- /dev/null
+++ b/tests/list_changes/test_get_file_changes.py
@@ -0,0 +1,128 @@
+import json
+import logging
+from pathlib import Path
+import pytest
+import requests_mock
+from cpp_linter import GithubApiClient, logger, FileFilter
+import cpp_linter.rest_api.github_api
+
+
+TEST_PR = 27
+TEST_REPO = "cpp-linter/test-cpp-linter-action"
+TEST_SHA = "708a1371f3a966a479b77f1f94ec3b7911dffd77"
+TEST_API_URL = "https://api.mock.com"
+TEST_ASSETS = Path(__file__).parent
+TEST_DIFF = (TEST_ASSETS / "patch.diff").read_text(encoding="utf-8")
+
+
+@pytest.mark.parametrize(
+    "event_name,paginated,fake_runner",
+    [
+        # push event (full diff)
+        (
+            "unknown",  # let coverage include logged warning about unknown event
+            False,
+            True,
+        ),
+        # pull request event (full diff)
+        (
+            "pull_request",
+            False,
+            True,
+        ),
+        # push event (paginated diff)
+        (
+            "push",  # let coverage include logged warning about unknown event
+            True,
+            True,
+        ),
+        # pull request event (paginated diff)
+        (
+            "pull_request",
+            True,
+            True,
+        ),
+        # local dev env
+        ("", False, False),
+    ],
+    ids=[
+        "push",
+        "pull_request",
+        "push(paginated)",
+        "pull_request(paginated)",
+        "local_dev",
+    ],
+)
+def test_get_changed_files(
+    caplog: pytest.LogCaptureFixture,
+    monkeypatch: pytest.MonkeyPatch,
+    tmp_path: Path,
+    event_name: str,
+    paginated: bool,
+    fake_runner: bool,
+):
+    """test getting a list of changed files for an event."""
+    caplog.set_level(logging.DEBUG, logger=logger.name)
+
+    # setup test to act as though executed in user's repo's CI
+    event_payload = {"number": TEST_PR}
+    event_payload_path = tmp_path / "event_payload.json"
+    event_payload_path.write_text(json.dumps(event_payload), encoding="utf-8")
+    monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_payload_path))
+    monkeypatch.setenv("GITHUB_EVENT_NAME", event_name)
+    monkeypatch.setenv("GITHUB_REPOSITORY", TEST_REPO)
+    monkeypatch.setenv("GITHUB_SHA", TEST_SHA)
+    monkeypatch.setenv("GITHUB_API_URL", TEST_API_URL)
+    monkeypatch.setenv("CI", str(fake_runner).lower())
+    monkeypatch.setenv("GITHUB_TOKEN", "123456")
+    gh_client = GithubApiClient()
+
+    if not fake_runner:
+        # getting a diff in CI (on a shallow checkout) fails
+        # monkey patch the .git.get_diff() to return the test's diff asset
+        monkeypatch.setattr(
+            cpp_linter.rest_api.github_api,
+            "get_diff",
+            lambda *args: TEST_DIFF,
+        )
+
+    endpoint = f"{TEST_API_URL}/repos/{TEST_REPO}/commits/{TEST_SHA}"
+    if event_name == "pull_request":
+        endpoint = f"{TEST_API_URL}/repos/{TEST_REPO}/pulls/{TEST_PR}"
+
+    with requests_mock.Mocker() as mock:
+        mock.get(
+            endpoint,
+            request_headers={
+                "Authorization": "token 123456",
+                "Accept": "application/vnd.github.diff",
+            },
+            text=TEST_DIFF if not paginated else "",
+            status_code=200 if not paginated else 403,
+        )
+
+        if paginated:
+            mock_endpoint = endpoint
+            if event_name == "pull_request":
+                mock_endpoint += "/files"
+            logger.debug("mock endpoint: %s", mock_endpoint)
+            for pg in (1, 2):
+                response_asset = f"{event_name}_files_pg{pg}.json"
+                mock.get(
+                    mock_endpoint + ("" if pg == 1 else "?page=2"),
+                    request_headers={
+                        "Authorization": "token 123456",
+                        "Accept": "application/vnd.github.raw+json",
+                    },
+                    headers={"link": f'<{mock_endpoint}?page=2>; rel="next"'}
+                    if pg == 1
+                    else {},
+                    text=(TEST_ASSETS / response_asset).read_text(encoding="utf-8"),
+                )
+
+        files = gh_client.get_list_of_changed_files(
+            FileFilter(extensions=["cpp", "hpp"]), lines_changed_only=1
+        )
+        assert files
+        for file in files:
+            assert file.name in ("src/demo.cpp", "src/demo.hpp")
diff --git a/tests/test_misc.py b/tests/test_misc.py
index 128d338..286d702 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -8,7 +8,6 @@
 from typing import List, cast
 
 import pytest
-import requests_mock
 
 from cpp_linter.common_fs import get_line_cnt_from_cols, FileObj
 from cpp_linter.common_fs.file_filter import FileFilter
@@ -19,7 +18,6 @@
     start_log_group,
     end_log_group,
 )
-import cpp_linter.rest_api.github_api
 from cpp_linter.rest_api.github_api import GithubApiClient
 
 
@@ -82,70 +80,6 @@ def test_list_src_files(
         assert Path(file.name).suffix.lstrip(".") in extensions
 
 
-@pytest.mark.parametrize(
-    "pseudo,expected_url,fake_runner",
-    [
-        (
-            dict(
-                repo="cpp-linter/test-cpp-linter-action",
-                sha="708a1371f3a966a479b77f1f94ec3b7911dffd77",
-                event_name="unknown",  # let coverage include logged warning
-            ),
-            "{rest_api_url}/repos/{repo}/commits/{sha}",
-            True,
-        ),
-        (
-            dict(
-                repo="cpp-linter/test-cpp-linter-action",
-                event_name="pull_request",
-            ),
-            "{rest_api_url}/repos/{repo}/pulls/{number}",
-            True,
-        ),
-        ({}, "", False),
-    ],
-    ids=["push", "pull_request", "local_dev"],
-)
-def test_get_changed_files(
-    caplog: pytest.LogCaptureFixture,
-    monkeypatch: pytest.MonkeyPatch,
-    pseudo: dict,
-    expected_url: str,
-    fake_runner: bool,
-):
-    """test getting a list of changed files for an event.
-
-    This is expected to fail if a github token not supplied as an env var.
-    We don't need to supply one for this test because the tested code will
-    execute anyway.
-    """
-    caplog.set_level(logging.DEBUG, logger=logger.name)
-    # setup test to act as though executed in user's repo's CI
-    monkeypatch.setenv("CI", str(fake_runner).lower())
-    gh_client = GithubApiClient()
-    for name, value in pseudo.items():
-        setattr(gh_client, name, value)
-    if "event_name" in pseudo and pseudo["event_name"] == "pull_request":
-        gh_client.pull_request = 19
-    if not fake_runner:
-        # getting a diff in CI (on a shallow checkout) fails
-        # monkey patch the .git.get_diff() to return nothing
-        monkeypatch.setattr(
-            cpp_linter.rest_api.github_api, "get_diff", lambda *args: ""
-        )
-    monkeypatch.setenv("GITHUB_TOKEN", "123456")
-
-    with requests_mock.Mocker() as mock:
-        mock.get(
-            expected_url.format(number=19, rest_api_url=gh_client.api_url, **pseudo),
-            request_headers={"Authorization": "token 123456"},
-            text="",
-        )
-
-        files = gh_client.get_list_of_changed_files(FileFilter(), 0)
-        assert not files
-
-
 @pytest.mark.parametrize("line,cols,offset", [(13, 5, 144), (19, 1, 189)])
 def test_file_offset_translation(line: int, cols: int, offset: int):
     """Validate output from ``get_line_cnt_from_cols()``"""