Merge pull request uyuni-project#9512 from cbbayburt/changelog-action

Changelog action: Fixes after test runs

.github/workflows/changelogs.yml

@@ -50,7 +50,8 @@ jobs:
     - uses: actions/checkout@v4
       with:
         fetch-depth: 1
-        ref: ${{ github.event.pull_request.head.sha }}
+        # Checkout only workflow code
+        sparse-checkout: '.github/workflows'
     - id: master
       name: Get modified master changelog files
       uses: Ana06/[email protected]
@@ -68,7 +69,17 @@ jobs:
         echo
         echo "See https://github.com/uyuni-project/uyuni/wiki/Contributing for a guide to writing changelogs."
         exit 1
+    - name: Checkout the HEAD branch
+      id: checkout
+      if: "!contains(github.event.pull_request.body, '[x] No changelog needed')"
+      # Check out the PR HEAD in a subdirectory to read the updated changelog files
+      uses: actions/checkout@v4
+      with:
+        fetch-depth: 1
+        ref: ${{ github.event.pull_request.head.sha }}
+        path: .head
     - name: Test changelog entries
+      if: steps.checkout.outcome == 'success'
       env:
         GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         BZ_TOKEN: ${{ secrets.BUGZILLA_TOKEN }}
@@ -80,9 +91,9 @@ jobs:
       run: |
         pip install python-bugzilla~=3.2.0
 
-        CHANGED_FILES=$(gh pr diff -R $GIT_REPO $PR_NUM --name-only)
-        python .github/workflows/changelogs/changelogs.py \
-            --tracker-file $TRACKER_FILE --git-repo $GIT_REPO --pr-number $PR_NUM $CHANGED_FILES
+        CHANGED_FILES=$(gh api --paginate repos/$GIT_REPO/pulls/$PR_NUM/files | jq -r '.[].filename')
+        python .github/workflows/changelogs/changelogs.py ${RUNNER_DEBUG:+--verbose} \
+            --tracker-file $TRACKER_FILE --git-repo $GIT_REPO --uyuni-dir .head --pr-number $PR_NUM $CHANGED_FILES
 
   # Warns the user if they merged the PR, but the changelog test failed
   warn_user_if_merged:

.github/workflows/changelogs/changelogs.py

@@ -31,6 +31,7 @@ class RegexRules:
     WRONG_CAP_START = re.compile(r"^\W*[a-z]")
     WRONG_CAP_AFTER = re.compile(r"\. *[a-z]")
     WRONG_SPACING = re.compile(r"([.,;:])[^ \n]")
+    VERSION_REGEX = re.compile(r"\b(?:v?(\d+\.\d+(\.\d+)?)(?:[-.][a-zA-Z0-9]+)?)\b")
     TRACKER_LIKE = re.compile(r".{2,5}#\d+")
 
     def __init__(self, tracker_filename: str = None):
@@ -206,8 +207,7 @@ def __init__(self, uyuni_root: str, git_repo: str, pr_number: int, max_line_leng
         self.pr_number = pr_number
         self.max_line_length = max_line_length
         self.regex = regex_rules
-        if regex_rules.trackers:
-            self.bzapi = self.get_bugzilla_api()
+        self.bzapi = self.get_bugzilla_api() if regex_rules.trackers else None
 
     def get_bugzilla_api(self) -> bugzilla.Bugzilla:
         """Initialize and authenticate the Bugzilla API
@@ -249,14 +249,20 @@ def get_modified_files_for_pkg(self, pkg_path: str, pkg_name: str, files: list[s
         pkg_chlogs = []
         for f in files:
             # Check if the file exists in a subdirectory of the base path of the package
-            if os.path.normpath(os.path.dirname(f)).startswith(os.path.normpath(pkg_path)):
+            if (f.startswith(pkg_path)):
                 if os.path.basename(f).startswith(pkg_name + ".changes."):
                     # Ignore if the change is a removal
                     if os.path.isfile(os.path.join(self.uyuni_root, f)):
                         pkg_chlogs.append(f)
                 else:
                     pkg_files.append(f)
 
+        if logging.getLogger().isEnabledFor(logging.DEBUG):
+            if len(pkg_files):
+                logging.debug(f"Found {len(pkg_files)} file(s) in package {pkg_name}:\n  " + "\n  ".join(pkg_files))
+            if len(pkg_chlogs):
+                logging.debug(f"Found {len(pkg_chlogs)} changelog(s) in package {pkg_name}:\n  " + "\n  ".join(pkg_chlogs))
+
         return { "files": pkg_files, "changes": pkg_chlogs }
 
     def get_pkg_index(self, files: list[str]) -> dict[str, list[str]]:
@@ -293,7 +299,7 @@ def get_pkg_index(self, files: list[str]) -> dict[str, list[str]]:
             # Each file contains the package version and the
             # package path, separated by a space character
             pkg_path = linecache.getline(os.path.join(packages_dir, pkg_name), 1).rstrip().split(maxsplit=1)[1]
-            logging.debug(f"Package {pkg_name} is in path {pkg_path}")
+            logging.debug(f"Indexing package {pkg_name} in path {pkg_path}")
 
             # Get the list of modified files and changelog files for the package
             modified_files = self.get_modified_files_for_pkg(pkg_path, pkg_name, files)
@@ -337,11 +343,15 @@ def get_pr_trackers(self, git_repo: str, pr_number: int) -> dict[str, list[tuple
         assert git_repo
 
         logging.info(f"Requesting information for PR#{pr_number} at '{git_repo}'")
-        stream = os.popen(f'gh pr view -R {git_repo} {pr_number} --json title,commits -q ".title, .commits[].messageHeadline, .commits[].messageBody | select(length > 0)"')
-        commits = stream.read()
+
+        pr_path = f'repos/{git_repo}/pulls/{pr_number}'
+        stream = os.popen(f'gh api {pr_path} -q ".title" && gh api {pr_path}/commits -q ".[].commit.message | select(length > 0)"')
+        title_and_commits = stream.read()
         if stream.close():
             raise Exception("An error occurred when getting the PR information from the GitHub API.")
-        return self.extract_trackers(commits)
+
+        logging.debug(f"Retrieved title and commit messages for PR#{pr_number}:\n{title_and_commits}")
+        return self.extract_trackers(title_and_commits)
 
     def validate_chlog_entry(self, entry: Entry) -> list[Issue]:
         """Validate a single changelog entry"""
@@ -350,9 +360,15 @@ def validate_chlog_entry(self, entry: Entry) -> list[Issue]:
         # Test capitalization
         if re.match(self.regex.WRONG_CAP_START, entry.entry) or re.search(self.regex.WRONG_CAP_AFTER, entry.entry):
             issues.append(Issue(IssueType.WRONG_CAP, entry.file, entry.line, entry.end_line))
-        # Test spacing
-        if re.search(self.regex.WRONG_SPACING, entry.entry):
-            issues.append(Issue(IssueType.WRONG_SPACING, entry.file, entry.line, entry.end_line))
+
+        # Test spacing (ignored in version strings)
+        # Test to check if a match overlaps with a version string
+        overlaps = lambda ver_str, match: ver_str.start() <= match.start() and ver_str.end() >= match.end()
+        for match in re.finditer(self.regex.WRONG_SPACING, entry.entry):
+            # Ignore if part of a version string
+            if not any(overlaps(ver_str, match) for ver_str in re.finditer(self.regex.VERSION_REGEX, entry.entry[:match.end()])):
+                issues.append(Issue(IssueType.WRONG_SPACING, entry.file, entry.line, entry.end_line))
+                break
 
         return issues
 

.github/workflows/changelogs/test_changelogs.py

@@ -34,7 +34,9 @@ def file_list():
         "pkg/path/myfile.txt",
         "pkg/path/mypkg.changes.my.feature",
         "pkg/other/path/file.txt",
-        "pkg/other/otherpkg.changes.my.feature"
+        "pkg/other/otherpkg.changes.my.feature",
+        "pkg/path-extra/myfile-extra.txt",
+        "pkg/path-extra/mypkg-extra.changes.my.feature"
     ]
 
 @pytest.fixture
@@ -53,11 +55,17 @@ def base_path(tmp_path, file_list):
     pkg_dir = base_path / "rel-eng/packages"
     pkg_dir.mkdir(parents=True)
 
+    #'mypkg' package
     pkg_file = pkg_dir / "mypkg"
     pkg_file.write_text("1.0.0 pkg/path/")
 
+    #'otherpkg' package
     pkg_file = pkg_dir / "otherpkg"
     pkg_file.write_text("1.0.0 pkg/other/")
+
+    #'mypkg-extra' package
+    pkg_file = pkg_dir / "mypkg-extra"
+    pkg_file.write_text("1.0.0 pkg/path-extra/")
     return base_path
 
 @pytest.fixture
@@ -80,7 +88,7 @@ def gh_api_call(api_cmd):
 First commit message (tckr#99)
 Second commit message
 """
-        if re.search(r"^gh pr view -R [^ ]+ 999 .*", api_cmd):
+        if re.search(r"^gh api repos/[^/]*/[^/]*/pulls/999 .*", api_cmd):
             return io.StringIO(pr_data)
         else:
             raise Exception("An error occurred when getting the PR information from the GitHub API.")
@@ -124,10 +132,25 @@ def test_issue_gh_action_string(monkeypatch):
 
 def test_get_pkg_index(validator, file_list):
     pkg_idx = validator.get_pkg_index(file_list)
+
     assert "mypkg" in pkg_idx
+    assert 1 == len(pkg_idx["mypkg"]["files"])
+    assert 1 == len(pkg_idx["mypkg"]["changes"])
     assert "pkg/path/myfile.txt" in pkg_idx["mypkg"]["files"]
     assert "pkg/path/mypkg.changes.my.feature" in pkg_idx["mypkg"]["changes"]
 
+    assert "mypkg-extra" in pkg_idx
+    assert 1 == len(pkg_idx["mypkg-extra"]["files"])
+    assert 1 == len(pkg_idx["mypkg-extra"]["changes"])
+    assert "pkg/path-extra/myfile-extra.txt" in pkg_idx["mypkg-extra"]["files"]
+    assert "pkg/path-extra/mypkg-extra.changes.my.feature" in pkg_idx["mypkg-extra"]["changes"]
+
+    assert "otherpkg" in pkg_idx
+    assert 1 == len(pkg_idx["otherpkg"]["files"])
+    assert 1 == len(pkg_idx["otherpkg"]["changes"])
+    assert "pkg/other/path/file.txt" in pkg_idx["otherpkg"]["files"]
+    assert "pkg/other/otherpkg.changes.my.feature" in pkg_idx["otherpkg"]["changes"]
+
 def test_extract_trackers(validator_with_trackers):
     trackers = validator_with_trackers.extract_trackers("""
         This is a tckr#23 tracker.
@@ -181,8 +204,13 @@ def test_get_entry_obj_with_multiple_trackers(validator_with_trackers):
     assert ("tckr#01", "01") in entry.trackers["tckr"]
     assert ("tckr#02", "02") in entry.trackers["tckr"]
 
-def test_validate_chlog_file_valid(validator, chlog_file):
-    chlog_file.write_text("- This is a valid\n  multiline changelog entry\n")
+@pytest.mark.parametrize("entry_text", [
+    "- This is a valid changelog entry\n",
+    "- This is a valid\n  multiline changelog entry\n",
+    "- This is an entry with a version-1.2.3 string\n"
+])
+def test_validate_chlog_file_valid(validator, chlog_file, entry_text):
+    chlog_file.write_text(entry_text)
     issues, entries = validator.validate_chlog_file(str(chlog_file))
     assert not issues, issues_to_str(issues, 0)
     assert len(entries) == 1
@@ -236,6 +264,7 @@ def test_validate_chlog_file_multiple_issues_and_entries(validator, chlog_file):
     ("- This entry has wrong capitalization.\n  right here.\n", IssueType.WRONG_CAP),
     ("- This entry does not have a space.After a full stop\n", IssueType.WRONG_SPACING),
     ("- This entry does not have a space:After a colon\n", IssueType.WRONG_SPACING),
+    ("- Entry with version string 2.0 with.Wrong spacing.\n", IssueType.WRONG_SPACING),
     ("- This entry is" + " very" * 10 + " long\n", IssueType.LINE_TOO_LONG.format(DEFAULT_LINE_LENGTH)),
 ])
 def test_validate_chlog_file_rules(validator, chlog_file, entry_text, issue_msg):