diff --git a/.github/workflows/update-pull-request.yml b/.github/workflows/update-pull-request.yml index 98ebcbd..a0a2d00 100644 --- a/.github/workflows/update-pull-request.yml +++ b/.github/workflows/update-pull-request.yml @@ -17,9 +17,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - with: - # Set fetch-depth to 0 to fetch all commit history (necessary for compiling pull request description). - fetch-depth: 0 - name: Install release note compiler run: pip install git+https://github.com/octue/conventional-commits - name: Compile new pull request description diff --git a/README.md b/README.md index 5c22a52..5a165ec 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,6 @@ A command-line tool that compiles release notes and pull request descriptions fr stopping at the specified stop point. The stop point can be one of: * The first commit of the pull request - `PULL_REQUEST_START` * The last semantically-versioned release tagged in the git history relative to the checked-out branch - `LAST_RELEASE` -* The last merged pull request in the git history relative to the checked-out branch - `LAST_PULL_REQUEST` If breaking changes are indicated in any of the commit messages' bodies via `BREAKING CHANGE` or `BREAKING-CHANGE`, these commits are marked and a warning is added to the top of the release notes. @@ -209,10 +208,10 @@ Note that these comment lines are invisible in rendered markdown. ```shell usage: compile-release-notes [-h] [--pull-request-url PULL_REQUEST_URL] [--api-token API_TOKEN] [--header HEADER] [--list-item-symbol LIST_ITEM_SYMBOL] [--no-link-to-pull-request] - {LAST_RELEASE,LAST_PULL_REQUEST,PULL_REQUEST_START} + {LAST_RELEASE,PULL_REQUEST_START} positional arguments: - {LAST_RELEASE,LAST_PULL_REQUEST,PULL_REQUEST_START} + {LAST_RELEASE,PULL_REQUEST_START} The point in the git history to stop compiling commits into the release notes. optional arguments: diff --git a/conventional_commits/check_commit_message.py b/conventional_commits/check_commit_message.py index e5f2337..13b46c4 100755 --- a/conventional_commits/check_commit_message.py +++ b/conventional_commits/check_commit_message.py @@ -65,7 +65,7 @@ def __init__( self, allowed_commit_codes=None, maximum_header_length=72, - valid_header_ending_pattern=r"[A-Za-z\d]", + valid_header_ending_pattern=r"[A-Za-z\d\"\']", require_body=False, maximum_body_line_length=72, ): diff --git a/conventional_commits/compile_release_notes.py b/conventional_commits/compile_release_notes.py index 3783460..a67d7b9 100644 --- a/conventional_commits/compile_release_notes.py +++ b/conventional_commits/compile_release_notes.py @@ -14,15 +14,13 @@ LAST_RELEASE = "LAST_RELEASE" -LAST_PULL_REQUEST = "LAST_PULL_REQUEST" PULL_REQUEST_START = "PULL_REQUEST_START" -STOP_POINTS = (LAST_RELEASE, LAST_PULL_REQUEST, PULL_REQUEST_START) +STOP_POINTS = (LAST_RELEASE, PULL_REQUEST_START) BREAKING_CHANGE_INDICATOR = "**BREAKING CHANGE:** " COMMIT_REF_MERGE_PATTERN = re.compile(r"Merge [0-9a-f]+ into [0-9a-f]+") SEMANTIC_VERSION_PATTERN = re.compile(r"tag: (\d+\.\d+\.\d+)") -PULL_REQUEST_INDICATOR = "Merge pull request #" COMMIT_CODES_TO_HEADINGS_MAPPING = { "FEA": "### New features", @@ -54,7 +52,7 @@ class ReleaseNotesCompiler: comment lines `` and `` will be replaced - anything outside of this will appear in the new release notes. - :param str stop_point: the point in the git history up to which commit messages should be used - should be either "LAST_RELEASE" or "LAST_PULL_REQUEST" + :param str stop_point: the point in the git history up to which commit messages should be used - should be either "LAST_RELEASE" or "PULL_REQUEST_START" :param str|None pull_request_url: GitHub API URL for the pull request - this can be accessed in a GitHub workflow as ${{ github.event.pull_request.url }} :param str|None api_token: GitHub API token - this can be accessed in a GitHub workflow as ${{ secrets.GITHUB_TOKEN }} :param str header: the header to put above the autogenerated release notes, including any markdown styling (defaults to "## Contents") @@ -93,12 +91,6 @@ def __init__( self.commit_codes_to_headings_mapping = commit_codes_to_headings_mapping or COMMIT_CODES_TO_HEADINGS_MAPPING self.include_link_to_pull_request = include_link_to_pull_request - if self.stop_point == PULL_REQUEST_START: - if self.current_pull_request is not None: - self.base_branch = self.current_pull_request["base"]["ref"] - else: - self.stop_point = LAST_RELEASE - logger.info(f"Using {self.stop_point!r} stop point.") def compile_release_notes(self): @@ -117,8 +109,11 @@ def compile_release_notes(self): if self.previous_notes and SKIP_INDICATOR in self.previous_notes: return self.previous_notes - git_log = self._get_git_log() - parsed_commits, unparsed_commits = self._parse_commit_messages(git_log) + if self.current_pull_request: + parsed_commits, unparsed_commits = self._parse_commit_messages_from_github() + else: + parsed_commits, unparsed_commits = self._parse_commit_messages_from_git_log() + categorised_commit_messages = self._categorise_commit_messages(parsed_commits, unparsed_commits) autogenerated_release_notes = self._build_release_notes(categorised_commit_messages) @@ -143,7 +138,7 @@ def _get_current_pull_request(self, pull_request_url, api_token): :param str pull_request_url: the GitHub API URL for the pull request :param str|None api_token: GitHub API token - :return dict: + :return dict|None: """ if api_token is None: headers = {} @@ -153,13 +148,18 @@ def _get_current_pull_request(self, pull_request_url, api_token): response = requests.get(pull_request_url, headers=headers) if response.status_code == 200: - return response.json() + pull_request = response.json() + pull_request["commits"] = requests.get(pull_request["commits_url"], headers=headers).json() + return pull_request logger.warning( f"Pull request could not be accessed; resorting to using {LAST_RELEASE} stop point.\n" f"{response.status_code}: {response.text}." ) + self.stop_point = LAST_RELEASE + return None + def _get_git_log(self): """Get the one-line decorated git log formatted in the pattern of "hash|§header|§body|§decoration@@@". @@ -171,30 +171,27 @@ def _get_git_log(self): * The specific characters used for the delimiters have been chosen so that they are very uncommon to reduce delimiting errors - :return str: + :return list(str): """ return ( subprocess.run(["git", "log", "--pretty=format:%h|§%s|§%b|§%d@@@"], capture_output=True) .stdout.strip() .decode() - ) + ).split("@@@") - def _parse_commit_messages(self, formatted_one_line_git_log): - """Parse commit messages from the git log (formatted using `--pretty=format:%s|%d`) until the stop point is - reached. The parsed commit messages are returned separately to any that fail to parse. + def _parse_commit_messages_from_git_log(self): + """Parse commit messages from the git log (formatted using `--pretty=format:%h|§%s|§%b|§%d@@@`) until the stop + point is reached. The parsed commit messages are returned separately to any that fail to parse. - :param str formatted_one_line_git_log: :return list(tuple), list(str): """ parsed_commits = [] unparsed_commits = [] - commits = formatted_one_line_git_log.split("@@@") - - for commit in commits: + for commit in self._get_git_log(): hash, header, body, decoration = commit.split("|§") - if self._is_stop_point(header, decoration): + if "tag" in decoration and bool(SEMANTIC_VERSION_PATTERN.search(decoration)): break # A colon separating the commit code from the commit header is required - keep commit messages that @@ -210,27 +207,39 @@ def _parse_commit_messages(self, formatted_one_line_git_log): code, *header = header.split(":") header = ":".join(header) - parsed_commits.append((code.strip(), header.strip(), body.strip(), decoration.strip())) + parsed_commits.append((code.strip(), header.strip(), body.strip())) return parsed_commits, unparsed_commits - def _is_stop_point(self, message, decoration): - """Check if this commit header is the stop point for collecting commits for the release notes. + def _parse_commit_messages_from_github(self): + """Parse commit messages from the GitHub pull request. The parsed commit messages are returned separately to + any that fail to parse. - :param str message: - :param str decoration: - :return bool: + :return list(tuple), list(str): """ - if self.stop_point == PULL_REQUEST_START: - if self.base_branch in decoration: - return True + parsed_commits = [] + unparsed_commits = [] + + for commit in self.current_pull_request["commits"]: + header, *body = commit["commit"]["message"].split("\n") + body = "\n".join(body) + + # A colon separating the commit code from the commit header is required - keep commit messages that + # don't conform to this but put them into an unparsed category. Ignore commits that are merges of one + # commit ref into another (GitHub Actions produces these - they don't appear in the actual history of + # the branch so can be safely ignored when making release notes). + if ":" not in header: + if not COMMIT_REF_MERGE_PATTERN.search(header): + unparsed_commits.append(header.strip()) + continue + + # Allow commit headers with extra colons. + code, *header = header.split(":") + header = ":".join(header) - elif self.stop_point == LAST_RELEASE: - if "tag" in decoration: - return bool(SEMANTIC_VERSION_PATTERN.search(decoration)) + parsed_commits.append((code.strip(), header.strip(), body.strip())) - elif self.stop_point == LAST_PULL_REQUEST: - return PULL_REQUEST_INDICATOR in message + return parsed_commits, unparsed_commits def _categorise_commit_messages(self, parsed_commits, unparsed_commits): """Categorise the commit messages into headed sections. Unparsed commits are put under an "uncategorised" @@ -243,7 +252,7 @@ def _categorise_commit_messages(self, parsed_commits, unparsed_commits): release_notes = {heading: [] for heading in self.commit_codes_to_headings_mapping.values()} release_notes[BREAKING_CHANGE_COUNT_KEY] = 0 - for code, header, body, _ in parsed_commits: + for code, header, body in parsed_commits: try: if any(indicator in body for indicator in CONVENTIONAL_COMMIT_BREAKING_CHANGE_INDICATORS): commit_note = BREAKING_CHANGE_INDICATOR + header diff --git a/examples/release_notes_compiler/workflow.yml b/examples/release_notes_compiler/workflow.yml index a8fbe08..68b3e6f 100644 --- a/examples/release_notes_compiler/workflow.yml +++ b/examples/release_notes_compiler/workflow.yml @@ -11,9 +11,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - with: - # Set fetch-depth to 0 to fetch all commit history (necessary for compiling pull request description). - fetch-depth: 0 - name: Install release note compiler run: pip install git+https://github.com/octue/conventional-commits - name: Compile new pull request description diff --git a/setup.cfg b/setup.cfg index 3b839b9..b447a38 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = conventional_commits -version = 0.4.1 +version = 0.5.0 description = A pre-commit hook, semantic version checker, and release note compiler for facilitating continuous deployment via Conventional Commits. long_description = file: README.md long_description_content_type = text/markdown diff --git a/tests/test_check_commit_message.py b/tests/test_check_commit_message.py index 2948cd8..e198fac 100644 --- a/tests/test_check_commit_message.py +++ b/tests/test_check_commit_message.py @@ -44,6 +44,10 @@ def test_non_blank_header_separator_line_raises_error(self): with self.assertRaises(ValueError): ConventionalCommitMessageChecker().check_commit_message(["FIX: Fix this bug", "Body started too early."]) + def test_valid_header_ending(self): + """Test that a commit message with a valid header ending is ok.""" + ConventionalCommitMessageChecker().check_commit_message(['REV: Reverts "FIX: Fix a bug"']) + def test_with_body_when_body_not_required(self): """Test that a commit message with a valid header and body when a body is not required is ok.""" ConventionalCommitMessageChecker().check_commit_message(["FIX: Fix this bug", "", "This is the body."]) diff --git a/tests/test_compile_release_notes.py b/tests/test_compile_release_notes.py index f67b340..5cfa6fd 100644 --- a/tests/test_compile_release_notes.py +++ b/tests/test_compile_release_notes.py @@ -4,24 +4,38 @@ from conventional_commits.compile_release_notes import ReleaseNotesCompiler, main -MOCK_GIT_LOG = "@@@\n".join( - [ - "3e7dc54|§REF: Merge commit message checker modules|§|§ (HEAD -> refactor/test-release-notes-generator, origin/refactor/test-release-notes-generator)", - "fabd2ab|§MRG: Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components|§|§", - "ef77729|§CHO: Remove hook installation from branch|§|§ (tag: 0.0.3, origin/main, origin/HEAD, main)", - "b043bc8|§ENH: Support getting versions from poetry and npm|§|§", - "27dcef0|§FIX: Fix semantic version script; add missing config|§|§", - ] -) - -EXPECTED_LAST_PULL_REQUEST_RELEASE_NOTES_WITH_NON_GENERATED_SECTION = "\n".join( +MOCK_GIT_LOG = [ + "3e7dc54|§REF: Merge commit message checker modules|§|§ (HEAD -> refactor/test-release-notes-generator, origin/refactor/test-release-notes-generator)", + "fabd2ab|§MRG: Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components|§|§", + "ef77729|§CHO: Remove hook installation from branch|§|§ (tag: 0.0.3, origin/main, origin/HEAD, main)", + "b043bc8|§ENH: Support getting versions from poetry and npm|§|§", + "27dcef0|§FIX: Fix semantic version script; add missing config|§|§", +] + +MOCK_PULL_REQUEST_COMMITS = [ + { + "commit": { + "message": "MRG: Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components" + } + }, + {"commit": {"message": "ENH: Support getting versions from poetry and npm"}}, + {"commit": {"message": "FIX: Fix semantic version script; add missing config"}}, +] + +EXPECTED_PULL_REQUEST_START_RELEASE_NOTES_WITH_NON_GENERATED_SECTION = "\n".join( [ "BLAH BLAH BLAH", "", "## Contents", "", - "### Refactoring", - "- Merge commit message checker modules", + "### Enhancements", + "- Support getting versions from poetry and npm", + "", + "### Fixes", + "- Fix semantic version script; add missing config", + "", + "### Other", + "- Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components", "", "", "YUM YUM YUM", @@ -41,32 +55,32 @@ def test_unsupported_stop_point_results_in_error(self): with self.assertRaises(ValueError): ReleaseNotesCompiler(stop_point="blah", pull_request_url="") - def test_skip_release_notes_auto_generations(self): + def test_skip_release_notes_auto_generation(self): """Test that release notes autogeneration is skipped if the skip indicator is present in the previous notes.""" previous_notes = ( "BLAH BLAH BLAH\n\nYUM YUM YUM" "" ) - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": previous_notes}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": previous_notes, "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() self.assertEqual(release_notes, previous_notes) def test_last_release_stop_point(self): """Test generating release notes that stop at the last release.""" with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": ""}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_RELEASE", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + release_notes = ReleaseNotesCompiler( + stop_point="LAST_RELEASE", + include_link_to_pull_request=False, + ).compile_release_notes() expected = "\n".join( [ @@ -85,89 +99,17 @@ def test_last_release_stop_point(self): self.assertEqual(release_notes, expected) - def test_last_pull_request_stop_point(self): - """Test generating release notes that stop at the last pull request merge.""" - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": ""}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() - - expected = "\n".join( - [ - "", - "## Contents", - "", - "### Refactoring", - "- Merge commit message checker modules", - "", - "", - ] - ) - - self.assertEqual(release_notes, expected) - - def test_branch_point_stop_point(self): - """Test generating release notes that stop at the last branch point.""" - mock_git_log = "@@@\n".join( - [ - "27dcef0|§TST: Improve presentation of long strings|§|§ (fix/fix-other-release-notes-stop-point-bug)", - "358ffd5|§REF: Move stop point checking into separate method|§|§", - "44927c6|§FIX: Fix LAST_PULL_REQUEST stop point bug|§|§", - "7cdc980|§FIX: Ensure uncategorised commits are not lost|§|§ (fix/allow-extra-colons-in-commit-message)", - "741bb8d|§OPS: Increase version to 0.0.11|§|§ (tag: 0.0.11, my-base-branch)", - "27092a4|§FIX: Allow extra colons in commit headers in release notes compiler|§|§", - "6dcdc41|§MRG: Merge pull request #17 from octue/fix/fix-release-notes-stop-point-bug|§|§ (tag: 0.0.10)", - ] - ) - - with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): - with patch( - self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": "", "base": {"ref": "my-base-branch"}} - ): - release_notes = ReleaseNotesCompiler( - stop_point="PULL_REQUEST_START", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() - - expected = "\n".join( - [ - "", - "## Contents", - "", - "### Fixes", - "- Fix LAST_PULL_REQUEST stop point bug", - "- Ensure uncategorised commits are not lost", - "", - "### Refactoring", - "- Move stop point checking into separate method", - "", - "### Testing", - "- Improve presentation of long strings", - "", - "", - ] - ) - - self.assertEqual(release_notes, expected) - - def test_compiler_reverts_to_last_release_stop_point_if_branch_point_is_not_found(self): - """Ensure the release note compiler reverts to the LAST_RELEASE stop point if no pull request URL is provided.""" - release_note_compiler = ReleaseNotesCompiler(stop_point="PULL_REQUEST_START") - self.assertEqual(release_note_compiler.stop_point, "LAST_RELEASE") - def test_with_previous_release_notes_missing_autogeneration_markers(self): """Test that previous release notes are not overwritten when the autogeneration markers are missing.""" - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": "BLAH BLAH BLAH"}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": "BLAH BLAH BLAH", "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() expected = "\n".join( [ @@ -175,8 +117,14 @@ def test_with_previous_release_notes_missing_autogeneration_markers(self): "", "## Contents", "", - "### Refactoring", - "- Merge commit message checker modules", + "### Enhancements", + "- Support getting versions from poetry and npm", + "", + "### Fixes", + "- Fix semantic version script; add missing config", + "", + "### Other", + "- Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components", "", "", ] @@ -192,15 +140,17 @@ def test_with_previous_release_notes_with_empty_autogenerated_section(self): "BLAH BLAH BLAH\n\nYUM YUM YUM" ) - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": previous_notes}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": previous_notes, "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() - self.assertEqual(release_notes, EXPECTED_LAST_PULL_REQUEST_RELEASE_NOTES_WITH_NON_GENERATED_SECTION) + self.assertEqual(release_notes, EXPECTED_PULL_REQUEST_START_RELEASE_NOTES_WITH_NON_GENERATED_SECTION) def test_with_previous_release_notes_with_other_text_on_autogeneration_markers_lines(self): """Test that text outside but on the same line as the autogeneration markers in previous release notes is not @@ -210,15 +160,17 @@ def test_with_previous_release_notes_with_other_text_on_autogeneration_markers_l "BLAH BLAH BLAH\nYUM YUM YUM" ) - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": previous_notes}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": previous_notes, "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() - self.assertEqual(release_notes, EXPECTED_LAST_PULL_REQUEST_RELEASE_NOTES_WITH_NON_GENERATED_SECTION) + self.assertEqual(release_notes, EXPECTED_PULL_REQUEST_START_RELEASE_NOTES_WITH_NON_GENERATED_SECTION) def test_autogenerated_section_gets_overwritten(self): """Test that text enclosed by the autogeneration markers is overwritten.""" @@ -226,21 +178,29 @@ def test_autogenerated_section_gets_overwritten(self): "\nBAM BAM BAM\nWAM WAM WAM\n" ) - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": previous_notes}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": previous_notes, "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() expected = "\n".join( [ "", "## Contents", "", - "### Refactoring", - "- Merge commit message checker modules", + "### Enhancements", + "- Support getting versions from poetry and npm", + "", + "### Fixes", + "- Fix semantic version script; add missing config", + "", + "### Other", + "- Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components", "", "", ] @@ -255,24 +215,24 @@ def test_autogenerated_section_gets_overwritten_but_text_outside_does_not(self): "YUM YUM" ) - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": previous_notes}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": previous_notes, "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() - self.assertEqual(release_notes, EXPECTED_LAST_PULL_REQUEST_RELEASE_NOTES_WITH_NON_GENERATED_SECTION) + self.assertEqual(release_notes, EXPECTED_PULL_REQUEST_START_RELEASE_NOTES_WITH_NON_GENERATED_SECTION) def test_commit_messages_in_non_standard_format_are_left_uncategorised(self): """Test that commit messages in a non-standard format are put under an uncategorised heading.""" - mock_git_log = "fabd2ab|§This is not in the right format|§|§@@@\n27dcef0|§FIX: Fix a bug|§|§" + mock_git_log = ["fabd2ab|§This is not in the right format|§|§", "27dcef0|§FIX: Fix a bug|§|§"] with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", include_link_to_pull_request=False - ).compile_release_notes() + release_notes = ReleaseNotesCompiler(stop_point="LAST_RELEASE").compile_release_notes() expected = "\n".join( [ @@ -293,15 +253,10 @@ def test_commit_messages_in_non_standard_format_are_left_uncategorised(self): def test_commit_messages_with_unrecognised_commit_codes_are_categorised_as_other(self): """Test that commit messages with an unrecognised commit code are categorised under "other".""" - mock_git_log = "27dcef0|§BAM: An unrecognised commit code|§|§@@@\nfabd2ab|§FIX: Fix a bug|§|§" + mock_git_log = ["27dcef0|§BAM: An unrecognised commit code|§|§", "fabd2ab|§FIX: Fix a bug|§|§"] with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": ""}): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + release_notes = ReleaseNotesCompiler(stop_point="LAST_RELEASE").compile_release_notes() expected = "\n".join( [ @@ -329,25 +284,29 @@ def test_updating_release_notes_works_and_does_not_add_extra_newlines_after_auto "BLAH BLAH BLAH\nYUM YUM YUM" ) - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": previous_notes}): - release_notes_1 = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": previous_notes, "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes_1 = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() # Add a new commit to the git log. - updated_mock_git_log = "fabd2ab|§FIX: Fix a bug|§|§@@@\n" + MOCK_GIT_LOG + updated_pull_request_commits = [{"commit": {"message": "FIX: Fix a bug"}}] + MOCK_PULL_REQUEST_COMMITS # Run the compiler on the new git log to update the previous set of release notes. - with patch(self.GIT_LOG_METHOD_PATH, return_value=updated_mock_git_log): - with patch(self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": release_notes_1}): - release_notes_2 = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url=self.MOCK_PULL_REQUEST_URL, - include_link_to_pull_request=False, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": release_notes_1, "commits": updated_pull_request_commits}, + ): + release_notes_2 = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=self.MOCK_PULL_REQUEST_URL, + include_link_to_pull_request=False, + ).compile_release_notes() expected = "\n".join( [ @@ -355,11 +314,15 @@ def test_updating_release_notes_works_and_does_not_add_extra_newlines_after_auto "", "## Contents", "", + "### Enhancements", + "- Support getting versions from poetry and npm", + "", "### Fixes", "- Fix a bug", + "- Fix semantic version script; add missing config", "", - "### Refactoring", - "- Merge commit message checker modules", + "### Other", + "- Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components", "", "", "YUM YUM YUM", @@ -373,31 +336,29 @@ def test_last_release_stop_point_is_respected_even_if_tagged_commit_has_no_commi made on GitHub that isn't subject to the Conventional Commits pre-commit check that we use locally) has no commit code. """ - mock_git_log = "@@@\n".join( - [ - "3e7dc54|§OPS: Update mkver.conf to correct pattern|§|§ (HEAD -> develop/issue-wq-521-turbine-prevailing-direction, origin/develop/issue-wq-521-turbine-prevailing-direction)", - "fabd2ab|§OPS: Update conventional commit version in python-ci|§|§", - "ef77729|§DOC: update readme for new pre-commit ops|§|§", - "b043bc8|§OPS: update pull request workflow updated|§|§", - "27dcef0|§DEP: Version bump to 0.1.0|§|§", - "358ffd5|§OPS: Delete repo issue and pr template|§|§", - "44927c6|§OPS: Precommit config updated to use conventional commit|§|§", - "6589a8e|§CHO: Add _mocks file with the mocked classes|§|§", - "7cdc980|§ENH: Windmap file inputs format now supports space separated headers|§|§", - "741bb8d|§ENH: Add prevailing_wind_direction to twine file output schema|§|§", - "27092a4|§ENH: Handle edge case when wind dir not in the data and WD_ in the time series file. Add tests for checking for None prevailing wind directions|§|§", - "6dcdc41|§ENH: Mast prevailing wind direction happens when each mast time series file is imported instead of in every turbine|§|§", - "b69e7e1|§FEA: Add turbine prevailing wind direction to the result as a property of turbine|§|§", - "0b20f41|§CHO: Clear up the comments and add logging|§|§", - "05ec990|§CHORE: Remove unused test|§|§", - "e506bb4|§ENH: The prevailing wind calculation now de-seasons the count of the bins and determines the prevailing wind direction. Slow as *|§|§", - "9c125ab|§FEAT: Added a utility module which calculates the prevailing wind direction given a wind direction time series|§|§", - "3d981b6|§Merge pull request #103 from windpioneers/release/0.0.11|§|§ (tag: 0.0.11, origin/main, main)", - "9b88bc6|§ENH: Added an assert to bring the coverage up|§|§", - "17d8de1|§ENH: Remove a json turbine data importer, we are not going to be importing a turbine from json 'cos we use pcu for that|§|§", - "d242271|§DEP: Update pcu version to 0.0.6|§|§", - ] - ) + mock_git_log = [ + "3e7dc54|§OPS: Update mkver.conf to correct pattern|§|§ (HEAD -> develop/issue-wq-521-turbine-prevailing-direction, origin/develop/issue-wq-521-turbine-prevailing-direction)", + "fabd2ab|§OPS: Update conventional commit version in python-ci|§|§", + "ef77729|§DOC: update readme for new pre-commit ops|§|§", + "b043bc8|§OPS: update pull request workflow updated|§|§", + "27dcef0|§DEP: Version bump to 0.1.0|§|§", + "358ffd5|§OPS: Delete repo issue and pr template|§|§", + "44927c6|§OPS: Precommit config updated to use conventional commit|§|§", + "6589a8e|§CHO: Add _mocks file with the mocked classes|§|§", + "7cdc980|§ENH: Windmap file inputs format now supports space separated headers|§|§", + "741bb8d|§ENH: Add prevailing_wind_direction to twine file output schema|§|§", + "27092a4|§ENH: Handle edge case when wind dir not in the data and WD_ in the time series file. Add tests for checking for None prevailing wind directions|§|§", + "6dcdc41|§ENH: Mast prevailing wind direction happens when each mast time series file is imported instead of in every turbine|§|§", + "b69e7e1|§FEA: Add turbine prevailing wind direction to the result as a property of turbine|§|§", + "0b20f41|§CHO: Clear up the comments and add logging|§|§", + "05ec990|§CHORE: Remove unused test|§|§", + "e506bb4|§ENH: The prevailing wind calculation now de-seasons the count of the bins and determines the prevailing wind direction. Slow as *|§|§", + "9c125ab|§FEAT: Added a utility module which calculates the prevailing wind direction given a wind direction time series|§|§", + "3d981b6|§Merge pull request #103 from windpioneers/release/0.0.11|§|§ (tag: 0.0.11, origin/main, main)", + "9b88bc6|§ENH: Added an assert to bring the coverage up|§|§", + "17d8de1|§ENH: Remove a json turbine data importer, we are not going to be importing a turbine from json 'cos we use pcu for that|§|§", + "d242271|§DEP: Update pcu version to 0.0.6|§|§", + ] with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): release_notes = ReleaseNotesCompiler( @@ -444,55 +405,14 @@ def test_last_release_stop_point_is_respected_even_if_tagged_commit_has_no_commi self.assertEqual(release_notes, expected_release_notes) - def test_last_pull_request_stop_point_is_respected_even_if_tagged_commit_has_no_commit_code(self): - """Test that the `LAST_PULL_REQUEST` stop point is still respected even if the tagged commit (usually a merge - commit made on GitHub that isn't subject to the Conventional Commits pre-commit check that we use locally) has - no commit code. - """ - mock_git_log = "@@@\n".join( - [ - "3e7dc54|§OPS: Update mkver.conf to correct pattern|§|§ (HEAD -> develop/issue-wq-521-turbine-prevailing)", - "fabd2ab|§OPS: update pull request workflow updated|§|§", - "ef77729|§DEP: Version bump to 0.1.0|§|§", - "b043bc8|§Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components|§|§", - "27dcef0|§OPS: Precommit config updated to use conventional commit|§|§", - "358ffd5|§CHO: Add _mocks file with the mocked classes|§|§", - ] - ) - - with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", include_link_to_pull_request=False - ).compile_release_notes() - - expected_release_notes = "\n".join( - [ - "", - "## Contents", - "", - "### Operations", - "- Update mkver.conf to correct pattern", - "- update pull request workflow updated", - "", - "### Dependencies", - "- Version bump to 0.1.0", - "", - "", - ] - ) - - self.assertEqual(release_notes, expected_release_notes) - def test_commit_message_with_extra_colons_are_still_categorised(self): """Test that commit headers containing extra colons in addition to the colon splitting the commit code from the rest of the commit header are still categorised correctly. """ - mock_git_log = "@@@\n".join( - [ - "3e7dc54|§OPS: My message: something|§|§", - "fabd2ab|§OPS: Update conventional commit version in python-ci|§|§", - ] - ) + mock_git_log = [ + "3e7dc54|§OPS: My message: something|§|§", + "fabd2ab|§OPS: Update conventional commit version in python-ci|§|§", + ] with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): release_notes = ReleaseNotesCompiler( @@ -515,13 +435,11 @@ def test_commit_message_with_extra_colons_are_still_categorised(self): def test_commit_hash_merges_are_ignored(self): """Ensure commit messages that are just a merge of a commit ref into another commit ref are ignored.""" - mock_git_log = "@@@\n".join( - [ - "3e7dc54|§OPS: My message: something|§|§", - "fabd2ab|§Merge ef777290453f11b7519dbd3410b01d34d2e13566 into b043bc85cf558f1706188fafe9676ecd0642ab5a|§|§", - "ef77729|§OPS: Update conventional commit version in python-ci|§|§", - ] - ) + mock_git_log = [ + "3e7dc54|§OPS: My message: something|§|§", + "fabd2ab|§Merge ef777290453f11b7519dbd3410b01d34d2e13566 into b043bc85cf558f1706188fafe9676ecd0642ab5a|§|§", + "ef77729|§OPS: Update conventional commit version in python-ci|§|§", + ] with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): release_notes = ReleaseNotesCompiler( @@ -546,7 +464,7 @@ def test_single_breaking_change_is_indicated(self): """Test that a single breaking change is indicated in the release notes at the top and next to the categorised commit message. """ - mock_git_log = "fabd2ab|§ENH: Make big change|§BREAKING CHANGE: blah blah blah|§@@@\n" + MOCK_GIT_LOG + mock_git_log = ["fabd2ab|§ENH: Make big change|§BREAKING CHANGE: blah blah blah|§"] + MOCK_GIT_LOG with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): release_notes = ReleaseNotesCompiler( @@ -579,11 +497,11 @@ def test_multiple_breaking_changes_are_indicated(self): """Test that multiple breaking changes are indicated in the release notes at the top and next to the categorised commit message. """ - mock_git_log = ( - "fabd2ab|§ENH: Make big change|§BREAKING-CHANGE: blah blah blah|§@@@\n" - "fabd2ab|§FIX: Make breaking fix|§BREAKING CHANGE: blob|§@@@\n" - "fabd2ab|§REF: Change interface|§BREAKING-CHANGE: glob|§@@@\n" - ) + MOCK_GIT_LOG + mock_git_log = [ + "fabd2ab|§ENH: Make big change|§BREAKING-CHANGE: blah blah blah|§", + "fabd2ab|§FIX: Make breaking fix|§BREAKING CHANGE: blob|§", + "fabd2ab|§REF: Change interface|§BREAKING-CHANGE: glob|§", + ] + MOCK_GIT_LOG with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): release_notes = ReleaseNotesCompiler( @@ -618,7 +536,7 @@ def test_multiple_breaking_changes_are_indicated(self): def test_commit_messages_with_multi_line_bodies(self): """Test that commits with multi-line bodies work with the release notes compiler.""" - mock_git_log = "fabd2ab|§ENH: Blah blah|§This is the body.\n Here is another body line|§@@@\n" + MOCK_GIT_LOG + mock_git_log = ["fabd2ab|§ENH: Blah blah|§This is the body.\n Here is another body line|§"] + MOCK_GIT_LOG with patch(self.GIT_LOG_METHOD_PATH, return_value=mock_git_log): release_notes = ReleaseNotesCompiler( @@ -649,23 +567,29 @@ def test_include_link_to_pull_request(self): """Test that the HTML URL of the pull request is included in the release notes if requested.""" html_url = "https://github.com/octue/conventional-commits/pull/40" - with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): - with patch( - self.GET_CURRENT_PULL_REQUEST_PATH, return_value={"body": "", "number": 40, "html_url": html_url} - ): - release_notes = ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", - pull_request_url="https://api.github.com/repos/octue/conventional-commits/pulls/40", - include_link_to_pull_request=True, - ).compile_release_notes() + with patch( + self.GET_CURRENT_PULL_REQUEST_PATH, + return_value={"body": "", "number": 40, "html_url": html_url, "commits": MOCK_PULL_REQUEST_COMMITS}, + ): + release_notes = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url="https://api.github.com/repos/octue/conventional-commits/pulls/40", + include_link_to_pull_request=True, + ).compile_release_notes() expected = "\n".join( [ "", "## Contents ([#40](https://github.com/octue/conventional-commits/pull/40))", "", - "### Refactoring", - "- Merge commit message checker modules", + "### Enhancements", + "- Support getting versions from poetry and npm", + "", + "### Fixes", + "- Fix semantic version script; add missing config", + "", + "### Other", + "- Merge pull request #3 from octue/feature/add-other-conventional-commit-ci-components", "", "", ] @@ -673,21 +597,47 @@ def test_include_link_to_pull_request(self): self.assertEqual(release_notes, expected) - def test_warning_raised_if_pull_request_is_not_accessible(self): - """Test that a warning is logged and the LAST_PULL_REQUEST stop point is used if the given pull request isn't + def test_last_release_stop_point_used_if_pull_request_is_not_accessible(self): + """Test that a warning is logged and the LAST_RELEASE stop point is used if the given pull request isn't accessible. """ with patch(self.GIT_LOG_METHOD_PATH, return_value=MOCK_GIT_LOG): with patch("requests.get", return_value=Mock(status_code=404)): with self.assertLogs() as logging_context: - ReleaseNotesCompiler( - stop_point="LAST_PULL_REQUEST", + compiler = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", pull_request_url="https://api.github.com/repos/octue/conventional-commits/pulls/40", include_link_to_pull_request=True, - ).compile_release_notes() + ) self.assertEqual(logging_context.records[0].levelname, "WARNING") - self.assertEqual(logging_context.records[1].message, "Using 'LAST_PULL_REQUEST' stop point.") + self.assertEqual(logging_context.records[1].message, "Using 'LAST_RELEASE' stop point.") + + compiler.compile_release_notes() + + self.assertEqual(compiler.stop_point, "LAST_RELEASE") + + def test_get_pull_request_with_api_token(self): + """Test that a pull request can be accessed using a GitHub API token.""" + pull_request_url = "https://api.github.com/repos/octue/conventional-commits/pulls/40" + + with patch( + "requests.get", + return_value=Mock(status_code=200, json=lambda: {"body": "blah", "commits_url": "https://url/to/commits"}), + ) as mock_get: + compiler = ReleaseNotesCompiler( + stop_point="PULL_REQUEST_START", + pull_request_url=pull_request_url, + api_token="blah", + ) + + # Check the pull request URL has been requested. + self.assertEqual(mock_get.call_args_list[0].args, (pull_request_url,)) + self.assertEqual(mock_get.call_args_list[0].kwargs, {"headers": {"Authorization": "token blah"}}) + + # Check the commits URL has been requested and the returned JSON added to the pull request JSON. + self.assertEqual(mock_get.call_args_list[1].args, ("https://url/to/commits",)) + self.assertIn("commits", compiler.current_pull_request) class TestMain(unittest.TestCase):