diff --git a/action.yml b/action.yml index 2d12475..3e32f9e 100644 --- a/action.yml +++ b/action.yml @@ -25,7 +25,7 @@ outputs: description: 'Path to a file containing the body to put in the Release' value: ${{ steps.create-tag.outputs.release_body_path }} commit_authors: - description: 'Comma-separated list of the first page of author usernames since the last Release' + description: 'Comma-separated author usernames since the last Release, in the order their commits appear (newest first)' value: ${{ steps.create-tag.outputs.commit_authors }} runs: using: 'composite' diff --git a/create_tag.py b/create_tag.py index 6addc1d..2c822bc 100755 --- a/create_tag.py +++ b/create_tag.py @@ -86,6 +86,25 @@ def enumerate_changes(repo, latest_tag, head_commit, max_commits=50): yield parsed +def github_logins(gh_repo, author_to_sha): + """GitHub logins for each commit author, in the order their commits were seen. + + Returns a list, not a set: the order is part of the output contract. Distinct + author emails can resolve to the same login, so duplicates are dropped while + keeping the first position. + """ + logins = [] + for email, sha in author_to_sha.items(): + try: + gh_commit = gh_repo.get_commit(sha) + except Exception: + logging.debug(f"could not look up GitHub user for {email}") + continue + if gh_commit.author and gh_commit.author.login not in logins: + logins.append(gh_commit.author.login) + return logins + + def main(): parser = argparse.ArgumentParser() parser.add_argument( @@ -151,18 +170,16 @@ def main(): ) ] - commit_authors = set() + commit_authors = [] if last_tag: by_type = collections.defaultdict(list) by_type["feat"] = [] by_type["fix"] = [] - change_authors = set() # email of author -> SHA of the first commit found in changes author_to_sha = {} for change in enumerate_changes(repo, last_tag, commit): by_type[change.type].append(f"{change.description} ({change.author})") - change_authors.add(change.author) if change.author not in author_to_sha: author_to_sha[change.author] = change.sha for breaker in change.breaking_changes: @@ -184,14 +201,7 @@ def main(): if github_client is not None and author_to_sha: # Map commit authors to GitHub usernames via commit SHA lookup # it is used to add contributors to the tag - gh_repo = github_client.get_repo(args.repository) - for email, sha in author_to_sha.items(): - try: - gh_commit = gh_repo.get_commit(sha) - if gh_commit.author: - commit_authors.add(gh_commit.author.login) - except Exception: - logging.debug(f"could not look up GitHub user for {email}") + commit_authors = github_logins(github_client.get_repo(args.repository), author_to_sha) release_body_path = f"release_notes-{new_name}.txt" with open(os.path.join(args.checkout_dir, release_body_path), "w") as tf: diff --git a/create_tag_test.py b/create_tag_test.py index ac05b80..c7dec77 100644 --- a/create_tag_test.py +++ b/create_tag_test.py @@ -2,9 +2,13 @@ from unittest import mock import git +import github +import github.Commit +import github.NamedUser +import github.Repository import pytest -from create_tag import PRETTY_TYPES, CommitMessage, enumerate_changes +from create_tag import PRETTY_TYPES, CommitMessage, enumerate_changes, github_logins def _commit(message, sha="abc123", email="dev@example.com"): @@ -109,3 +113,63 @@ def test_enumerate_changes_without_merge_base_yields_nothing(repo): """No merge base (e.g. an orphan history) is swallowed, not raised.""" orphan = repo.git.commit_tree(repo.head.commit.tree.hexsha, m="feat: orphan") assert list(enumerate_changes(repo, TAG, repo.commit(orphan))) == [] + + +def _gh_repo(sha_to_login): + """Repository stub whose get_commit maps a SHA to a GitHub login (None = ghost).""" + gh_repo = mock.create_autospec(github.Repository.Repository, instance=True) + + def get_commit(sha): + gh_commit = mock.create_autospec(github.Commit.Commit, instance=True) + login = sha_to_login[sha] + if login is None: + gh_commit.author = None + else: + gh_commit.author = mock.create_autospec(github.NamedUser.NamedUser, instance=True) + gh_commit.author.login = login + return gh_commit + + gh_repo.get_commit.side_effect = get_commit + return gh_repo + + +def test_github_logins_preserves_commit_order(): + """Order is the output contract -- #11 wanted commit order, a set gave hash order.""" + author_to_sha = { + "zoe@example.com": "sha1", + "adam@example.com": "sha2", + "mia@example.com": "sha3", + } + gh_repo = _gh_repo({"sha1": "zoe-gh", "sha2": "adam-gh", "sha3": "mia-gh"}) + assert github_logins(gh_repo, author_to_sha) == ["zoe-gh", "adam-gh", "mia-gh"] + + +def test_github_logins_dedupes_keeping_first_position(): + author_to_sha = { + "zoe@work.com": "sha1", + "adam@example.com": "sha2", + "zoe@personal.com": "sha3", + } + gh_repo = _gh_repo({"sha1": "zoe-gh", "sha2": "adam-gh", "sha3": "zoe-gh"}) + assert github_logins(gh_repo, author_to_sha) == ["zoe-gh", "adam-gh"] + + +def test_github_logins_skips_lookup_failures_without_losing_the_rest(): + author_to_sha = {"a@example.com": "sha1", "b@example.com": "sha2", "c@example.com": "sha3"} + resolvable = _gh_repo({"sha1": "a-gh", "sha3": "c-gh"}) + + def get_commit(sha): + if sha == "sha2": + raise github.GithubException(404, "Not Found", None) + return resolvable.get_commit(sha) + + gh_repo = mock.create_autospec(github.Repository.Repository, instance=True) + gh_repo.get_commit.side_effect = get_commit + assert github_logins(gh_repo, author_to_sha) == ["a-gh", "c-gh"] + + +def test_github_logins_skips_commits_with_no_github_author(): + """Unlinked email -> gh_commit.author is None; must not emit an entry.""" + author_to_sha = {"a@example.com": "sha1", "ghost@example.com": "sha2"} + gh_repo = _gh_repo({"sha1": "a-gh", "sha2": None}) + assert github_logins(gh_repo, author_to_sha) == ["a-gh"]