Merge pull request #4216 from boegel/fix_skip_github_tests

fix typo in condition guarding installation of GitHub token in CI workflow to run unit tests + fix broken tests related to GitHub integration features
easybuilders · Apr 14, 2023 · 3a09a37 · 3a09a37
2 parents 5ec768e + 9eb5902
commit 3a09a37
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 42 deletions.
diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml
@@ -120,14 +120,16 @@ jobs:
 
     - name: install GitHub token (if available)
       env:
-        # see https://github.com/<username>/easybuild-framework/settings/secrets
-        GITHUB_TOKEN: ${{secrets.TEST_GITHUB_TOKEN}}
+        # token (owned by @boegelbot) with gist permissions (required for some of the tests for GitHub integration);
+        # this token is not available in pull requests, so tests that require it are skipped in PRs,
+        # and are only run after the PR gets merged
+        GITHUB_TOKEN: ${{secrets.CI_UNIT_TESTS_GITHUB_TOKEN}}
       run: |
         # don't install GitHub token when testing with Lmod 7.x or non-Lmod module tools,
         # and only when testing with Lua as module syntax,
         # to avoid hitting GitHub rate limit;
         # tests that require a GitHub token are skipped automatically when no GitHub token is available
-        if [[ ! "${{matrix.modules_tool}}" =~ 'Lmod-7' ]] && [[ ! "${{matrix.modules_tool}}" =~ 'modules-' ]] && [[ "${{matrix.modules_syntax}}" == 'Lua' ]]; then
+        if [[ ! "${{matrix.modules_tool}}" =~ 'Lmod-7' ]] && [[ ! "${{matrix.modules_tool}}" =~ 'modules-' ]] && [[ "${{matrix.module_syntax}}" == 'Lua' ]]; then
           if [ ! -z $GITHUB_TOKEN ]; then
             if [ "x${{matrix.python}}" == 'x2.6' ];
               then SET_KEYRING="keyring.set_keyring(keyring.backends.file.PlaintextKeyring())";

diff --git a/test/framework/github.py b/test/framework/github.py
@@ -266,7 +266,7 @@ def test_github_reasons_for_closing(self):
         }
         init_config(build_options=build_options)
 
-        pr_data, _ = gh.fetch_pr_data(1844, repo_owner, repo_name, GITHUB_TEST_ACCOUNT, full=True)
+        pr_data, _ = gh.fetch_pr_data(16080, repo_owner, repo_name, GITHUB_TEST_ACCOUNT, full=True)
 
         self.mock_stdout(True)
         self.mock_stderr(True)
@@ -278,12 +278,12 @@ def test_github_reasons_for_closing(self):
         self.mock_stderr(False)
 
         self.assertIsInstance(res, list)
-        self.assertEqual(stderr.strip(), "WARNING: Using easyconfigs from closed PR #1844")
+        self.assertEqual(stderr.strip(), "WARNING: Using easyconfigs from closed PR #16080")
         patterns = [
             "Status of last commit is SUCCESS",
             "Last comment on",
             "No activity since",
-            "* QEMU-2.4.0",
+            "* c-ares-1.18.1",
         ]
         for pattern in patterns:
             self.assertIn(pattern, stdout)
@@ -633,46 +633,47 @@ def test_github_det_commit_status(self):
             print("Skipping test_det_commit_status, no GitHub token available?")
             return
 
-        # ancient commit, from Jenkins era
+        # ancient commit, from Jenkins era, no commit status available anymore
         commit_sha = 'ec5d6f7191676a86a18404616691796a352c5f1d'
         res = gh.det_commit_status('easybuilders', 'easybuild-easyconfigs', commit_sha, GITHUB_TEST_ACCOUNT)
-        self.assertEqual(res, 'success')
+        self.assertEqual(res, None)
 
-        # commit with failing tests from Travis CI era (no GitHub Actions yet)
-        commit_sha = 'd0c62556caaa78944722dc84bbb1072bf9688f74'
+        # ancient commit with passing tests from Travis CI era (no GitHub Actions yet),
+        # no commit status available anymore
+        commit_sha = '21354990e4e6b4ca169b93d563091db4c6b2693e'
         res = gh.det_commit_status('easybuilders', 'easybuild-easyconfigs', commit_sha, GITHUB_TEST_ACCOUNT)
-        self.assertEqual(res, 'failure')
+        self.assertEqual(res, None)
 
-        # commit with passing tests from Travis CI era (no GitHub Actions yet)
-        commit_sha = '21354990e4e6b4ca169b93d563091db4c6b2693e'
+        # ancient commit tested by both Travis CI and GitHub Actions, no commit status available anymore
+        commit_sha = '1fba8ac835d62e78cdc7988b08f4409a1570cef1'
         res = gh.det_commit_status('easybuilders', 'easybuild-easyconfigs', commit_sha, GITHUB_TEST_ACCOUNT)
-        self.assertEqual(res, 'success')
+        self.assertEqual(res, None)
 
-        # commit with failing tests, tested by both Travis CI and GitHub Actions
-        commit_sha = '3a596de93dd95b651b0d1503562d888409364a96'
+        # old commit only tested by GitHub Actions, no commit status available anymore
+        commit_sha = 'd7130683f02fe8284df3557f0b2fd3947c2ea153'
         res = gh.det_commit_status('easybuilders', 'easybuild-easyconfigs', commit_sha, GITHUB_TEST_ACCOUNT)
-        self.assertEqual(res, 'failure')
+        self.assertEqual(res, None)
 
-        # commit with passing tests, tested by both Travis CI and GitHub Actions
-        commit_sha = '1fba8ac835d62e78cdc7988b08f4409a1570cef1'
+        # commit in test repo where no CI is running at all, no None as result
+        commit_sha = '8456f867b03aa001fd5a6fe5a0c4300145c065dc'
+        res = gh.det_commit_status('easybuilders', GITHUB_REPO, commit_sha, GITHUB_TEST_ACCOUNT)
+        self.assertEqual(res, None)
+
+        # recent commit (2023-04-11) with cancelled checks (GitHub Actions only)
+        commit_sha = 'c074f0bb3110c27d9969c3d0b19dde3eca868bd4'
         res = gh.det_commit_status('easybuilders', 'easybuild-easyconfigs', commit_sha, GITHUB_TEST_ACCOUNT)
-        self.assertEqual(res, 'success')
+        self.assertEqual(res, 'cancelled')
 
-        # commit with failing tests, only tested by GitHub Actions
-        commit_sha = 'd7130683f02fe8284df3557f0b2fd3947c2ea153'
+        # recent commit (2023-04-10) with failing checks (GitHub Actions only)
+        commit_sha = '1b4a45c62d7deaf19125756c46dc8f011fef66e1'
         res = gh.det_commit_status('easybuilders', 'easybuild-easyconfigs', commit_sha, GITHUB_TEST_ACCOUNT)
         self.assertEqual(res, 'failure')
 
-        # commit with passing tests, only tested by GitHub Actions
-        commit_sha = 'e6df09700a1b90c63b4f760eda4b590ee1a9c2fd'
+        # recent commit (2023-04-10) with successful checks (GitHub Actions only)
+        commit_sha = '56812a347acbaaa87f229fe319425020fe399647'
         res = gh.det_commit_status('easybuilders', 'easybuild-easyconfigs', commit_sha, GITHUB_TEST_ACCOUNT)
         self.assertEqual(res, 'success')
 
-        # commit in test repo where no CI is running at all
-        commit_sha = '8456f867b03aa001fd5a6fe5a0c4300145c065dc'
-        res = gh.det_commit_status('easybuilders', GITHUB_REPO, commit_sha, GITHUB_TEST_ACCOUNT)
-        self.assertEqual(res, None)
-
     def test_github_check_pr_eligible_to_merge(self):
         """Test check_pr_eligible_to_merge function"""
         def run_check(expected_result=False):

diff --git a/test/framework/options.py b/test/framework/options.py
@@ -4453,7 +4453,7 @@ def test_github_new_update_pr(self):
         ]
         self._assert_regexs(regexs, txt, assert_true=False)
 
-    def test_new_pr_warning_missing_patch(self):
+    def test_github_new_pr_warning_missing_patch(self):
         """Test warning printed by --new-pr (dry run only) when a specified patch file could not be found."""
 
         if self.github_token is None:
@@ -4678,7 +4678,7 @@ def test_github_new_pr_dependencies(self):
 
         self._assert_regexs(regexs, txt)
 
-    def test_new_pr_easyblock(self):
+    def test_github_new_pr_easyblock(self):
         """
         Test using --new-pr to open an easyblocks PR
         """
@@ -4751,14 +4751,15 @@ def test_github_merge_pr(self):
 
         expected_stdout = '\n'.join([
             "Checking eligibility of easybuilders/easybuild-easyconfigs PR #4781 for merging...",
-            "* test suite passes: OK",
             "* last test report is successful: OK",
             "* no pending change requests: OK",
             "* milestone is set: OK (3.3.1)",
             "* mergeable state is clean: PR is already merged",
         ])
         expected_stderr = '\n'.join([
             "* targets some_branch branch: FAILED; found 'develop' => not eligible for merging!",
+            # since commit status for old PRs is no longer available, so test suite check fails
+            "* test suite passes: (status: None) => not eligible for merging!",
             "* approved review: MISSING => not eligible for merging!",
             '',
             "WARNING: Review indicates this PR should not be merged (use -f/--force to do so anyway)",
@@ -4768,24 +4769,24 @@ def test_github_merge_pr(self):
 
         # full eligible merged PR, default target branch
         del args[-1]
-        args[1] = '4832'
+        args[1] = '17065'
 
         stdout, stderr = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False)
 
         expected_stdout = '\n'.join([
-            "Checking eligibility of easybuilders/easybuild-easyconfigs PR #4832 for merging...",
+            "Checking eligibility of easybuilders/easybuild-easyconfigs PR #17065 for merging...",
             "* targets develop branch: OK",
             "* test suite passes: OK",
             "* last test report is successful: OK",
             "* no pending change requests: OK",
-            "* approved review: OK (by wpoely86)",
-            "* milestone is set: OK (3.3.1)",
+            "* approved review: OK (by SebastianAchilles)",
+            "* milestone is set: OK (4.7.1)",
             "* mergeable state is clean: PR is already merged",
             '',
             "Review OK, merging pull request!",
             '',
-            "[DRY RUN] Adding comment to easybuild-easyconfigs issue #4832: 'Going in, thanks @boegel!'",
-            "[DRY RUN] Merged easybuilders/easybuild-easyconfigs pull request #4832",
+            "[DRY RUN] Adding comment to easybuild-easyconfigs issue #17065: 'Going in, thanks @boegel!'",
+            "[DRY RUN] Merged easybuilders/easybuild-easyconfigs pull request #17065",
         ])
         expected_stderr = ''
         self.assertEqual(stderr.strip(), expected_stderr)
@@ -4794,20 +4795,20 @@ def test_github_merge_pr(self):
         # --merge-pr also works on easyblocks (& framework) PRs
         args = [
             '--merge-pr',
-            '1206',
+            '2805',
             '--pr-target-repo=easybuild-easyblocks',
             '-D',
             '--github-user=%s' % GITHUB_TEST_ACCOUNT,
         ]
         stdout, stderr = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False)
         self.assertEqual(stderr.strip(), '')
         expected_stdout = '\n'.join([
-            "Checking eligibility of easybuilders/easybuild-easyblocks PR #1206 for merging...",
+            "Checking eligibility of easybuilders/easybuild-easyblocks PR #2805 for merging...",
             "* targets develop branch: OK",
             "* test suite passes: OK",
             "* no pending change requests: OK",
-            "* approved review: OK (by migueldiascosta)",
-            "* milestone is set: OK (3.3.1)",
+            "* approved review: OK (by ocaisa)",
+            "* milestone is set: OK (4.6.2)",
             "* mergeable state is clean: PR is already merged",
             '',
             "Review OK, merging pull request!",