From 6ccc8ec4f9de64d6d57d0e8dd61164b4e014699c Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Thu, 21 Nov 2024 09:21:23 +0100 Subject: [PATCH 1/6] - Fix typo in README.md. - Added hint how to get values. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ee03f94..78ed8cf 100644 --- a/README.md +++ b/README.md @@ -85,8 +85,8 @@ See the default action step definition: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: - github-repository: "{ org }/{ repo }" - pr-numer: 109 + github-repository: "{ org }/{ repo }" # ${{ github.repository }} + pr-number: 109 # ${{ github.event.number }} location: "body" title: "[Rr]elease [Nn]otes:" skip-labels: "skip-release-notes,no-release-notes" From b1c002765ece9f9a91583c54213d4389d661d032 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 22 Nov 2024 10:07:01 +0100 Subject: [PATCH 2/6] - Removed fail on error input. - Removed valid output. --- README.md | 23 ------- action.yml | 10 --- .../release_notes_presence_check_action.py | 63 ++++++------------ ...est_release_notes_presence_check_action.py | 65 ------------------- 4 files changed, 21 insertions(+), 140 deletions(-) diff --git a/README.md b/README.md index 78ed8cf..ec0b397 100644 --- a/README.md +++ b/README.md @@ -57,17 +57,6 @@ This action is designed to help maintainers and contributors ensure that release - **Required**: No - **Default**: `` -### `fails-on-error` -- **Description**: Whether the action should fail if an error occurs. -- **Required**: No -- **Default**: `true` - -## Outputs - -### `valid` -- **Description**: Whether the release notes are present. -- **Value**: `true` or `false` - ## Usage ### Adding the Action to Your Workflow @@ -90,7 +79,6 @@ See the default action step definition: location: "body" title: "[Rr]elease [Nn]otes:" skip-labels: "skip-release-notes,no-release-notes" - fails-on-error: "false" ``` ## Running Static Code Analysis @@ -218,20 +206,9 @@ export INPUT_GITHUB_REPOSITORY="AbsaOSS/generate-release-notes" export INPUT_LOCATION="body" export INPUT_TITLE="[Rr]elease notes:" export INPUT_SKIP_LABELS="skip-release-notes,another-skip-label" -export INPUT_FAILS_ON_ERROR="true" -export GITHUB_OUTPUT="output.txt" # File to capture outputs - -# Remove existing output file if it exists -if [ -f "$GITHUB_OUTPUT" ]; then - rm "$GITHUB_OUTPUT" -fi # Run the main script python main.py - -# Display the outputs -echo "Action Outputs:" -cat "$GITHUB_OUTPUT" ``` ## Contribution Guidelines diff --git a/action.yml b/action.yml index 045d54b..feaed25 100644 --- a/action.yml +++ b/action.yml @@ -35,15 +35,6 @@ inputs: description: 'A comma-separated list of labels that will cause the action to skip the check.' required: false default: '' - fails-on-error: - description: 'Set to "true" to fail the action if validation errors are found.' - required: false - default: 'true' - -outputs: - valid: - description: 'Indicates whether the release notes are present in the PR.' - value: ${{ steps.release-notes-presence-check.outputs.valid }} branding: icon: 'book' @@ -84,7 +75,6 @@ runs: INPUT_LOCATION: ${{ inputs.location }} INPUT_TITLE: ${{ inputs.title }} INPUT_SKIP_LABELS: ${{ inputs.skip-labels }} - INPUT_FAILS_ON_ERROR: ${{ inputs.fails-on-error }} run: | source .venv/bin/activate python ${{ github.action_path }}/main.py diff --git a/release_notes_presence_check/release_notes_presence_check_action.py b/release_notes_presence_check/release_notes_presence_check_action.py index 5286d73..125d2ca 100644 --- a/release_notes_presence_check/release_notes_presence_check_action.py +++ b/release_notes_presence_check/release_notes_presence_check_action.py @@ -43,7 +43,6 @@ def __init__(self) -> None: self.location: str = os.environ.get("INPUT_LOCATION", default="body") self.title: str = os.environ.get("INPUT_TITLE", default="[Rr]elease [Nn]otes:") self.skip_labels: list[str] = os.environ.get("INPUT_SKIP_LABELS", default="") - self.fails_on_error: bool = os.environ.get("INPUT_FAILS_ON_ERROR", "true").lower() == "true" self.__validate_inputs() @@ -65,19 +64,18 @@ def run(self) -> None: labels: list[str] = [label.get("name", "") for label in pr_data.get("labels", [])] if self.skip_labels in labels: logger.info("Skipping release notes check because '%s' label is present.", self.skip_labels) - self.write_output("true") sys.exit(0) # Exiting with code 0 indicates success but the action is skipped. # check release notes presence in defined location pr_body = pr_data.get("body", "") if len(pr_body.strip()) == 0: logger.error("Error: Pull request description is empty.") - self.handle_failure() + sys.exit(0) # Check if release notes tag is present if not re.search(self.title, pr_body): logger.error("Error: Release notes title '%s' not found in pull request body.", self.title) - self.handle_failure() + sys.exit(0) # Get line index of the release notes tag lines = pr_body.split("\n") @@ -90,81 +88,62 @@ def run(self) -> None: # Check if there is content after the release notes tag if release_notes_start_index is None or release_notes_start_index >= len(lines): logger.error("Error: No content found after the release notes tag.") - self.handle_failure() + sys.exit(1) # Check if there is a bullet list directly under the release notes tag text_below_release_notes = lines[release_notes_start_index:] if not text_below_release_notes or not text_below_release_notes[0].strip().startswith(("-", "+", "*")): logger.error("Error: No bullet list found directly under release notes tag.") - self.handle_failure() + sys.exit(1) - self.write_output("true") logger.info("Release Notes detected.") sys.exit(0) - def write_output(self, valid_value: str) -> None: - """ - Write the output to the file specified by the GITHUB_OUTPUT environment variable. - - @param valid_value: The value to write to the output file. - @return: None - """ - output_file = os.environ.get("GITHUB_OUTPUT", default="output.txt") - with open(output_file, "a", encoding="utf-8") as fh: - print(f"valid={valid_value}", file=fh) - - def handle_failure(self) -> None: - """ - Handle the failure of the action. - - @return: None - """ - self.write_output("false") - if self.fails_on_error: - sys.exit(1) - else: - sys.exit(0) - def __validate_inputs(self) -> None: """ Validate the required inputs. When the inputs are not valid, the action will fail. @return: None """ + error_detected = False + if len(self.github_token) == 0: logger.error("Failure: GITHUB_TOKEN is not set correctly.") - self.handle_failure() + error_detected = True value = os.environ.get("INPUT_PR_NUMBER", default="") if len(value) == 0: logger.error("Failure: PR_NUMBER is not set correctly.") - self.handle_failure() + error_detected = True if not value.isdigit(): logger.error("Failure: PR_NUMBER is not a valid number.") - self.handle_failure() + error_detected = True value = os.environ.get("INPUT_GITHUB_REPOSITORY", default="") if len(value) == 0: logger.error("Failure: GITHUB_REPOSITORY is not set correctly.") - self.handle_failure() + error_detected = True if value.count("/") != 1: logger.error("Failure: GITHUB_REPOSITORY is not in the correct format.") - self.handle_failure() - - if len(value.split("/")[0]) == 0 or len(value.split("/")[1]) == 0: - logger.error("Failure: GITHUB_REPOSITORY is not in the correct format.") - self.handle_failure() + error_detected = True + else: + if len(value.split("/")[0]) == 0 or len(value.split("/")[1]) == 0: + logger.error("Failure: GITHUB_REPOSITORY is not in the correct format.") + error_detected = True if len(self.location) == 0: logger.error("Failure: LOCATION is not set correctly.") - self.handle_failure() + error_detected = True if self.location not in ["body"]: logger.error("Failure: LOCATION is not one of the supported values.") - self.handle_failure() + error_detected = True if len(self.title) == 0: logger.error("Failure: TITLE is not set correctly.") - self.handle_failure() + error_detected = True + + if error_detected: + sys.exit(1) diff --git a/tests/test_release_notes_presence_check_action.py b/tests/test_release_notes_presence_check_action.py index c6400af..eff5f20 100644 --- a/tests/test_release_notes_presence_check_action.py +++ b/tests/test_release_notes_presence_check_action.py @@ -78,7 +78,6 @@ def test_validate_inputs_invalid(monkeypatch, caplog, env_name, env_value, error "INPUT_LOCATION": "body", "INPUT_TITLE": "[Rr]elease [Nn]otes:", "INPUT_SKIP_LABELS": "", - "INPUT_FAILS_ON_ERROR": "true", } # Update or remove the environment variable for the tested scenario @@ -136,14 +135,10 @@ def test_run_successful(mocker): "labels": [{"name": "bug"}, {"name": "enhancement"}] } - # Mock the output writing method - mock_output = mocker.patch("release_notes_presence_check.release_notes_presence_check_action.ReleaseNotesPresenceCheckAction.write_output") - # Run the action action = ReleaseNotesPresenceCheckAction() action.run() - mock_output.assert_called_once_with("true") mock_exit.assert_called_once_with(0) @@ -174,9 +169,6 @@ def test_run_skip_by_label(mocker): "labels": [{"name": "bug"}, {"name": "enhancement"}, {"name": "skip-release-notes-check"}] } - # Mock the output writing method - mock_output = mocker.patch("release_notes_presence_check.release_notes_presence_check_action.ReleaseNotesPresenceCheckAction.write_output") - # Run the action with pytest.raises(SystemExit) as exit_info: action = ReleaseNotesPresenceCheckAction() @@ -185,8 +177,6 @@ def test_run_skip_by_label(mocker): assert SystemExit == exit_info.type assert 0 == exit_info.value.code - mock_output.assert_called_once_with("true") - def test_run_fail_no_body(mocker): # Set environment variables @@ -214,9 +204,6 @@ def test_run_fail_no_body(mocker): "labels": [{"name": "bug"}, {"name": "enhancement"}] } - # Mock the output writing method - mock_output = mocker.patch("release_notes_presence_check.release_notes_presence_check_action.ReleaseNotesPresenceCheckAction.write_output") - # Run the action with pytest.raises(SystemExit) as exit_info: action = ReleaseNotesPresenceCheckAction() @@ -225,8 +212,6 @@ def test_run_fail_no_body(mocker): assert SystemExit == exit_info.type assert 1 == exit_info.value.code - mock_output.assert_called_once_with("false") - def test_run_fail_empty_body(mocker): # Set environment variables env_vars = { @@ -254,9 +239,6 @@ def test_run_fail_empty_body(mocker): "labels": [{"name": "bug"}, {"name": "enhancement"}] } - # Mock the output writing method - mock_output = mocker.patch("release_notes_presence_check.release_notes_presence_check_action.ReleaseNotesPresenceCheckAction.write_output") - # Run the action with pytest.raises(SystemExit) as exit_info: action = ReleaseNotesPresenceCheckAction() @@ -265,8 +247,6 @@ def test_run_fail_empty_body(mocker): assert SystemExit == exit_info.type assert 1 == exit_info.value.code - mock_output.assert_called_once_with("false") - def test_run_fail_title_not_found(mocker): # Set environment variables env_vars = { @@ -294,9 +274,6 @@ def test_run_fail_title_not_found(mocker): "labels": [{"name": "bug"}, {"name": "enhancement"}] } - # Mock the output writing method - mock_output = mocker.patch("release_notes_presence_check.release_notes_presence_check_action.ReleaseNotesPresenceCheckAction.write_output") - # Run the action with pytest.raises(SystemExit) as exit_info: action = ReleaseNotesPresenceCheckAction() @@ -305,8 +282,6 @@ def test_run_fail_title_not_found(mocker): assert SystemExit == exit_info.type assert 1 == exit_info.value.code - mock_output.assert_called_once_with("false") - def test_run_fail_release_notes_lines_not_found(mocker): # Set environment variables env_vars = { @@ -334,9 +309,6 @@ def test_run_fail_release_notes_lines_not_found(mocker): "labels": [{"name": "bug"}, {"name": "enhancement"}] } - # Mock the output writing method - mock_output = mocker.patch("release_notes_presence_check.release_notes_presence_check_action.ReleaseNotesPresenceCheckAction.write_output") - # Run the action with pytest.raises(SystemExit) as exit_info: action = ReleaseNotesPresenceCheckAction() @@ -345,8 +317,6 @@ def test_run_fail_release_notes_lines_not_found(mocker): assert SystemExit == exit_info.type assert 1 == exit_info.value.code - mock_output.assert_called_once_with("false") - def test_run_fail_no_lines_after_title(mocker): # Set environment variables env_vars = { @@ -374,9 +344,6 @@ def test_run_fail_no_lines_after_title(mocker): "labels": [{"name": "bug"}, {"name": "enhancement"}] } - # Mock the output writing method - mock_output = mocker.patch("release_notes_presence_check.release_notes_presence_check_action.ReleaseNotesPresenceCheckAction.write_output") - # Run the action with pytest.raises(SystemExit) as exit_info: action = ReleaseNotesPresenceCheckAction() @@ -384,35 +351,3 @@ def test_run_fail_no_lines_after_title(mocker): assert SystemExit == exit_info.type assert 1 == exit_info.value.code - - mock_output.assert_called_once_with("false") - -# handle_failure - -def test_handle_failure_fails_on_error_false(mocker): - # Set environment variables with 'INPUT_FAILS_ON_ERROR' set to 'false' - env_vars = { - "INPUT_GITHUB_TOKEN": "fake_token", - "INPUT_PR_NUMBER": "109", - "INPUT_GITHUB_REPOSITORY": "owner/repo", - "INPUT_LOCATION": "body", - "INPUT_TITLE": "[Rr]elease [Nn]otes:", - "INPUT_SKIP_LABELS": "", - "INPUT_FAILS_ON_ERROR": "false", # Set to 'false' to test else branch - } - mocker.patch.dict(os.environ, env_vars) - - # Mock sys.exit to raise SystemExit exception - def mock_exit(code): - raise SystemExit(code) - mocker.patch("sys.exit", mock_exit) - - # Instantiate the action - action = ReleaseNotesPresenceCheckAction() - - # Call handle_failure and expect SystemExit - with pytest.raises(SystemExit) as e: - action.handle_failure() - - # Assert that sys.exit was called with exit code 0 - assert e.value.code == 0 From cbe6faf216717efab34e486506175af16df8b369 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 22 Nov 2024 10:08:47 +0100 Subject: [PATCH 3/6] - Adding missing unit test files. --- tests/conftest.py | 29 ++++++++++++++ tests/utils/__init__.py | 0 tests/utils/test_logging_config.py | 62 ++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 tests/conftest.py create mode 100644 tests/utils/__init__.py create mode 100644 tests/utils/test_logging_config.py diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..4cc981c --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,29 @@ +# +# Copyright 2024 ABSA Group Limited +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import pytest + +@pytest.fixture +def mock_logging_setup(mocker): + """Fixture to mock the basic logging setup using pytest-mock.""" + mock_log_config = mocker.patch("logging.basicConfig") + yield mock_log_config + +@pytest.fixture() +def mock_exit(code) -> list: + exit_calls = [] + exit_calls.append(code) + return exit_calls diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/utils/test_logging_config.py b/tests/utils/test_logging_config.py new file mode 100644 index 0000000..8fa5dc0 --- /dev/null +++ b/tests/utils/test_logging_config.py @@ -0,0 +1,62 @@ +# +# Copyright 2024 ABSA Group Limited +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import logging +import os +import sys + +from logging import StreamHandler + +from release_notes_presence_check.utils.logging_config import setup_logging + + +def validate_logging_config(mock_logging_setup, caplog, expected_level, expected_message): + mock_logging_setup.assert_called_once() + + # Get the actual call arguments from the mock + call_args = mock_logging_setup.call_args[1] # Extract the kwargs from the call + + # Validate the logging level and format + assert call_args["level"] == expected_level + assert call_args["format"] == "%(asctime)s - %(levelname)s - %(message)s" + assert call_args["datefmt"] == "%Y-%m-%d %H:%M:%S" + + # Check that the handler is a StreamHandler and outputs to sys.stdout + handlers = call_args["handlers"] + assert 1 == len(handlers) # Only one handler is expected + assert isinstance(handlers[0], StreamHandler) # Handler should be StreamHandler + assert handlers[0].stream is sys.stdout # Stream should be sys.stdout + + # Check that the log message is present + assert expected_message in caplog.text + + +# setup_logging + +def test_setup_logging_default_logging_level(mock_logging_setup, caplog): + with caplog.at_level(logging.INFO): + setup_logging() + + validate_logging_config(mock_logging_setup, caplog, logging.INFO, "Logging configuration set up.") + + +def test_setup_logging_debug_mode_enabled_by_ci(mock_logging_setup, caplog): + os.environ["RUNNER_DEBUG"] = "1" + + with caplog.at_level(logging.DEBUG): + setup_logging() + + validate_logging_config(mock_logging_setup, caplog, logging.DEBUG, "Debug mode enabled by CI runner.") From 665f1e1845d86be4737b55ed9b7d1fbe8c2b230d Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Fri, 22 Nov 2024 10:11:07 +0100 Subject: [PATCH 4/6] - Applied black. --- .../release_notes_presence_check_action.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_notes_presence_check/release_notes_presence_check_action.py b/release_notes_presence_check/release_notes_presence_check_action.py index 125d2ca..a604ad9 100644 --- a/release_notes_presence_check/release_notes_presence_check_action.py +++ b/release_notes_presence_check/release_notes_presence_check_action.py @@ -129,7 +129,7 @@ def __validate_inputs(self) -> None: logger.error("Failure: GITHUB_REPOSITORY is not in the correct format.") error_detected = True else: - if len(value.split("/")[0]) == 0 or len(value.split("/")[1]) == 0: + if len(value.split("/")[0]) == 0 or len(value.split("/")[1]) == 0: logger.error("Failure: GITHUB_REPOSITORY is not in the correct format.") error_detected = True From 4ca4e07e0fbbec937828724d22e44dc46c8539ca Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 25 Nov 2024 14:34:24 +0100 Subject: [PATCH 5/6] - Fix review comments. --- README.md | 4 ++-- tests/conftest.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ec0b397..ba5f992 100644 --- a/README.md +++ b/README.md @@ -74,8 +74,8 @@ See the default action step definition: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: - github-repository: "{ org }/{ repo }" # ${{ github.repository }} - pr-number: 109 # ${{ github.event.number }} + github-repository: "{ org }/{ repo }" # e.g. ${{ github.repository }} + pr-number: 109 # e.h. ${{ github.event.number }} location: "body" title: "[Rr]elease [Nn]otes:" skip-labels: "skip-release-notes,no-release-notes" diff --git a/tests/conftest.py b/tests/conftest.py index 4cc981c..780a96f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -24,6 +24,7 @@ def mock_logging_setup(mocker): @pytest.fixture() def mock_exit(code) -> list: + """Fixture to mock the exit function using pytest-mock.""" exit_calls = [] exit_calls.append(code) return exit_calls From dae7acc245f7ad9834291edd8d8a632e389c6174 Mon Sep 17 00:00:00 2001 From: miroslavpojer Date: Mon, 25 Nov 2024 14:36:30 +0100 Subject: [PATCH 6/6] - Fix typo. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ba5f992..0e64a0d 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ See the default action step definition: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: github-repository: "{ org }/{ repo }" # e.g. ${{ github.repository }} - pr-number: 109 # e.h. ${{ github.event.number }} + pr-number: 109 # e.g. ${{ github.event.number }} location: "body" title: "[Rr]elease [Nn]otes:" skip-labels: "skip-release-notes,no-release-notes"