Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After Release fix #6

Merged
merged 6 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 2 additions & 25 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -85,12 +74,11 @@ 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 }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in VersionTagChecker project, this comment is little confusing. I see that the number below is the number of github.event. But for repository, I don't think it is true. It should be repository.login.owner and repository.login.name or something like it. OR it is same as repository.id.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

pr-number: 109 # ${{ github.event.number }}
location: "body"
title: "[Rr]elease [Nn]otes:"
skip-labels: "skip-release-notes,no-release-notes"
fails-on-error: "false"
```

## Running Static Code Analysis
Expand Down Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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")
Expand All @@ -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)
29 changes: 29 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -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:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the same pattern of having or not having the method docstrings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

exit_calls = []
exit_calls.append(code)
return exit_calls
Loading