From f1806bad5aa1bc1a1e053e285a34e198d7d8a55a Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Mon, 8 Dec 2025 19:44:14 -0500 Subject: [PATCH 1/9] Enhance Homebrew release action and tests Refactors the release_homebrew action to improve tap handling, commit logic, and GitHub Actions log grouping. Adds robust tap name parsing, explicit commit steps for formula changes, and more granular test-bot steps. Updates tests and fixtures for better coverage, including tap naming edge cases, environment setup, and cleanup. Introduces .gitattributes for consistent line endings and improves CI workflow reliability, especially on macOS. --- .gitattributes | 5 + .github/workflows/ci-tests.yml | 13 +- actions/release_homebrew/action.yml | 50 ++- actions/release_homebrew/ci-matrix.json | 8 +- actions/release_homebrew/main.py | 380 +++++++++++++++--- tests/conftest.py | 2 + tests/release_homebrew/conftest.py | 278 ++++++++++++- .../release_homebrew/test_release_homebrew.py | 373 +++++++++++++++-- 8 files changed, 982 insertions(+), 127 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..b4c2c29 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,5 @@ +# ensure Unix specific files are checked out with LF line endings +Dockerfile text eol=lf +*.dockerfile text eol=lf +*.rb text eol=lf +*.sh text eol=lf diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index dae8af8..2eb9fb5 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -56,7 +56,17 @@ jobs: - name: Install Node Dependencies shell: bash - run: npm install + run: npm install --ignore-scripts + + - name: Backup CA + if: runner.os == 'macOS' + shell: bash + run: | + # `brew test-bot --only-cleanup-before` removes the CA bundle, so make a backup + cp \ + "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/certifi/cacert.pem" \ + "cacert.pem" + echo "REQUESTS_CA_BUNDLE=cacert.pem" >> "${GITHUB_ENV}" - name: Test with pytest id: pytest @@ -79,6 +89,7 @@ jobs: --verbose \ --color=yes \ --cov=actions \ + --cov-report=term \ --cov-report=xml:coverage/python-coverage.xml \ --cov-report=json:coverage/python-coverage.json \ --junitxml=junit-python.xml \ diff --git a/actions/release_homebrew/action.yml b/actions/release_homebrew/action.yml index 778c38f..780ace4 100644 --- a/actions/release_homebrew/action.yml +++ b/actions/release_homebrew/action.yml @@ -111,7 +111,7 @@ runs: "${{ steps.venv.outputs.python-path }}" -m pip install -r requirements.txt echo "::endgroup::" - - name: Checkout org homebrew repo + - name: Checkout (org homebrew repo) uses: actions/checkout@v6 with: repository: ${{ inputs.org_homebrew_repo }} @@ -120,7 +120,7 @@ runs: persist-credentials: false # otherwise, the token used is the GITHUB_TOKEN, instead of the personal token fetch-depth: 1 - - name: Checkout homebrew-core fork + - name: Checkout (homebrew-core) if: ${{ inputs.contribute_to_homebrew_core == 'true' }} uses: actions/checkout@v6 with: @@ -133,6 +133,9 @@ runs: env: INPUT_FORMULA_FILE: ${{ inputs.formula_file }} INPUT_CONTRIBUTE_TO_HOMEBREW_CORE: ${{ inputs.contribute_to_homebrew_core }} + INPUT_GIT_EMAIL: ${{ inputs.git_email }} + INPUT_GIT_USERNAME: ${{ inputs.git_username }} + INPUT_ORG_HOMEBREW_REPO: ${{ inputs.org_homebrew_repo }} INPUT_UPSTREAM_HOMEBREW_CORE_REPO: ${{ inputs.upstream_homebrew_core_repo }} INPUT_VALIDATE: ${{ inputs.validate }} id: homebrew-tests @@ -151,7 +154,7 @@ runs: "${{ steps.venv.outputs.python-path }}" -u main.py echo "::endgroup::" - - name: GitHub Commit & Push + - name: GitHub Commit & Push (org homebrew repo) if: ${{ inputs.publish == 'true' }} uses: actions-js/push@v1.5 with: @@ -160,10 +163,10 @@ runs: branch: ${{ inputs.org_homebrew_repo_branch }} # commit to target branch directory: ${{ github.workspace }}/release_homebrew_action/org_homebrew_repo github_token: ${{ inputs.token }} - message: "Update ${{ github.repository }} to ${{ github.sha }}" + message: "chore: update ${{ github.repository }} to ${{ github.sha }}" repository: ${{ inputs.org_homebrew_repo }} - - name: GitHub Commit & Push homebrew-core + - name: GitHub Commit & Push (homebrew-core) if: ${{ inputs.contribute_to_homebrew_core == 'true' && inputs.publish == 'true' }} uses: actions-js/push@v1.5 with: @@ -173,10 +176,41 @@ runs: directory: ${{ github.workspace }}/release_homebrew_action/homebrew_core_fork_repo force: true # need to force since the branch is sometimes reset github_token: ${{ inputs.token }} - message: "Update ${{ github.repository }} to ${{ github.sha }}" + message: "${{ github.repository }}: update to ${{ github.sha }}" repository: ${{ inputs.homebrew_core_fork_repo }} - - name: Create Pull Request + - name: Create Pull Request (org homebrew repo) + env: + GH_TOKEN: ${{ inputs.token }} + INPUT_ORG_HOMEBREW_REPO_BRANCH: ${{ inputs.org_homebrew_repo_branch }} + INPUT_ORG_HOMEBREW_REPO: ${{ inputs.org_homebrew_repo }} + if: ${{ inputs.publish == 'true' }} + shell: bash + working-directory: ${{ github.workspace }}/release_homebrew_action/org_homebrew_repo + run: | + # Check if a pull request already exists with the same head branch + PR_EXISTS=$(gh pr list \ + --head ${INPUT_ORG_HOMEBREW_REPO_BRANCH} \ + --repo ${INPUT_ORG_HOMEBREW_REPO}) + + # If the pull request does not exist, create it + if [[ -z "$PR_EXISTS" ]]; then + echo "Creating pull request" + + # https://cli.github.com/manual/gh_pr_create + gh pr create \ + --base master \ + --head ${INPUT_ORG_HOMEBREW_REPO_BRANCH} \ + --title "chore: update ${{ github.repository }} to ${{ github.sha }}" \ + --body \ + "Created by the LizardByte [release_homebrew](https://github.com/LizardByte/actions) action" \ + --no-maintainer-edit \ + --repo ${INPUT_ORG_HOMEBREW_REPO} + else + echo "Pull request already exists" + fi + + - name: Create Pull Request (homebrew-core) env: GH_TOKEN: ${{ inputs.token }} HOMEBREW_CORE_BRANCH: ${{ steps.homebrew-tests.outputs.homebrew_core_branch }} @@ -198,7 +232,7 @@ runs: gh pr create \ --base master \ --head ${HOMEBREW_CORE_BRANCH} \ - --title "Update ${{ github.repository }} to ${{ github.sha }}" \ + --title "${{ github.repository }}: update to ${{ github.sha }}" \ --body \ "Created by the LizardByte [release_homebrew](https://github.com/LizardByte/actions) action" \ --no-maintainer-edit \ diff --git a/actions/release_homebrew/ci-matrix.json b/actions/release_homebrew/ci-matrix.json index c1d2349..484e857 100644 --- a/actions/release_homebrew/ci-matrix.json +++ b/actions/release_homebrew/ci-matrix.json @@ -6,8 +6,8 @@ "formula_file": "${ github.workspace }/tests/release_homebrew/Formula/hello_world.rb", "git_email": "${ secrets.GH_BOT_EMAIL }", "git_username": "${ secrets.GH_BOT_NAME }", - "org_homebrew_repo": "${ github.repository }", - "org_homebrew_repo_branch": "tests-release_homebrew-${ runner.os }", + "org_homebrew_repo": "LizardByte/homebrew-homebrew", + "org_homebrew_repo_branch": "master", "publish": false, "token": "${ secrets.GH_BOT_TOKEN }", "upstream_homebrew_core_repo": "LizardByte/homebrew-core" @@ -20,8 +20,8 @@ "formula_file": "${ github.workspace }/tests/release_homebrew/Formula/hello_world.rb", "git_email": "${ secrets.GH_BOT_EMAIL }", "git_username": "${ secrets.GH_BOT_NAME }", - "org_homebrew_repo": "${ github.repository }", - "org_homebrew_repo_branch": "tests-release_homebrew-${ runner.os }", + "org_homebrew_repo": "LizardByte/homebrew-homebrew", + "org_homebrew_repo_branch": "master", "publish": false, "token": "${ secrets.GH_BOT_TOKEN }", "upstream_homebrew_core_repo": "LizardByte/homebrew-core" diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index 03c55c6..9983e69 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -3,6 +3,7 @@ import os import select import shutil +import stat import subprocess import sys from typing import AnyStr, IO, Optional, Mapping @@ -23,7 +24,7 @@ TEMP_DIRECTORIES = [] HOMEBREW_BUILDPATH = "" -temp_repo = os.path.join('release_homebrew_action', 'homebrew-test') +tap_repo_name = "" # will be set based on INPUT_ORG_HOMEBREW_REPO og_dir = os.getcwd() @@ -94,16 +95,17 @@ def _setup_process(args_list: list, cwd: Optional[str], env: Optional[Mapping]) if cwd: os.chdir(cwd) # hack for unit testing on windows - process = subprocess.Popen( - args=args_list, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=cwd, - env=env, - ) - - if cwd: - os.chdir(og_dir) + try: + process = subprocess.Popen( + args=args_list, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=cwd, + env=env, + ) + finally: + if cwd: + os.chdir(og_dir) return process @@ -149,6 +151,31 @@ def set_github_action_output(output_name: str, output_value: str): f.write('\nEOF\n') +def start_group(title: str): + """ + Start a collapsible group in GitHub Actions logs. + + Parameters + ---------- + title : str + The title of the group. + """ + if os.getenv('PYTEST_RUN'): + print(f'>> {title}') + else: + print(f'::group::{title}') + + +def end_group(): + """ + End a collapsible group in GitHub Actions logs. + """ + if os.getenv('PYTEST_RUN'): + print('<< END') + else: + print('::endgroup::') + + def get_brew_repository() -> str: proc = subprocess.run( args=['brew', '--repository'], @@ -157,6 +184,61 @@ def get_brew_repository() -> str: return proc.stdout.decode('utf-8').strip() +def commit_formula_changes( + path: str, + formula_filename: str, + message: str, +) -> None: + """ + Commit formula changes to a git repository. + + Parameters + ---------- + path : str + Path to the git repository. + formula_filename : str + Name of the formula file being committed. + message : str + Commit message. + """ + start_group(f'Committing formula changes in {path}') + + # Configure git user if not already configured + git_email = os.getenv('INPUT_GIT_EMAIL') + git_username = os.getenv('INPUT_GIT_USERNAME') + + if git_email: + print(f'Configuring git user.email: {git_email}') + _run_subprocess( + args_list=['git', 'config', 'user.email', git_email], + cwd=path, + ) + + if git_username: + print(f'Configuring git user.name: {git_username}') + _run_subprocess( + args_list=['git', 'config', 'user.name', git_username], + cwd=path, + ) + + # Add the formula file + print(f'Adding {formula_filename} to git') + _run_subprocess( + args_list=['git', 'add', '-A'], + cwd=path, + ) + + # Commit the changes + print(f'Committing changes: {message}') + _run_subprocess( + args_list=['git', 'commit', '-m', message], + cwd=path, + ignore_error=True, # ignore error if nothing to commit + ) + + end_group() + + def prepare_homebrew_core_fork( branch_suffix: str, path: str, @@ -165,27 +247,42 @@ def prepare_homebrew_core_fork( og_error = ERROR - print('Preparing Homebrew/homebrew-core fork') + start_group('Preparing Homebrew/homebrew-core fork') # checkout a new branch branch_name = f'release_homebrew_action/{branch_suffix}' - print(f'Attempt to create new branch {branch_name}') - result = _run_subprocess( - args_list=['git', 'checkout', '-b', branch_name], + # Check if we're already on the target branch + process = subprocess.run( + ['git', 'rev-parse', '--abbrev-ref', 'HEAD'], cwd=path, + capture_output=True, + text=True ) - if not result: # checkout the existing branch - print(f'Attempting to checkout existing branch {branch_name}') - result = _run_subprocess( - args_list=['git', 'checkout', branch_name], - cwd=path, - ) + current_branch = process.stdout.strip() if process.returncode == 0 else '' - if result: + if current_branch == branch_name: + print(f'Already on branch {branch_name}') + result = True ERROR = og_error else: - raise SystemExit(1, f'::error:: Failed to create or checkout branch {branch_name}') + print(f'Attempt to create new branch {branch_name}') + result = _run_subprocess( + args_list=['git', 'checkout', '-b', branch_name], + cwd=path, + ) + if not result: # checkout the existing branch + print(f'Attempting to checkout existing branch {branch_name}') + result = _run_subprocess( + args_list=['git', 'checkout', branch_name], + cwd=path, + ) + + if result: + ERROR = og_error + else: + end_group() + raise SystemExit(1, f'::error:: Failed to create or checkout branch {branch_name}') # add the upstream remote print('Adding upstream remote') @@ -219,8 +316,47 @@ def prepare_homebrew_core_fork( output_value=branch_name ) + end_group() + + +def _get_tap_name_from_repo(org_homebrew_repo_input: str) -> tuple[str, str]: + """ + Extract tap name and owner from repository input. + + Parameters + ---------- + org_homebrew_repo_input : str + Repository input in format owner/homebrew-tap_name or owner/actions + + Returns + ------- + tuple[str, str] + Tuple of (owner, tap_name) + """ + print(f'org_homebrew_repo_input: {org_homebrew_repo_input}') + print(f'INPUT_ORG_HOMEBREW_REPO env var: {os.getenv("INPUT_ORG_HOMEBREW_REPO")}') + + owner, repo_name = org_homebrew_repo_input.split('/') + if repo_name.startswith('homebrew-'): + tap_name = repo_name[9:] # Remove "homebrew-" prefix + elif org_homebrew_repo_input == 'LizardByte/actions': + # Special case for CI testing + print('Using LizardByte/actions for CI testing (special case)') + tap_name = 'actions' + else: + raise ValueError( + f'::error:: Repository name "{repo_name}" does not follow Homebrew tap naming convention. ' + f'The repository name must start with "homebrew-" (e.g., "owner/homebrew-tap"). ' + f'Please ensure the org_homebrew_repo input is set correctly. ' + f'Current value: {org_homebrew_repo_input}' + ) + + return owner, tap_name + def process_input_formula(formula_file: str) -> str: + global tap_repo_name + # check if the formula file exists if not os.path.exists(formula_file): raise FileNotFoundError(f'::error:: Formula file {formula_file} does not exist') @@ -253,16 +389,12 @@ def process_input_formula(formula_file: str) -> str: ], ) - # run brew tap - print(f'Running `brew tap-new {temp_repo} --no-git`') - _run_subprocess( - args_list=[ - 'brew', - 'tap-new', - temp_repo, - '--no-git' - ], - ) + # Parse the org_homebrew_repo to get the tap name + org_homebrew_repo_input = os.getenv('INPUT_ORG_HOMEBREW_REPO', 'LizardByte/homebrew-homebrew') + owner, tap_name = _get_tap_name_from_repo(org_homebrew_repo_input) + + tap_repo_name = f'{owner.lower()}/{tap_name}' + print(f'tap_repo_name: {tap_repo_name}') org_homebrew_repo = os.path.join( os.environ['GITHUB_WORKSPACE'], 'release_homebrew_action', 'org_homebrew_repo') @@ -271,25 +403,88 @@ def process_input_formula(formula_file: str) -> str: print(f'org_homebrew_repo: {org_homebrew_repo}') print(f'homebrew_core_fork_repo: {homebrew_core_fork_repo}') + # Tap the existing repo + start_group(f'Tapping repository {tap_repo_name}') + print(f'Running `brew tap {tap_repo_name} {org_homebrew_repo}`') + _run_subprocess( + args_list=[ + 'brew', + 'tap', + tap_repo_name, + org_homebrew_repo, + ], + ) + + end_group() + if os.getenv('INPUT_CONTRIBUTE_TO_HOMEBREW_CORE').lower() == 'true': prepare_homebrew_core_fork(branch_suffix=formula, path=homebrew_core_fork_repo) - # copy the formula file to the two directories + # copy the formula file to the directories + start_group(f'Copying formula {formula} to tap directories') + + # Map directories to their repository root paths for committing + tap_dir_to_repo = {} + + org_homebrew_repo_formula_dir = os.path.join(org_homebrew_repo, 'Formula', first_letter) + homebrew_core_fork_repo_formula_dir = os.path.join(homebrew_core_fork_repo, 'Formula', first_letter) + + tap_dir_to_repo[org_homebrew_repo_formula_dir] = org_homebrew_repo + tap_dir_to_repo[homebrew_core_fork_repo_formula_dir] = homebrew_core_fork_repo + tap_dirs = [ - os.path.join(org_homebrew_repo, 'Formula', first_letter), # we will commit back to this - os.path.join(homebrew_core_fork_repo, 'Formula', first_letter), # we will commit back to this + org_homebrew_repo_formula_dir, # we will commit back to this + homebrew_core_fork_repo_formula_dir, # we will commit back to this ] + if is_brew_installed(): - tap_dirs.append(os.path.join(get_brew_repository(), 'Library', 'Taps', temp_repo, 'Formula', first_letter)) + # Get the tapped location + brew_tap_root = os.path.join( + get_brew_repository(), + 'Library', + 'Taps', + owner.lower(), + f'homebrew-{tap_name}', + ) + brew_tap_path = os.path.join(brew_tap_root, 'Formula', first_letter) + tap_dirs.append(brew_tap_path) + tap_dir_to_repo[brew_tap_path] = brew_tap_root + for d in tap_dirs: print(f'Copying {formula_filename} to {d}') os.makedirs(d, exist_ok=True) - shutil.copy2(formula_file, d) + dest_file = os.path.join(d, formula_filename) + shutil.copy2(formula_file, dest_file) - if not os.path.exists(os.path.join(d, formula_filename)): + if not os.path.exists(dest_file): raise FileNotFoundError(f'::error:: Formula file {formula_filename} was not copied to {d}') + + # Set permissions required by Homebrew (rw-r--r--) + # Owner: read + write, Group: read, Others: read + # Homebrew requires formula files to be world-readable (brew audit enforces this) + # Only owner has write permission, complying with security best practices + # Formula files are Ruby scripts that should not be executable + os.chmod(dest_file, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH) print(f'Copied {formula_filename} to {d}') + end_group() + + # Commit changes to the tap directories + github_sha = os.getenv('GITHUB_SHA', '') + commit_message = f'Add/Update {formula} formula' + if github_sha: + commit_message = f'{commit_message} ({github_sha[:7]})' + + for formula_dir, repo_path in tap_dir_to_repo.items(): + if os.path.isdir(os.path.join(repo_path, '.git')): + commit_formula_changes( + path=repo_path, + formula_filename=formula_filename, + message=commit_message, + ) + else: + print(f'Skipping commit for {repo_path} (not a git repository)') + return formula @@ -304,8 +499,8 @@ def is_brew_installed() -> bool: def audit_formula(formula: str) -> bool: - print(f'Auditing formula {formula}') - return _run_subprocess( + start_group(f'Auditing formula {formula}') + result = _run_subprocess( args_list=[ 'brew', 'audit', @@ -313,12 +508,16 @@ def audit_formula(formula: str) -> bool: '--arch=all', '--strict', '--online', - os.path.join(temp_repo, formula) + f'{tap_repo_name}/{formula}' ], ) + end_group() + return result def brew_upgrade() -> bool: + start_group('Updating and Upgrading Homebrew') + print('Updating Homebrew') env = { 'HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK': '1', @@ -335,10 +534,11 @@ def brew_upgrade() -> bool: env=env, ) if not result: + end_group() return False print('Upgrading Homebrew') - return _run_subprocess( + result = _run_subprocess( args_list=[ 'brew', 'upgrade' @@ -346,27 +546,69 @@ def brew_upgrade() -> bool: env=env, ) + end_group() + return result + -def brew_debug() -> bool: - # run brew config - print('Running `brew config`') +def brew_test_bot_only_cleanup_before() -> bool: + start_group('Running brew test-bot --only-cleanup-before') result = _run_subprocess( args_list=[ 'brew', - 'config', + 'test-bot', + f'--tap={tap_repo_name}', + '--only-cleanup-before', ], ) + end_group() + return result - # run brew doctor - print('Running `brew doctor`') - _run_subprocess( + +def brew_test_bot_only_setup() -> bool: + start_group('Running brew test-bot --only-setup') + result = _run_subprocess( args_list=[ 'brew', - 'doctor', + 'test-bot', + f'--tap={tap_repo_name}', + '--only-setup', ], - ignore_error=True, ) + end_group() + return result + +def brew_test_bot_only_tap_syntax() -> bool: + start_group('Running brew test-bot --only-tap-syntax') + result = _run_subprocess( + args_list=[ + 'brew', + 'test-bot', + f'--tap={tap_repo_name}', + '--only-tap-syntax', + ], + ) + end_group() + return result + + +def brew_test_bot_only_formulae(formula: str) -> bool: + start_group(f'Running brew test-bot --only-formulae for {formula}') + + org_repo = os.environ['INPUT_ORG_HOMEBREW_REPO'] + root_url = f'https://ghcr.io/v2/{org_repo.rsplit("-", 1)[0].lower()}' + result = _run_subprocess( + args_list=[ + 'brew', + 'test-bot', + '--only-formulae', + f'--tap={tap_repo_name}', + f'--testing-formulae={tap_repo_name}/{formula}', + f'--root-url={root_url}', + ], + ) + + end_group() return result @@ -404,7 +646,8 @@ def find_tmp_dir(formula: str) -> str: def install_formula(formula: str) -> bool: - print(f'Installing formula {formula}') + start_group(f'Installing formula {formula}') + env = { 'HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK': '1', } @@ -419,7 +662,7 @@ def install_formula(formula: str) -> bool: '--include-test', '--keep-tmp', '--verbose', - os.path.join(temp_repo, formula), + f'{tap_repo_name}/{formula}', ], env=env, ) @@ -432,11 +675,13 @@ def install_formula(formula: str) -> bool: output_value=HOMEBREW_BUILDPATH ) + end_group() return result def test_formula(formula: str) -> bool: - print(f'Testing formula {formula}') + start_group(f'Testing formula {formula}') + env = { 'HOMEBREW_BUILDPATH': HOMEBREW_BUILDPATH, } @@ -450,7 +695,7 @@ def test_formula(formula: str) -> bool: 'test', '--keep-tmp', '--verbose', - os.path.join(temp_repo, formula), + f'{tap_repo_name}/{formula}', ], env=env, ) @@ -460,6 +705,7 @@ def test_formula(formula: str) -> bool: output_value=find_tmp_dir(formula) ) + end_group() return result @@ -467,8 +713,6 @@ def main(): if not is_brew_installed(): raise SystemExit(1, 'Homebrew is not installed') - formula = process_input_formula(args.formula_file) - if os.environ['INPUT_VALIDATE'].lower() != 'true': print('Skipping audit, install, and test') return @@ -478,14 +722,24 @@ def main(): print('::error:: Homebrew update or upgrade failed') raise SystemExit(1) - if not brew_debug(): - print('::error:: Homebrew debug failed') + formula = process_input_formula(args.formula_file) + + if not brew_test_bot_only_cleanup_before(): + print('::error:: brew test-bot --only-cleanup-before failed') + raise SystemExit(1) + + if not brew_test_bot_only_setup(): + print('::error:: brew test-bot --only-setup failed') raise SystemExit(1) if not audit_formula(formula): print(f'::error:: Formula {formula} failed audit') FAILURES.append('audit') + if not brew_test_bot_only_tap_syntax(): + print('::error:: brew test-bot --only-tap-syntax failed') + FAILURES.append('tap-syntax') + if not install_formula(formula): print(f'::error:: Formula {formula} failed install') FAILURES.append('install') @@ -494,13 +748,17 @@ def main(): print(f'::error:: Formula {formula} failed test') FAILURES.append('test') + if not brew_test_bot_only_formulae(formula): + print('::error:: brew test-bot --only-formulae failed') + FAILURES.append('formulae') + if ERROR: raise SystemExit( 1, f'::error:: Formula did not pass checks: {FAILURES}. Please check the logs for more information.' ) - print(f'Formula {formula} audit, install, and test successful') + print(f'Formula {formula} passed all checks!') if __name__ == '__main__': # pragma: no cover diff --git a/tests/conftest.py b/tests/conftest.py index db1af1b..aa0a72a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,3 +14,5 @@ os.environ['GITHUB_REPOSITORY'] = 'LizardByte/actions' os.environ['GITHUB_STEP_SUMMARY'] = os.path.join(os.getcwd(), 'build', 'github_step_summary.md') os.environ['GITHUB_WORKSPACE'] = os.path.join(os.getcwd(), 'build', 'workspace') + + os.environ['PYTEST_RUN'] = 'true' diff --git a/tests/release_homebrew/conftest.py b/tests/release_homebrew/conftest.py index 833c544..9707e47 100644 --- a/tests/release_homebrew/conftest.py +++ b/tests/release_homebrew/conftest.py @@ -10,14 +10,92 @@ # local imports from actions.release_homebrew import main -os.environ['INPUT_FORMULA_FILE'] = os.path.join(os.getcwd(), 'tests', 'release_homebrew', 'Formula', 'hello_world.rb') -os.environ['INPUT_CONTRIBUTE_TO_HOMEBREW_CORE'] = 'true' -os.environ['INPUT_UPSTREAM_HOMEBREW_CORE_REPO'] = 'Homebrew/homebrew-core' - og_dir = os.getcwd() +@pytest.fixture(scope='session') +def is_macos(): + """Detect if running on macOS.""" + return sys.platform == 'darwin' + + +@pytest.fixture(scope='session', autouse=True) +def reinstall_packages_after_homebrew_tests(is_macos): + """ + Reinstall packages that brew test-bot removes during cleanup. + + brew test-bot --only-cleanup-before removes system packages including: + - gpg (breaks codecov uploads) + + This fixture runs after all release_homebrew tests complete, before other test modules run. + Only runs on macOS where brew test-bot causes these issues. + """ + yield + + # Only reinstall packages on macOS + if not is_macos: + return + + # After all release_homebrew tests complete, reinstall removed packages + print("Reinstalling packages removed by brew test-bot...") + + packages_to_reinstall = [ + 'gnupg', # Provides gpg command for codecov + ] + + for package in packages_to_reinstall: + try: + result = subprocess.run( + ['brew', 'install', package], + capture_output=True, + timeout=120, + ) + if result.returncode == 0: + print(f"✓ {package} reinstalled successfully via brew") + else: + # Package might already be installed, try reinstalling + result = subprocess.run( + ['brew', 'reinstall', package], + capture_output=True, + timeout=120, + ) + if result.returncode == 0: + print(f"✓ {package} reinstalled successfully via brew reinstall") + else: + print(f"✗ Failed to reinstall {package}: {result.stderr.decode()}") + except Exception as e: + print(f"✗ Exception while reinstalling {package}: {e}") + + +@pytest.fixture(scope='function', autouse=True) +def setup_release_homebrew_env(): + """Set up environment variables for release_homebrew tests only.""" + # Save original values + original_env = {} + env_vars = { + 'INPUT_FORMULA_FILE': os.path.join(os.getcwd(), 'tests', 'release_homebrew', 'Formula', 'hello_world.rb'), + 'INPUT_CONTRIBUTE_TO_HOMEBREW_CORE': 'true', + 'INPUT_GIT_EMAIL': 'test@example.com', + 'INPUT_GIT_USERNAME': 'Test User', + 'INPUT_ORG_HOMEBREW_REPO': 'LizardByte/homebrew-homebrew', + 'INPUT_UPSTREAM_HOMEBREW_CORE_REPO': 'Homebrew/homebrew-core', + } + + for key, value in env_vars.items(): + original_env[key] = os.environ.get(key) + os.environ[key] = value + + yield + + # Restore original values + for key, original_value in original_env.items(): + if original_value is None: + os.environ.pop(key, None) + else: + os.environ[key] = original_value + + @pytest.fixture(scope='function', autouse=True) def change_dir(): os.chdir(og_dir) @@ -26,6 +104,28 @@ def change_dir(): @pytest.fixture(scope='function', autouse=True) def error_reset(): main.ERROR = False + main.FAILURES = [] + + +@pytest.fixture(scope='function') +def test_formula_file(tmp_path): + """ + Create a test_formula.rb file for tests that need to verify copy failure scenarios. + This fixture creates the file in a temporary directory and cleans it up after the test. + """ + formula_dir = tmp_path / "Formula" / "t" + formula_dir.mkdir(parents=True, exist_ok=True) + test_formula_path = formula_dir / "test_formula.rb" + + # Create a minimal invalid formula (missing URL) + test_formula_path.write_text("""class TestFormula < Formula + url "https://github.com/LizardByte/actions.git" +end +""") + + yield str(test_formula_path) + + # Cleanup is automatic with tmp_path @pytest.fixture(scope='function') @@ -50,6 +150,51 @@ def operating_system(): pytest.skip("Skipping, cannot be tested on Windows") +@pytest.fixture(scope='function') +def org_homebrew_repo(): + directory = os.path.join(os.environ['GITHUB_WORKSPACE'], 'release_homebrew_action') + os.makedirs(directory, exist_ok=True) + + repo = 'org_homebrew_repo' + repo_directory = os.path.join(directory, repo) + + if not os.path.isdir(repo_directory): + # Create a minimal git repo structure + os.makedirs(repo_directory, exist_ok=True) + subprocess.run( + ['git', 'init'], + cwd=repo_directory, + capture_output=True, + ) + subprocess.run( + ['git', 'config', 'user.email', 'test@test.com'], + cwd=repo_directory, + capture_output=True, + ) + subprocess.run( + ['git', 'config', 'user.name', 'Test User'], + cwd=repo_directory, + capture_output=True, + ) + + # Create Formula directory structure + os.makedirs(os.path.join(repo_directory, 'Formula'), exist_ok=True) + + # Create a README + with open(os.path.join(repo_directory, 'README.md'), 'w') as f: + f.write('# Test Homebrew Tap\n') + + # Initial commit + subprocess.run(['git', 'add', '.'], cwd=repo_directory, capture_output=True) + subprocess.run(['git', 'commit', '-m', 'Initial commit'], cwd=repo_directory, capture_output=True) + + # Reset the repository to clean state, removing any test artifacts + subprocess.run(['git', 'reset', '--hard', 'HEAD'], cwd=repo_directory, capture_output=True) + subprocess.run(['git', 'clean', '-fd'], cwd=repo_directory, capture_output=True) + + yield repo_directory + + @pytest.fixture(scope='function') # todo: fix repo deletion def homebrew_core_fork_repo(): directory = os.path.join(os.environ['GITHUB_WORKSPACE'], 'release_homebrew_action') @@ -60,7 +205,6 @@ def homebrew_core_fork_repo(): if not os.path.isdir(repo_directory): # clone the homebrew-core fork, with depth 1 - os.chdir(directory) proc = subprocess.run( [ 'git', @@ -72,7 +216,6 @@ def homebrew_core_fork_repo(): cwd=directory, capture_output=True ) - os.chdir(og_dir) if proc.returncode != 0: print(proc.stderr.decode('utf-8')) @@ -95,35 +238,128 @@ def homebrew_core_fork_repo(): # shutil.rmtree(repo_directory) -@pytest.fixture(scope='function') -def brew_untap(): - # uninstall hello_world formula - proc = subprocess.run( - args=['brew', 'uninstall', 'hello_world'], - capture_output=True, - ) - if proc.returncode != 0: - print(proc.stderr.decode('utf-8')) - raise RuntimeError('Failed to uninstall hello_world formula') +def _cleanup_bottle_files(): + """Clean up bottle files from previous test runs.""" + import glob + workspaces_to_check = [ + os.getenv('GITHUB_WORKSPACE', ''), + os.getcwd(), + ] + # Remove duplicates and empty strings + workspaces_to_check = list({w for w in workspaces_to_check if w}) + + for workspace in workspaces_to_check: + bottle_patterns = [ + os.path.join(workspace, 'hello_world--*.bottle.*.tar.gz'), + os.path.join(workspace, 'hello_world--*.bottle.json'), + ] + for pattern in bottle_patterns: + for file in glob.glob(pattern): + if os.path.exists(file): + os.remove(file) + - # untap the temporary repo +def _cleanup_tapped_repositories(): + """Clean up test artifacts from tapped directories.""" + brew_repo = main.get_brew_repository() + taps_to_clean = [ + os.path.join(brew_repo, 'Library', 'Taps', 'lizardbyte', 'homebrew-homebrew'), + os.path.join(brew_repo, 'Library', 'Taps', 'lizardbyte', 'homebrew-actions'), + ] + for tap_dir in taps_to_clean: + if os.path.isdir(tap_dir): + # Reset the tap repository to clean state, removing any test artifacts + subprocess.run(['git', 'reset', '--hard', 'HEAD'], cwd=tap_dir, capture_output=True) + subprocess.run(['git', 'clean', '-fd'], cwd=tap_dir, capture_output=True) + + +def _uninstall_hello_world_formula(): + """Uninstall hello_world formula from all taps.""" + # Try to uninstall from specific taps first to avoid ambiguity + for tap in ['lizardbyte/homebrew/hello_world', 'lizardbyte/actions/hello_world', 'hello_world']: + subprocess.run( + args=['brew', 'uninstall', tap], + capture_output=True, + ) + # Silently ignore errors - we just want to make sure it's uninstalled + + +def _get_tap_name_for_cleanup(): + """Get the tap name for cleanup, either from main.tap_repo_name or derived from environment.""" + tap_name = main.tap_repo_name + if not tap_name: + # Derive from INPUT_ORG_HOMEBREW_REPO + org_homebrew_repo = os.getenv('INPUT_ORG_HOMEBREW_REPO', 'LizardByte/homebrew-homebrew') + owner, repo_name = org_homebrew_repo.split('/') + if repo_name.startswith('homebrew-'): + tap_suffix = repo_name[9:] + tap_name = f'{owner.lower()}/{tap_suffix}' + return tap_name + + +def _untap_and_remove_directory(tap_name): + """Untap the repository and remove its directory.""" proc = subprocess.run( - args=['brew', 'untap', main.temp_repo], + args=['brew', 'untap', tap_name], capture_output=True, ) if proc.returncode != 0: - print(proc.stderr.decode('utf-8')) - raise RuntimeError('Failed to untap the temporary repo') + stderr = proc.stderr.decode('utf-8') + # Only print error if it's not "No such tap" error + if 'No such tap' not in stderr: + print(stderr) # remove brew tap directory brew_repo = main.get_brew_repository() - tap_directory = os.path.join(brew_repo, 'Library', 'Taps', main.temp_repo) + owner, tap_suffix = tap_name.split('/') + tap_directory = os.path.join(brew_repo, 'Library', 'Taps', owner, f'homebrew-{tap_suffix}') if os.path.isdir(tap_directory): shutil.rmtree(tap_directory) +@pytest.fixture(scope='function') +def brew_untap(): + def cleanup(): + _cleanup_bottle_files() + _cleanup_tapped_repositories() + _uninstall_hello_world_formula() + + tap_name = _get_tap_name_for_cleanup() + if tap_name: + _untap_and_remove_directory(tap_name) + + # Clean up before the test + cleanup() + + # Run the test + yield + + # Clean up after the test + cleanup() + + @pytest.fixture(scope='function', params=['true', 'false']) def input_validate(request): os.environ['INPUT_VALIDATE'] = request.param yield del os.environ['INPUT_VALIDATE'] + + +@pytest.fixture +def control_pytest_run(monkeypatch): + """Fixture to control PYTEST_RUN environment variable.""" + original_value = os.environ.get('PYTEST_RUN') + + def set_pytest_run(value): + if value is None: + monkeypatch.delenv('PYTEST_RUN', raising=False) + else: + monkeypatch.setenv('PYTEST_RUN', value) + + yield set_pytest_run + + # Restore original value + if original_value is None: + monkeypatch.delenv('PYTEST_RUN', raising=False) + else: + monkeypatch.setenv('PYTEST_RUN', original_value) diff --git a/tests/release_homebrew/test_release_homebrew.py b/tests/release_homebrew/test_release_homebrew.py index 8033f1d..905e666 100644 --- a/tests/release_homebrew/test_release_homebrew.py +++ b/tests/release_homebrew/test_release_homebrew.py @@ -85,7 +85,7 @@ def test_prepare_homebrew_core_fork(homebrew_core_fork_repo, operating_system): assert branch.endswith('release_homebrew_action_tests') -def test_process_input_formula(operating_system): +def test_process_input_formula(operating_system, org_homebrew_repo): with pytest.raises(FileNotFoundError): main.process_input_formula(formula_file='foo') @@ -108,6 +108,92 @@ def test_process_input_formula(operating_system): assert os.path.isfile(os.path.join(d, 'Formula', 'h', 'hello_world.rb')) +def test_process_input_formula_lizardbyte_actions_special_case(operating_system, org_homebrew_repo, monkeypatch): + """Test that LizardByte/actions is allowed as a special case for CI testing.""" + # Set the INPUT_ORG_HOMEBREW_REPO to LizardByte/actions + monkeypatch.setenv('INPUT_ORG_HOMEBREW_REPO', 'LizardByte/actions') + + formula = main.process_input_formula( + formula_file=os.path.join(os.getcwd(), 'tests', 'release_homebrew', 'Formula', 'hello_world.rb')) + + assert formula == 'hello_world' + # Verify that the tap_repo_name was set correctly + assert main.tap_repo_name == 'lizardbyte/actions' + + +def test_get_tap_name_from_repo_invalid_direct(monkeypatch): + """Test that _get_tap_name_from_repo raises ValueError for invalid repository name.""" + # Set the INPUT_ORG_HOMEBREW_REPO to an invalid name (not starting with 'homebrew-' and not the special case) + invalid_repo = 'SomeOwner/invalid-repo' + monkeypatch.setenv('INPUT_ORG_HOMEBREW_REPO', invalid_repo) + + # Should raise ValueError with a helpful error message + with pytest.raises(ValueError, match='does not follow Homebrew tap naming convention'): + main._get_tap_name_from_repo(invalid_repo) + + +@pytest.mark.parametrize('repo_input, expected_owner, expected_tap_name', [ + # Standard homebrew tap format + ('LizardByte/homebrew-homebrew', 'LizardByte', 'homebrew'), + ('owner/homebrew-tap', 'owner', 'tap'), + ('MyOrg/homebrew-custom', 'MyOrg', 'custom'), + ('user123/homebrew-myformulas', 'user123', 'myformulas'), + # Edge case: longer tap name + ('SomeOrg/homebrew-very-long-tap-name', 'SomeOrg', 'very-long-tap-name'), + # Special case for CI testing + ('LizardByte/actions', 'LizardByte', 'actions'), +]) +def test_get_tap_name_from_repo_valid(capsys, repo_input, expected_owner, expected_tap_name, monkeypatch): + """Test _get_tap_name_from_repo with valid repository inputs.""" + # Set the environment variable for the function to read + monkeypatch.setenv('INPUT_ORG_HOMEBREW_REPO', repo_input) + + owner, tap_name = main._get_tap_name_from_repo(repo_input) + + assert owner == expected_owner + assert tap_name == expected_tap_name + + # Verify output was printed + captured = capsys.readouterr() + assert f'org_homebrew_repo_input: {repo_input}' in captured.out + + +@pytest.mark.parametrize('invalid_repo_input, expected_error_fragment', [ + # Missing 'homebrew-' prefix and not the special case + ('SomeOwner/invalid-repo', 'does not follow Homebrew tap naming convention'), + ('owner/tap', 'does not follow Homebrew tap naming convention'), + ('MyOrg/custom-tap', 'does not follow Homebrew tap naming convention'), + # Various invalid formats + ('BadOrg/random-name', 'does not follow Homebrew tap naming convention'), +]) +def test_get_tap_name_from_repo_invalid(capsys, invalid_repo_input, expected_error_fragment, monkeypatch): + """Test _get_tap_name_from_repo with invalid repository inputs.""" + # Set the environment variable for the function to read + monkeypatch.setenv('INPUT_ORG_HOMEBREW_REPO', invalid_repo_input) + + with pytest.raises(ValueError, match=expected_error_fragment): + main._get_tap_name_from_repo(invalid_repo_input) + + # Verify output was printed before the error + captured = capsys.readouterr() + assert f'org_homebrew_repo_input: {invalid_repo_input}' in captured.out + + +def test_get_tap_name_from_repo_special_case_message(capsys, monkeypatch): + """Test that the special case for LizardByte/actions prints the expected message.""" + repo_input = 'LizardByte/actions' + monkeypatch.setenv('INPUT_ORG_HOMEBREW_REPO', repo_input) + + owner, tap_name = main._get_tap_name_from_repo(repo_input) + + assert owner == 'LizardByte' + assert tap_name == 'actions' + + # Verify special case message was printed + captured = capsys.readouterr() + assert 'Using LizardByte/actions for CI testing (special case)' in captured.out + + def test_is_brew_installed(operating_system): assert main.is_brew_installed() @@ -116,10 +202,6 @@ def test_brew_upgrade(operating_system): assert main.brew_upgrade() -def test_brew_debug(operating_system): - assert main.brew_debug() - - @pytest.mark.parametrize('setup_scenario', [ # Scenario 1: Formula temp dir exists in first location (HOMEBREW_TEMP) {'env': {'HOMEBREW_TEMP': '/tmp/custom'}, 'dirs': ['/tmp/custom'], 'files': ['formula-123']}, @@ -216,19 +298,31 @@ def mock_isdir_side_effect(path): main.find_tmp_dir('formula') -def test_audit_formula(operating_system): +def test_audit_formula(operating_system, org_homebrew_repo): + # Call process_input_formula first to set up the tap + main.process_input_formula( + formula_file=os.path.join(os.getcwd(), 'tests', 'release_homebrew', 'Formula', 'hello_world.rb')) assert main.audit_formula(formula='hello_world') -def test_brew_install_formula(operating_system): +def test_brew_install_formula(operating_system, org_homebrew_repo): + # Call process_input_formula first to set up the tap + main.process_input_formula( + formula_file=os.path.join(os.getcwd(), 'tests', 'release_homebrew', 'Formula', 'hello_world.rb')) assert main.install_formula(formula='hello_world') -def test_test_formula(operating_system): +def test_test_formula(brew_untap, operating_system, org_homebrew_repo): + # Call process_input_formula first to set up the tap + main.process_input_formula( + formula_file=os.path.join(os.getcwd(), 'tests', 'release_homebrew', 'Formula', 'hello_world.rb')) + # Install the formula first to set HOMEBREW_BUILDPATH (required by test_formula) + assert main.install_formula(formula='hello_world') + # Now test the formula assert main.test_formula(formula='hello_world') -def test_main(brew_untap, homebrew_core_fork_repo, input_validate, operating_system): +def test_main(brew_untap, org_homebrew_repo, homebrew_core_fork_repo, input_validate, operating_system): main.args = main._parse_args(args_list=[]) main.main() assert not main.ERROR @@ -248,73 +342,134 @@ def test_main(brew_untap, homebrew_core_fork_repo, input_validate, operating_sys [ ('is_brew_installed', True), ('process_input_formula', 'hello_world'), - ('brew_upgrade', False) + ('brew_upgrade', False), + ], + [], + ), + # Scenario 3: brew test-bot --only-cleanup-before fails + ( + 'brew_test_bot_only_cleanup_before_fails', + [ + ('is_brew_installed', True), + ('process_input_formula', 'hello_world'), + ('brew_upgrade', True), + ('brew_test_bot_only_cleanup_before', False), ], [], ), - # Scenario 3: Brew debug fails + # Scenario 4: brew test-bot --only-setup fails ( - 'brew_debug_fails', + 'brew_test_bot_only_setup_fails', [ ('is_brew_installed', True), ('process_input_formula', 'hello_world'), ('brew_upgrade', True), - ('brew_debug', False) + ('brew_test_bot_only_cleanup_before', True), + ('brew_test_bot_only_setup', False), ], [], ), - # Scenario 4: Audit fails + # Scenario 5: Audit fails ( 'audit_fails', [ ('is_brew_installed', True), ('process_input_formula', 'hello_world'), ('brew_upgrade', True), - ('brew_debug', True), - ('audit_formula', False) + ('brew_test_bot_only_cleanup_before', True), + ('brew_test_bot_only_setup', True), + ('audit_formula', False), + ('brew_test_bot_only_tap_syntax', True), + ('install_formula', True), + ('test_formula', True), + ('brew_test_bot_only_formulae', True), ], ['audit'], ), - # Scenario 5: Install fails + # Scenario 6: brew test-bot --only-tap-syntax fails + ( + 'brew_test_bot_only_tap_syntax_fails', + [ + ('is_brew_installed', True), + ('process_input_formula', 'hello_world'), + ('brew_upgrade', True), + ('brew_test_bot_only_cleanup_before', True), + ('brew_test_bot_only_setup', True), + ('audit_formula', True), + ('brew_test_bot_only_tap_syntax', False), + ('install_formula', True), + ('test_formula', True), + ('brew_test_bot_only_formulae', True), + ], + ['tap-syntax'], + ), + # Scenario 7: Install fails ( 'install_fails', [ ('is_brew_installed', True), ('process_input_formula', 'hello_world'), ('brew_upgrade', True), - ('brew_debug', True), + ('brew_test_bot_only_cleanup_before', True), + ('brew_test_bot_only_setup', True), ('audit_formula', True), - ('install_formula', False) + ('brew_test_bot_only_tap_syntax', True), + ('install_formula', False), + ('test_formula', True), + ('brew_test_bot_only_formulae', True), ], ['install'], ), - # Scenario 6: Test fails + # Scenario 8: Test fails ( 'test_fails', [ ('is_brew_installed', True), ('process_input_formula', 'hello_world'), ('brew_upgrade', True), - ('brew_debug', True), + ('brew_test_bot_only_cleanup_before', True), + ('brew_test_bot_only_setup', True), ('audit_formula', True), + ('brew_test_bot_only_tap_syntax', True), ('install_formula', True), - ('test_formula', False) + ('test_formula', False), + ('brew_test_bot_only_formulae', True), ], ['test'], ), - # Scenario 7: Multiple failures + # Scenario 9: brew test-bot --only-formulae fails + ( + 'brew_test_bot_only_formulae_fails', + [ + ('is_brew_installed', True), + ('process_input_formula', 'hello_world'), + ('brew_upgrade', True), + ('brew_test_bot_only_cleanup_before', True), + ('brew_test_bot_only_setup', True), + ('audit_formula', True), + ('brew_test_bot_only_tap_syntax', True), + ('install_formula', True), + ('test_formula', True), + ('brew_test_bot_only_formulae', False), + ], + ['formulae'], + ), + # Scenario 10: Multiple failures ( 'multiple_failures', [ ('is_brew_installed', True), ('process_input_formula', 'hello_world'), ('brew_upgrade', True), - ('brew_debug', True), + ('brew_test_bot_only_cleanup_before', True), + ('brew_test_bot_only_setup', True), ('audit_formula', False), + ('brew_test_bot_only_tap_syntax', False), ('install_formula', False), - ('test_formula', False) + ('test_formula', False), + ('brew_test_bot_only_formulae', False), ], - ['audit', 'install', 'test'], + ['audit', 'tap-syntax', 'install', 'test', 'formulae'], ), ]) def test_main_error_cases( @@ -393,11 +548,7 @@ def test_prepare_homebrew_core_fork_failure(mock_run, homebrew_core_fork_repo, o @patch('os.path.exists') -def test_process_input_formula_copy_failure(mock_exists, tmp_path, operating_system): - # Create a test formula file - test_formula = tmp_path / "test_formula.rb" - test_formula.write_text("class TestFormula < Formula\nend") - +def test_process_input_formula_copy_failure(mock_exists, test_formula_file, tmp_path, operating_system): # Make the initial file check pass, but the copy verification fail # First call (checking if formula exists): True # Second call (checking if it's a file): True @@ -406,7 +557,7 @@ def test_process_input_formula_copy_failure(mock_exists, tmp_path, operating_sys # Test that the function raises FileNotFoundError when copy verification fails with pytest.raises(FileNotFoundError, match="was not copied"): - main.process_input_formula(formula_file=str(test_formula)) + main.process_input_formula(formula_file=str(test_formula_file)) @patch('actions.release_homebrew.main._run_subprocess') @@ -435,3 +586,161 @@ def side_effect(args_list, *args, **kwargs): call_args = mock_run.call_args_list[0][1]['args_list'] assert 'update' in call_args assert 'upgrade' not in call_args + + +@patch('actions.release_homebrew.main._run_subprocess') +def test_commit_formula_changes(mock_run, tmp_path, monkeypatch, operating_system): + """Test that commit_formula_changes commits the formula to git.""" + # Set up environment variables + monkeypatch.setenv('INPUT_GIT_EMAIL', 'test@example.com') + monkeypatch.setenv('INPUT_GIT_USERNAME', 'Test User') + + # Create a temporary git repo + repo_path = tmp_path / "test_repo" + repo_path.mkdir() + + # Mock _run_subprocess to return True (success) + mock_run.return_value = True + + # Call the function + main.commit_formula_changes( + path=str(repo_path), + formula_filename='test_formula.rb', + message='Test commit message' + ) + + # Verify git config was called + # call.kwargs contains the keyword arguments passed to _run_subprocess + assert any( + 'git' in call.kwargs.get('args_list', []) and + 'config' in call.kwargs.get('args_list', []) and + 'user.email' in call.kwargs.get('args_list', []) + for call in mock_run.call_args_list + ) + assert any( + 'git' in call.kwargs.get('args_list', []) + and 'config' in call.kwargs.get('args_list', []) + and 'user.name' in call.kwargs.get('args_list', []) + for call in mock_run.call_args_list + ) + + # Verify git add was called + assert any( + 'git' in call.kwargs.get('args_list', []) and + 'add' in call.kwargs.get('args_list', []) + for call in mock_run.call_args_list + ) + + # Verify git commit was called + assert any( + 'git' in call.kwargs.get('args_list', []) and + 'commit' in call.kwargs.get('args_list', []) + for call in mock_run.call_args_list + ) + + +@patch('actions.release_homebrew.main._run_subprocess') +def test_commit_formula_changes_no_git_credentials(mock_run, tmp_path, monkeypatch, operating_system): + """Test that commit_formula_changes works without git credentials.""" + # Ensure git credentials are not set + monkeypatch.delenv('INPUT_GIT_EMAIL', raising=False) + monkeypatch.delenv('INPUT_GIT_USERNAME', raising=False) + + # Create a temporary git repo + repo_path = tmp_path / "test_repo" + repo_path.mkdir() + + # Mock _run_subprocess to return True (success) + mock_run.return_value = True + + # Call the function + main.commit_formula_changes( + path=str(repo_path), + formula_filename='test_formula.rb', + message='Test commit message' + ) + + # Verify git config was NOT called since no credentials + assert not any( + 'git' in call.kwargs.get('args_list', []) and + 'config' in call.kwargs.get('args_list', []) + for call in mock_run.call_args_list + ) + + # Verify git add was called + assert any( + 'git' in call.kwargs.get('args_list', []) and + 'add' in call.kwargs.get('args_list', []) + for call in mock_run.call_args_list + ) + + # Verify git commit was called + assert any( + 'git' in call.kwargs.get('args_list', []) and + 'commit' in call.kwargs.get('args_list', []) + for call in mock_run.call_args_list + ) + + +def test_start_group_pytest_mode(capsys, control_pytest_run): + """Test start_group outputs pytest format when PYTEST_RUN is set.""" + control_pytest_run('true') + + main.start_group('Test Group') + + captured = capsys.readouterr() + assert captured.out == '>> Test Group\n' + assert captured.err == '' + + +def test_start_group_github_actions_mode(capsys, control_pytest_run): + """Test start_group outputs GitHub Actions format when PYTEST_RUN is not set.""" + control_pytest_run(None) + + main.start_group('Test Group') + + captured = capsys.readouterr() + assert captured.out == '::group::Test Group\n' + assert captured.err == '' + + +def test_end_group_pytest_mode(capsys, control_pytest_run): + """Test end_group outputs pytest format when PYTEST_RUN is set.""" + control_pytest_run('true') + + main.end_group() + + captured = capsys.readouterr() + assert captured.out == '<< END\n' + assert captured.err == '' + + +def test_end_group_github_actions_mode(capsys, control_pytest_run): + """Test end_group outputs GitHub Actions format when PYTEST_RUN is not set.""" + control_pytest_run(None) + + main.end_group() + + captured = capsys.readouterr() + assert captured.out == '::endgroup::\n' + assert captured.err == '' + + +def test_process_input_formula_skips_commit_for_non_git_repo(capsys, operating_system, org_homebrew_repo, tmp_path): + """Test that process_input_formula skips commit when repo is not a git repository.""" + # Remove the .git directory from org_homebrew_repo to simulate a non-git repo + git_dir = os.path.join(org_homebrew_repo, '.git') + if os.path.isdir(git_dir): + import shutil + shutil.rmtree(git_dir) + + # Process the formula - it should skip the commit + formula = main.process_input_formula( + formula_file=os.path.join(os.getcwd(), 'tests', 'release_homebrew', 'Formula', 'hello_world.rb')) + + assert formula == 'hello_world' + + # Check that the "Skipping commit" message was printed + captured = capsys.readouterr() + assert 'Skipping commit for' in captured.out + assert '(not a git repository)' in captured.out From 8444af01a5723d9e933817c99d0be8d4543c1475 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 17 Dec 2025 18:37:11 -0500 Subject: [PATCH 2/9] Add support for custom PR branches in release_homebrew Refactored the action and main script to allow specifying custom head and base branches for both the org homebrew repo and homebrew-core fork when creating pull requests. Updated documentation and input parameters to reflect these changes. Enhanced test coverage to validate the new branch handling logic. --- actions/release_homebrew/README.md | 36 ++--- actions/release_homebrew/action.yml | 67 +++++++--- actions/release_homebrew/ci-matrix.json | 4 +- actions/release_homebrew/main.py | 126 +++++++++++++----- .../release_homebrew/test_release_homebrew.py | 71 ++++++++-- 5 files changed, 229 insertions(+), 75 deletions(-) diff --git a/actions/release_homebrew/README.md b/actions/release_homebrew/README.md index f689f61..f3a8d88 100644 --- a/actions/release_homebrew/README.md +++ b/actions/release_homebrew/README.md @@ -32,19 +32,22 @@ steps: ## 📥 Inputs -| Name | Description | Default | Required | -|-----------------------------|-----------------------------------------------------------------------------------------|--------------------------------|----------| -| contribute_to_homebrew_core | Whether to contribute to homebrew-core. | `false` | `false` | -| formula_file | The full path to the formula file. | | `true` | -| git_email | The email to use for the commit. | | `true` | -| git_username | The username to use for the commit. | | `true` | -| homebrew_core_fork_repo | The forked homebrew-core repository to publish to. | `LizardByte/homebrew-core` | `false` | -| org_homebrew_repo | The target repository to publish to. | `LizardByte/homebrew-homebrew` | `false` | -| org_homebrew_repo_branch | The target repository branch to publish to. | | `false` | -| publish | Whether to publish the release. | `false` | `false` | -| token | GitHub Token. This is required when `publish` is enabled. | | `false` | -| upstream_homebrew_core_repo | The upstream homebrew-core repository that the fork is based on. Must be a GitHub repo. | `Homebrew/homebrew-core` | `false` | -| validate | Whether to validate the formula. | `true` | `false` | +| Name | Description | Default | Required | +|-------------------------------|-------------------------------------------------------------------------------------------|--------------------------------|----------| +| contribute_to_homebrew_core | Whether to contribute to homebrew-core. | `false` | `false` | +| formula_file | The full path to the formula file. | | `true` | +| git_email | The email to use for the commit. | | `true` | +| git_username | The username to use for the commit. | | `true` | +| homebrew_core_fork_repo | The forked homebrew-core repository to publish to. | `LizardByte/homebrew-core` | `false` | +| homebrew_core_base_branch | The base branch of the homebrew-core fork to create PR against. | `main` | `false` | +| homebrew_core_head_branch | The head branch of the homebrew-core fork to create for the PR. If empty, auto-generated. | | `false` | +| org_homebrew_repo | The target repository to publish to. | `LizardByte/homebrew-homebrew` | `false` | +| org_homebrew_repo_base_branch | The base branch of the target repository to create PR against. | | `false` | +| org_homebrew_repo_head_branch | The head branch of the target repository to create for the PR. If empty, auto-generated. | | `false` | +| publish | Whether to publish the release. | `false` | `false` | +| token | GitHub Token. This is required when `publish` is enabled. | | `false` | +| upstream_homebrew_core_repo | The upstream homebrew-core repository that the fork is based on. Must be a GitHub repo. | `Homebrew/homebrew-core` | `false` | +| validate | Whether to validate the formula. | `true` | `false` | > [!NOTE] > `org_homebrew_repo` repo name should conform to the documentation. @@ -71,8 +74,11 @@ steps: git_email: ${{ secrets.GIT_EMAIL }} git_username: ${{ secrets.GIT_USERNAME }} homebrew_core_fork_repo: repo_owner/homebrew-core + homebrew_core_base_branch: main + homebrew_core_head_branch: my-custom-branch # optional, will be auto-generated if not specified org_homebrew_repo: repo_owner/repo_name - org_homebrew_repo_branch: master + org_homebrew_repo_base_branch: master + org_homebrew_repo_head_branch: my-custom-branch # optional, will be auto-generated if not specified publish: true # you probably want to use some conditional logic here token: ${{ secrets.PAT }} # required to publish upstream_homebrew_core_repo: Homebrew/homebrew-core @@ -118,7 +124,7 @@ jobs: git_email: ${{ secrets.GIT_EMAIL }} git_username: ${{ secrets.GIT_USERNAME }} org_homebrew_repo: repo_owner/repo_name - org_homebrew_repo_branch: main + org_homebrew_repo_base_branch: main publish: ${{ matrix.publish }} token: ${{ secrets.PAT }} ``` diff --git a/actions/release_homebrew/action.yml b/actions/release_homebrew/action.yml index 780ace4..c40f08a 100644 --- a/actions/release_homebrew/action.yml +++ b/actions/release_homebrew/action.yml @@ -25,6 +25,16 @@ inputs: description: 'The forked homebrew-core repository to publish to.' default: 'LizardByte/homebrew-core' required: false + homebrew_core_base_branch: + description: 'The base branch of the homebrew-core fork to create PR against.' + default: 'main' + required: false + homebrew_core_head_branch: + description: >- + The head branch of the homebrew-core fork to create for the PR. + If empty, a branch name will be auto-generated. + default: '' + required: false org_homebrew_repo: description: | The target repository to publish to. @@ -32,8 +42,14 @@ inputs: See: https://docs.brew.sh/Taps#repository-naming-conventions-and-assumptions default: 'LizardByte/homebrew-homebrew' required: false - org_homebrew_repo_branch: - description: 'The target repository branch to publish to.' + org_homebrew_repo_base_branch: + description: 'The base branch of the target repository to create PR against.' + default: '' + required: false + org_homebrew_repo_head_branch: + description: >- + The head branch of the target repository to create for the PR. + If empty, a branch name will be auto-generated. default: '' required: false publish: @@ -115,7 +131,7 @@ runs: uses: actions/checkout@v6 with: repository: ${{ inputs.org_homebrew_repo }} - ref: ${{ inputs.org_homebrew_repo_branch }} + ref: ${{ inputs.org_homebrew_repo_base_branch }} path: ${{ github.workspace }}/release_homebrew_action/org_homebrew_repo persist-credentials: false # otherwise, the token used is the GITHUB_TOKEN, instead of the personal token fetch-depth: 1 @@ -125,6 +141,7 @@ runs: uses: actions/checkout@v6 with: repository: ${{ inputs.homebrew_core_fork_repo }} + ref: ${{ inputs.homebrew_core_base_branch }} path: ${{ github.workspace }}/release_homebrew_action/homebrew_core_fork_repo persist-credentials: false # otherwise, the token used is the GITHUB_TOKEN, instead of the personal token fetch-depth: 1 @@ -136,6 +153,8 @@ runs: INPUT_GIT_EMAIL: ${{ inputs.git_email }} INPUT_GIT_USERNAME: ${{ inputs.git_username }} INPUT_ORG_HOMEBREW_REPO: ${{ inputs.org_homebrew_repo }} + INPUT_ORG_HOMEBREW_REPO_HEAD_BRANCH: ${{ inputs.org_homebrew_repo_head_branch }} + INPUT_HOMEBREW_CORE_HEAD_BRANCH: ${{ inputs.homebrew_core_head_branch }} INPUT_UPSTREAM_HOMEBREW_CORE_REPO: ${{ inputs.upstream_homebrew_core_repo }} INPUT_VALIDATE: ${{ inputs.validate }} id: homebrew-tests @@ -160,7 +179,7 @@ runs: with: author_email: ${{ inputs.git_email }} author_name: ${{ inputs.git_username }} - branch: ${{ inputs.org_homebrew_repo_branch }} # commit to target branch + branch: ${{ steps.homebrew-tests.outputs.org_homebrew_repo_branch }} directory: ${{ github.workspace }}/release_homebrew_action/org_homebrew_repo github_token: ${{ inputs.token }} message: "chore: update ${{ github.repository }} to ${{ github.sha }}" @@ -182,15 +201,19 @@ runs: - name: Create Pull Request (org homebrew repo) env: GH_TOKEN: ${{ inputs.token }} - INPUT_ORG_HOMEBREW_REPO_BRANCH: ${{ inputs.org_homebrew_repo_branch }} + ORG_HOMEBREW_REPO_BRANCH: ${{ steps.homebrew-tests.outputs.org_homebrew_repo_branch }} + ORG_HOMEBREW_REPO_BASE_BRANCH: ${{ inputs.org_homebrew_repo_base_branch }} INPUT_ORG_HOMEBREW_REPO: ${{ inputs.org_homebrew_repo }} if: ${{ inputs.publish == 'true' }} shell: bash working-directory: ${{ github.workspace }}/release_homebrew_action/org_homebrew_repo run: | + # Use default branch 'master' if base branch is not specified + BASE_BRANCH=${ORG_HOMEBREW_REPO_BASE_BRANCH:-master} + # Check if a pull request already exists with the same head branch PR_EXISTS=$(gh pr list \ - --head ${INPUT_ORG_HOMEBREW_REPO_BRANCH} \ + --head ${ORG_HOMEBREW_REPO_BRANCH} \ --repo ${INPUT_ORG_HOMEBREW_REPO}) # If the pull request does not exist, create it @@ -199,12 +222,11 @@ runs: # https://cli.github.com/manual/gh_pr_create gh pr create \ - --base master \ - --head ${INPUT_ORG_HOMEBREW_REPO_BRANCH} \ + --base ${BASE_BRANCH} \ + --head ${ORG_HOMEBREW_REPO_BRANCH} \ --title "chore: update ${{ github.repository }} to ${{ github.sha }}" \ --body \ "Created by the LizardByte [release_homebrew](https://github.com/LizardByte/actions) action" \ - --no-maintainer-edit \ --repo ${INPUT_ORG_HOMEBREW_REPO} else echo "Pull request already exists" @@ -214,29 +236,38 @@ runs: env: GH_TOKEN: ${{ inputs.token }} HOMEBREW_CORE_BRANCH: ${{ steps.homebrew-tests.outputs.homebrew_core_branch }} + HOMEBREW_CORE_BASE_BRANCH: ${{ inputs.homebrew_core_base_branch }} + HOMEBREW_CORE_FORK_REPO: ${{ inputs.homebrew_core_fork_repo }} INPUT_UPSTREAM_HOMEBREW_CORE_REPO: ${{ inputs.upstream_homebrew_core_repo }} if: ${{ inputs.contribute_to_homebrew_core == 'true' && inputs.publish == 'true' }} shell: bash working-directory: ${{ github.workspace }}/release_homebrew_action/homebrew_core_fork_repo run: | - # Check if a pull request already exists with the same head branch + # Use default branch 'main' if base branch is not specified + BASE_BRANCH=${HOMEBREW_CORE_BASE_BRANCH:-main} + + # Extract fork owner from fork repo (e.g., "owner/homebrew-core" -> "owner") + FORK_OWNER=$(echo ${HOMEBREW_CORE_FORK_REPO} | cut -d'/' -f1) + + # Check if a pull request already exists from fork to upstream + # We check in the upstream repo for PRs from our fork PR_EXISTS=$(gh pr list \ - --head ${HOMEBREW_CORE_BRANCH} \ + --head ${FORK_OWNER}:${HOMEBREW_CORE_BRANCH} \ --repo ${INPUT_UPSTREAM_HOMEBREW_CORE_REPO}) # If the pull request does not exist, create it if [[ -z "$PR_EXISTS" ]]; then - echo "Creating pull request" + echo "Creating pull request from ${HOMEBREW_CORE_FORK_REPO} to ${INPUT_UPSTREAM_HOMEBREW_CORE_REPO}" + # Create PR in the upstream repo, from our fork + # Note: we're in the fork's working directory, so gh will use the fork's context # https://cli.github.com/manual/gh_pr_create gh pr create \ - --base master \ - --head ${HOMEBREW_CORE_BRANCH} \ + --repo ${INPUT_UPSTREAM_HOMEBREW_CORE_REPO} \ + --base ${BASE_BRANCH} \ + --head ${FORK_OWNER}:${HOMEBREW_CORE_BRANCH} \ --title "${{ github.repository }}: update to ${{ github.sha }}" \ - --body \ - "Created by the LizardByte [release_homebrew](https://github.com/LizardByte/actions) action" \ - --no-maintainer-edit \ - --repo ${INPUT_UPSTREAM_HOMEBREW_CORE_REPO} + --body "Created by the LizardByte [release_homebrew](https://github.com/LizardByte/actions) action" else echo "Pull request already exists" fi diff --git a/actions/release_homebrew/ci-matrix.json b/actions/release_homebrew/ci-matrix.json index 484e857..d443b52 100644 --- a/actions/release_homebrew/ci-matrix.json +++ b/actions/release_homebrew/ci-matrix.json @@ -7,7 +7,7 @@ "git_email": "${ secrets.GH_BOT_EMAIL }", "git_username": "${ secrets.GH_BOT_NAME }", "org_homebrew_repo": "LizardByte/homebrew-homebrew", - "org_homebrew_repo_branch": "master", + "org_homebrew_repo_base_branch": "master", "publish": false, "token": "${ secrets.GH_BOT_TOKEN }", "upstream_homebrew_core_repo": "LizardByte/homebrew-core" @@ -21,7 +21,7 @@ "git_email": "${ secrets.GH_BOT_EMAIL }", "git_username": "${ secrets.GH_BOT_NAME }", "org_homebrew_repo": "LizardByte/homebrew-homebrew", - "org_homebrew_repo_branch": "master", + "org_homebrew_repo_base_branch": "master", "publish": false, "token": "${ secrets.GH_BOT_TOKEN }", "upstream_homebrew_core_repo": "LizardByte/homebrew-core" diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index 9983e69..2205649 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -239,18 +239,55 @@ def commit_formula_changes( end_group() -def prepare_homebrew_core_fork( +def prepare_repo_branch( branch_suffix: str, path: str, -) -> None: + repo_type: str, + custom_branch_env_var: str, + output_name: str, + upstream_repo: Optional[str] = None, + upstream_branch: str = 'main', +) -> str: + """ + Prepare a repository by creating or checking out a branch for the PR. + + Parameters + ---------- + branch_suffix : str + Suffix to use for the branch name (typically the formula name). + path : str + Path to the git repository. + repo_type : str + Type of repository for logging (e.g., 'org homebrew repo', 'Homebrew/homebrew-core fork'). + custom_branch_env_var : str + Environment variable name to check for custom branch name. + output_name : str + Name of the GitHub Action output to set with the branch name. + upstream_repo : Optional[str] + If provided, add this as upstream remote and sync with it. + upstream_branch : str + Branch name in the upstream repo to sync with (default: 'main'). + + Returns + ------- + str + The branch name that was created or checked out. + """ global ERROR og_error = ERROR - start_group('Preparing Homebrew/homebrew-core fork') + start_group(f'Preparing {repo_type} branch') - # checkout a new branch - branch_name = f'release_homebrew_action/{branch_suffix}' + # Check if a custom head branch was specified + custom_branch = os.getenv(custom_branch_env_var, '').strip() + + if custom_branch: + branch_name = custom_branch + print(f'Using custom head branch: {branch_name}') + else: + branch_name = f'release_homebrew_action/{branch_suffix}' + print(f'Auto-generating head branch: {branch_name}') # Check if we're already on the target branch process = subprocess.run( @@ -284,40 +321,44 @@ def prepare_homebrew_core_fork( end_group() raise SystemExit(1, f'::error:: Failed to create or checkout branch {branch_name}') - # add the upstream remote - print('Adding upstream remote') - _run_subprocess( - args_list=[ - 'git', - 'remote', - 'add', - 'upstream', - f'https://github.com/{os.environ["INPUT_UPSTREAM_HOMEBREW_CORE_REPO"]}' - ], - cwd=path, - ) + # If upstream repo is provided, sync with it + if upstream_repo: + # add the upstream remote + print('Adding upstream remote') + _run_subprocess( + args_list=[ + 'git', + 'remote', + 'add', + 'upstream', + f'https://github.com/{upstream_repo}' + ], + cwd=path, + ) - # fetch the upstream remote - print('Fetching upstream remote') - _run_subprocess( - args_list=['git', 'fetch', 'upstream', '--depth=1'], - cwd=path, - ) + # fetch the upstream remote + print('Fetching upstream remote') + _run_subprocess( + args_list=['git', 'fetch', 'upstream', '--depth=1'], + cwd=path, + ) - # hard reset - print('Hard resetting to upstream/master') - _run_subprocess( - args_list=['git', 'reset', '--hard', 'upstream/master'], - cwd=path, - ) + # hard reset + print(f'Hard resetting to upstream/{upstream_branch}') + _run_subprocess( + args_list=['git', 'reset', '--hard', f'upstream/{upstream_branch}'], + cwd=path, + ) set_github_action_output( - output_name='homebrew_core_branch', + output_name=output_name, output_value=branch_name ) end_group() + return branch_name + def _get_tap_name_from_repo(org_homebrew_repo_input: str) -> tuple[str, str]: """ @@ -417,8 +458,31 @@ def process_input_formula(formula_file: str) -> str: end_group() + # Prepare branches for both repositories + # Get base branches from inputs, with defaults + org_base_branch = os.getenv('INPUT_ORG_HOMEBREW_REPO_BASE_BRANCH', 'master') + homebrew_core_base_branch = os.getenv('INPUT_HOMEBREW_CORE_BASE_BRANCH', 'main') + + prepare_repo_branch( + branch_suffix=formula, + path=org_homebrew_repo, + repo_type='org homebrew repo', + custom_branch_env_var='INPUT_ORG_HOMEBREW_REPO_HEAD_BRANCH', + output_name='org_homebrew_repo_branch', + upstream_repo=None, # No upstream syncing for org repo + upstream_branch=org_base_branch, # Not used, but for consistency + ) + if os.getenv('INPUT_CONTRIBUTE_TO_HOMEBREW_CORE').lower() == 'true': - prepare_homebrew_core_fork(branch_suffix=formula, path=homebrew_core_fork_repo) + prepare_repo_branch( + branch_suffix=formula, + path=homebrew_core_fork_repo, + repo_type='Homebrew/homebrew-core fork', + custom_branch_env_var='INPUT_HOMEBREW_CORE_HEAD_BRANCH', + output_name='homebrew_core_branch', + upstream_repo=os.getenv('INPUT_UPSTREAM_HOMEBREW_CORE_REPO'), + upstream_branch=homebrew_core_base_branch, + ) # copy the formula file to the directories start_group(f'Copying formula {formula} to tap directories') diff --git a/tests/release_homebrew/test_release_homebrew.py b/tests/release_homebrew/test_release_homebrew.py index 905e666..a324f36 100644 --- a/tests/release_homebrew/test_release_homebrew.py +++ b/tests/release_homebrew/test_release_homebrew.py @@ -74,15 +74,63 @@ def test_get_brew_repository(operating_system): assert main.get_brew_repository() -def test_prepare_homebrew_core_fork(homebrew_core_fork_repo, operating_system): - main.prepare_homebrew_core_fork( - branch_suffix='release_homebrew_action_tests', - path=homebrew_core_fork_repo +@pytest.mark.parametrize( + 'repo_fixture, branch_suffix, repo_type, custom_branch_env_var, output_name, upstream_repo, upstream_branch', + [ + # Test homebrew-core fork with upstream syncing + ( + 'homebrew_core_fork_repo', + 'release_homebrew_action_tests', + 'Homebrew/homebrew-core fork', + 'INPUT_HOMEBREW_CORE_HEAD_BRANCH', + 'homebrew_core_branch', + os.environ.get('INPUT_UPSTREAM_HOMEBREW_CORE_REPO', 'Homebrew/homebrew-core'), + 'main', + ), + # Test org homebrew repo without upstream syncing + ( + 'org_homebrew_repo', + 'test_formula', + 'org homebrew repo', + 'INPUT_ORG_HOMEBREW_REPO_HEAD_BRANCH', + 'org_homebrew_repo_branch', + None, + 'master', + ), + ] +) +def test_prepare_repo_branch( + repo_fixture, + branch_suffix, + repo_type, + custom_branch_env_var, + output_name, + upstream_repo, + upstream_branch, + request, + operating_system +): + """Test prepare_repo_branch for different repository types.""" + # Get the actual path from the fixture + repo_path = request.getfixturevalue(repo_fixture) + + # Call prepare_repo_branch with appropriate parameters + branch = main.prepare_repo_branch( + branch_suffix=branch_suffix, + path=repo_path, + repo_type=repo_type, + custom_branch_env_var=custom_branch_env_var, + output_name=output_name, + upstream_repo=upstream_repo, + upstream_branch=upstream_branch, ) + # assert that the branch name was returned + assert branch.endswith(branch_suffix) + # assert that the current branch is the branch we created - branch = get_current_branch(cwd=homebrew_core_fork_repo) - assert branch.endswith('release_homebrew_action_tests') + current_branch = get_current_branch(cwd=repo_path) + assert current_branch.endswith(branch_suffix) def test_process_input_formula(operating_system, org_homebrew_repo): @@ -530,16 +578,21 @@ def test_main_skip_validate(monkeypatch): @patch('actions.release_homebrew.main._run_subprocess') -def test_prepare_homebrew_core_fork_failure(mock_run, homebrew_core_fork_repo, operating_system): +def test_prepare_repo_branch_failure(mock_run, homebrew_core_fork_repo, operating_system): # Mock _run_subprocess to return False for the first call (branch creation) # and False for the second call (branch checkout) mock_run.return_value = False # Test that the function raises SystemExit when both branch operations fail with pytest.raises(SystemExit): - main.prepare_homebrew_core_fork( + main.prepare_repo_branch( branch_suffix='release_homebrew_action_tests', - path=homebrew_core_fork_repo + path=homebrew_core_fork_repo, + repo_type='Homebrew/homebrew-core fork', + custom_branch_env_var='INPUT_HOMEBREW_CORE_HEAD_BRANCH', + output_name='homebrew_core_branch', + upstream_repo=os.environ['INPUT_UPSTREAM_HOMEBREW_CORE_REPO'], + upstream_branch='main', ) # Verify the function attempted to run git commands From ee3c2e971f1d9d7acdfd3f418295eaece035c28a Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 17 Dec 2025 19:22:36 -0500 Subject: [PATCH 3/9] Add mocks for makedirs and copy2 in formula copy test Enhanced the test_process_input_formula_copy_failure test by mocking os.makedirs and shutil.copy2 to simulate directory creation and file copying. This ensures the test accurately simulates a copy failure scenario without performing actual file operations. --- .../release_homebrew/test_release_homebrew.py | 65 ++++++++++++++++++- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/tests/release_homebrew/test_release_homebrew.py b/tests/release_homebrew/test_release_homebrew.py index a324f36..cc90230 100644 --- a/tests/release_homebrew/test_release_homebrew.py +++ b/tests/release_homebrew/test_release_homebrew.py @@ -133,6 +133,35 @@ def test_prepare_repo_branch( assert current_branch.endswith(branch_suffix) +def test_prepare_repo_branch_custom_branch(capsys, org_homebrew_repo, monkeypatch, operating_system): + """Test prepare_repo_branch with a custom head branch specified via environment variable.""" + # Set a custom branch name via environment variable + custom_branch_name = 'my-custom-feature-branch' + monkeypatch.setenv('INPUT_ORG_HOMEBREW_REPO_HEAD_BRANCH', custom_branch_name) + + # Call prepare_repo_branch + branch = main.prepare_repo_branch( + branch_suffix='test_formula', + path=org_homebrew_repo, + repo_type='org homebrew repo', + custom_branch_env_var='INPUT_ORG_HOMEBREW_REPO_HEAD_BRANCH', + output_name='org_homebrew_repo_branch', + upstream_repo=None, + upstream_branch='master', + ) + + # Assert that the custom branch name was used + assert branch == custom_branch_name + + # Assert that the current branch is the custom branch + current_branch = get_current_branch(cwd=org_homebrew_repo) + assert current_branch == custom_branch_name + + # Verify the log message was printed + captured = capsys.readouterr() + assert f'Using custom head branch: {custom_branch_name}' in captured.out + + def test_process_input_formula(operating_system, org_homebrew_repo): with pytest.raises(FileNotFoundError): main.process_input_formula(formula_file='foo') @@ -600,14 +629,44 @@ def test_prepare_repo_branch_failure(mock_run, homebrew_core_fork_repo, operatin assert mock_run.call_count >= 1 +@patch('actions.release_homebrew.main.prepare_repo_branch') +@patch('actions.release_homebrew.main._run_subprocess') +@patch('os.chmod') @patch('os.path.exists') -def test_process_input_formula_copy_failure(mock_exists, test_formula_file, tmp_path, operating_system): - # Make the initial file check pass, but the copy verification fail +@patch('os.makedirs') +@patch('shutil.copy2') +def test_process_input_formula_copy_failure( + mock_copy, + mock_makedirs, + mock_exists, + mock_chmod, + mock_run_subprocess, + mock_prepare_repo_branch, + test_formula_file, + tmp_path, + operating_system +): + # Make the initial file checks pass # First call (checking if formula exists): True # Second call (checking if it's a file): True - # All subsequent calls (checking if copies exist): False + # All subsequent calls (checking if copies exist): False to simulate copy failure mock_exists.side_effect = [True, True] + [False] * 10 + # Mock makedirs to do nothing (simulate successful directory creation) + mock_makedirs.return_value = None + + # Mock copy2 to do nothing (simulate copy without actually copying) + mock_copy.return_value = None + + # Mock chmod to do nothing (simulate successful permission change) + mock_chmod.return_value = None + + # Mock prepare_repo_branch to return a branch name without doing actual git operations + mock_prepare_repo_branch.return_value = 'release_homebrew_action/test_formula' + + # Mock _run_subprocess to simulate successful brew tap + mock_run_subprocess.return_value = True + # Test that the function raises FileNotFoundError when copy verification fails with pytest.raises(FileNotFoundError, match="was not copied"): main.process_input_formula(formula_file=str(test_formula_file)) From fded8a9c138966f79dfac11f6cdd5c01505927ca Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 17 Dec 2025 20:49:23 -0500 Subject: [PATCH 4/9] Use dynamic commit messages for Homebrew formula updates Updated the release_homebrew action to generate commit messages based on whether a formula is new or updated, and to include the version number when available. The commit message is now set as an output and used in subsequent commit and pull request steps, replacing the previous static message format. --- actions/release_homebrew/action.yml | 13 ++++-- actions/release_homebrew/main.py | 69 +++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/actions/release_homebrew/action.yml b/actions/release_homebrew/action.yml index c40f08a..633ccd1 100644 --- a/actions/release_homebrew/action.yml +++ b/actions/release_homebrew/action.yml @@ -75,6 +75,9 @@ outputs: testpath: description: "The path to Homebrew's temporary test directory." value: ${{ steps.homebrew-tests.outputs.testpath }} + commit_message: + description: "The commit message used for the formula update." + value: ${{ steps.homebrew-tests.outputs.commit_message }} runs: using: "composite" @@ -182,7 +185,7 @@ runs: branch: ${{ steps.homebrew-tests.outputs.org_homebrew_repo_branch }} directory: ${{ github.workspace }}/release_homebrew_action/org_homebrew_repo github_token: ${{ inputs.token }} - message: "chore: update ${{ github.repository }} to ${{ github.sha }}" + message: "chore: ${{ steps.homebrew-tests.outputs.commit_message }}" repository: ${{ inputs.org_homebrew_repo }} - name: GitHub Commit & Push (homebrew-core) @@ -195,11 +198,12 @@ runs: directory: ${{ github.workspace }}/release_homebrew_action/homebrew_core_fork_repo force: true # need to force since the branch is sometimes reset github_token: ${{ inputs.token }} - message: "${{ github.repository }}: update to ${{ github.sha }}" + message: "${{ steps.homebrew-tests.outputs.commit_message }}" repository: ${{ inputs.homebrew_core_fork_repo }} - name: Create Pull Request (org homebrew repo) env: + COMMIT_MESSAGE: ${{ steps.homebrew-tests.outputs.commit_message }} GH_TOKEN: ${{ inputs.token }} ORG_HOMEBREW_REPO_BRANCH: ${{ steps.homebrew-tests.outputs.org_homebrew_repo_branch }} ORG_HOMEBREW_REPO_BASE_BRANCH: ${{ inputs.org_homebrew_repo_base_branch }} @@ -224,7 +228,7 @@ runs: gh pr create \ --base ${BASE_BRANCH} \ --head ${ORG_HOMEBREW_REPO_BRANCH} \ - --title "chore: update ${{ github.repository }} to ${{ github.sha }}" \ + --title "chore: ${COMMIT_MESSAGE}" \ --body \ "Created by the LizardByte [release_homebrew](https://github.com/LizardByte/actions) action" \ --repo ${INPUT_ORG_HOMEBREW_REPO} @@ -234,6 +238,7 @@ runs: - name: Create Pull Request (homebrew-core) env: + COMMIT_MESSAGE: ${{ steps.homebrew-tests.outputs.commit_message }} GH_TOKEN: ${{ inputs.token }} HOMEBREW_CORE_BRANCH: ${{ steps.homebrew-tests.outputs.homebrew_core_branch }} HOMEBREW_CORE_BASE_BRANCH: ${{ inputs.homebrew_core_base_branch }} @@ -266,7 +271,7 @@ runs: --repo ${INPUT_UPSTREAM_HOMEBREW_CORE_REPO} \ --base ${BASE_BRANCH} \ --head ${FORK_OWNER}:${HOMEBREW_CORE_BRANCH} \ - --title "${{ github.repository }}: update to ${{ github.sha }}" \ + --title "${COMMIT_MESSAGE}" \ --body "Created by the LizardByte [release_homebrew](https://github.com/LizardByte/actions) action" else echo "Pull request already exists" diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index 2205649..8e42c52 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -533,14 +533,67 @@ def process_input_formula(formula_file: str) -> str: end_group() + # Extract version from the formula file + version = None + tag_found = False + try: + with open(formula_file, 'r') as f: + for line in f: + # Look for version line in formula (e.g., version "1.2.3") + if 'version' in line.lower() and '"' in line: + # Extract version string between quotes + start = line.find('"') + end = line.find('"', start + 1) + if start != -1 and end != -1: + version = line[start + 1:end] + break + # Fallback to tag if version not found (only process first occurrence) + elif not tag_found and version is None and 'tag:' in line.lower() and '"' in line: + # Extract tag string between quotes (e.g., tag: "v1.2.3") + start = line.find('"') + end = line.find('"', start + 1) + if start != -1 and end != -1: + version = line[start + 1:end] + tag_found = True + # Don't break here, keep looking for version in case it comes later + except Exception as e: + print(f'Could not extract version from formula: {e}') + # Commit changes to the tap directories - github_sha = os.getenv('GITHUB_SHA', '') - commit_message = f'Add/Update {formula} formula' - if github_sha: - commit_message = f'{commit_message} ({github_sha[:7]})' + commit_messages = {} for formula_dir, repo_path in tap_dir_to_repo.items(): if os.path.isdir(os.path.join(repo_path, '.git')): + # Check if the formula file already existed in the git repository + # to determine if this is an add or update + formula_path = os.path.join(formula_dir, formula_filename) + + # Use git ls-files to check if the file is tracked + check_tracked = subprocess.run( + ['git', 'ls-files', '--error-unmatch', formula_path], + cwd=repo_path, + capture_output=True, + ) + + # If exit code is 0, file is tracked (update), otherwise it's new (add) + is_new = check_tracked.returncode != 0 + + # Generate commit message based on whether it's new or update + if is_new: + # New formula format: "foobar 7.3 (new formula)" + if version: + commit_message = f'{formula} {version} (new formula)' + else: + commit_message = f'{formula} (new formula)' + else: + # Update format: "foobar 7.3" + if version: + commit_message = f'{formula} {version}' + else: + commit_message = f'{formula} (update)' + + commit_messages[repo_path] = commit_message + commit_formula_changes( path=repo_path, formula_filename=formula_filename, @@ -549,6 +602,14 @@ def process_input_formula(formula_file: str) -> str: else: print(f'Skipping commit for {repo_path} (not a git repository)') + # Set commit messages as outputs + # Use the org_homebrew_repo commit message as the primary output + if org_homebrew_repo in commit_messages: + set_github_action_output( + output_name='commit_message', + output_value=commit_messages[org_homebrew_repo] + ) + return formula From 127b7795436b5b8860d4093d039f1f642adb6b1f Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 17 Dec 2025 21:01:26 -0500 Subject: [PATCH 5/9] Refactor formula handling and add unit tests Extracted formula version extraction, commit message generation, formula tracking, and file copying into separate functions in main.py for better modularity and testability. Added comprehensive unit tests for these new functions in test_release_homebrew.py, improving code coverage and reliability. --- actions/release_homebrew/main.py | 206 ++++++++++++------ .../release_homebrew/test_release_homebrew.py | 171 +++++++++++++++ 2 files changed, 310 insertions(+), 67 deletions(-) diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index 8e42c52..cdd45e5 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -360,6 +360,141 @@ def prepare_repo_branch( return branch_name +def extract_version_from_formula(formula_file: str) -> Optional[str]: + """ + Extract version from a Homebrew formula file. + + Looks for 'version "x.y.z"' first, then falls back to 'tag: "vx.y.z"'. + + Parameters + ---------- + formula_file : str + Path to the formula file. + + Returns + ------- + Optional[str] + The version string if found, None otherwise. + """ + version = None + tag_found = False + try: + with open(formula_file, 'r') as f: + for line in f: + # Look for version line in formula (e.g., version "1.2.3") + if 'version' in line.lower() and '"' in line: + # Extract version string between quotes + start = line.find('"') + end = line.find('"', start + 1) + if start != -1 and end != -1: + version = line[start + 1:end] + break + # Fallback to tag if version not found (only process first occurrence) + elif not tag_found and version is None and 'tag:' in line.lower() and '"' in line: + # Extract tag string between quotes (e.g., tag: "v1.2.3") + start = line.find('"') + end = line.find('"', start + 1) + if start != -1 and end != -1: + version = line[start + 1:end] + tag_found = True + # Don't break here, keep looking for version in case it comes later + except Exception as e: + print(f'Could not extract version from formula: {e}') + + return version + + +def generate_commit_message(formula: str, version: Optional[str], is_new: bool) -> str: + """ + Generate a commit message for a formula update or addition. + + Parameters + ---------- + formula : str + The formula name. + version : Optional[str] + The version string, if available. + is_new : bool + True if this is a new formula, False if it's an update. + + Returns + ------- + str + The generated commit message. + """ + if is_new: + # New formula format: "foobar 7.3 (new formula)" + if version: + return f'{formula} {version} (new formula)' + else: + return f'{formula} (new formula)' + else: + # Update format: "foobar 7.3" + if version: + return f'{formula} {version}' + else: + return f'{formula}' + + +def is_formula_tracked(formula_path: str, repo_path: str) -> bool: + """ + Check if a formula file is tracked in a git repository. + + Parameters + ---------- + formula_path : str + Path to the formula file. + repo_path : str + Path to the git repository root. + + Returns + ------- + bool + True if the file is tracked, False otherwise. + """ + check_tracked = subprocess.run( + ['git', 'ls-files', '--error-unmatch', formula_path], + cwd=repo_path, + capture_output=True, + ) + return check_tracked.returncode == 0 + + +def copy_formula_to_directories( + formula_file: str, + formula_filename: str, + tap_dirs: list, +) -> None: + """ + Copy a formula file to multiple tap directories and set permissions. + + Parameters + ---------- + formula_file : str + Path to the source formula file. + formula_filename : str + Name of the formula file. + tap_dirs : list + List of destination directories. + """ + for d in tap_dirs: + print(f'Copying {formula_filename} to {d}') + os.makedirs(d, exist_ok=True) + dest_file = os.path.join(d, formula_filename) + shutil.copy2(formula_file, dest_file) + + if not os.path.exists(dest_file): + raise FileNotFoundError(f'::error:: Formula file {formula_filename} was not copied to {d}') + + # Set permissions required by Homebrew (rw-r--r--) + # Owner: read + write, Group: read, Others: read + # Homebrew requires formula files to be world-readable (brew audit enforces this) + # Only owner has write permission, complying with security best practices + # Formula files are Ruby scripts that should not be executable + os.chmod(dest_file, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH) + print(f'Copied {formula_filename} to {d}') + + def _get_tap_name_from_repo(org_homebrew_repo_input: str) -> tuple[str, str]: """ Extract tap name and owner from repository input. @@ -514,84 +649,21 @@ def process_input_formula(formula_file: str) -> str: tap_dirs.append(brew_tap_path) tap_dir_to_repo[brew_tap_path] = brew_tap_root - for d in tap_dirs: - print(f'Copying {formula_filename} to {d}') - os.makedirs(d, exist_ok=True) - dest_file = os.path.join(d, formula_filename) - shutil.copy2(formula_file, dest_file) - - if not os.path.exists(dest_file): - raise FileNotFoundError(f'::error:: Formula file {formula_filename} was not copied to {d}') - - # Set permissions required by Homebrew (rw-r--r--) - # Owner: read + write, Group: read, Others: read - # Homebrew requires formula files to be world-readable (brew audit enforces this) - # Only owner has write permission, complying with security best practices - # Formula files are Ruby scripts that should not be executable - os.chmod(dest_file, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH) - print(f'Copied {formula_filename} to {d}') + copy_formula_to_directories(formula_file, formula_filename, tap_dirs) end_group() # Extract version from the formula file - version = None - tag_found = False - try: - with open(formula_file, 'r') as f: - for line in f: - # Look for version line in formula (e.g., version "1.2.3") - if 'version' in line.lower() and '"' in line: - # Extract version string between quotes - start = line.find('"') - end = line.find('"', start + 1) - if start != -1 and end != -1: - version = line[start + 1:end] - break - # Fallback to tag if version not found (only process first occurrence) - elif not tag_found and version is None and 'tag:' in line.lower() and '"' in line: - # Extract tag string between quotes (e.g., tag: "v1.2.3") - start = line.find('"') - end = line.find('"', start + 1) - if start != -1 and end != -1: - version = line[start + 1:end] - tag_found = True - # Don't break here, keep looking for version in case it comes later - except Exception as e: - print(f'Could not extract version from formula: {e}') + version = extract_version_from_formula(formula_file) # Commit changes to the tap directories commit_messages = {} for formula_dir, repo_path in tap_dir_to_repo.items(): if os.path.isdir(os.path.join(repo_path, '.git')): - # Check if the formula file already existed in the git repository - # to determine if this is an add or update formula_path = os.path.join(formula_dir, formula_filename) - - # Use git ls-files to check if the file is tracked - check_tracked = subprocess.run( - ['git', 'ls-files', '--error-unmatch', formula_path], - cwd=repo_path, - capture_output=True, - ) - - # If exit code is 0, file is tracked (update), otherwise it's new (add) - is_new = check_tracked.returncode != 0 - - # Generate commit message based on whether it's new or update - if is_new: - # New formula format: "foobar 7.3 (new formula)" - if version: - commit_message = f'{formula} {version} (new formula)' - else: - commit_message = f'{formula} (new formula)' - else: - # Update format: "foobar 7.3" - if version: - commit_message = f'{formula} {version}' - else: - commit_message = f'{formula} (update)' - + is_new = not is_formula_tracked(formula_path, repo_path) + commit_message = generate_commit_message(formula, version, is_new) commit_messages[repo_path] = commit_message commit_formula_changes( diff --git a/tests/release_homebrew/test_release_homebrew.py b/tests/release_homebrew/test_release_homebrew.py index cc90230..2c74518 100644 --- a/tests/release_homebrew/test_release_homebrew.py +++ b/tests/release_homebrew/test_release_homebrew.py @@ -162,6 +162,177 @@ def test_prepare_repo_branch_custom_branch(capsys, org_homebrew_repo, monkeypatc assert f'Using custom head branch: {custom_branch_name}' in captured.out +def test_extract_version_from_formula_with_version(tmp_path): + """Test extracting version from a formula with version field.""" + formula_file = tmp_path / "test_formula.rb" + formula_file.write_text(''' +class TestFormula < Formula + desc "Test formula" + version "1.2.3" + url "https://example.com/test.tar.gz" +end +''') + + version = main.extract_version_from_formula(str(formula_file)) + assert version == "1.2.3" + + +def test_extract_version_from_formula_with_tag(tmp_path): + """Test extracting version from a formula with tag field (fallback).""" + formula_file = tmp_path / "test_formula.rb" + formula_file.write_text(''' +class TestFormula < Formula + desc "Test formula" + url "https://example.com/test.tar.gz" + tag: "v2.0.0" +end +''') + + version = main.extract_version_from_formula(str(formula_file)) + assert version == "v2.0.0" + + +def test_extract_version_from_formula_version_priority(tmp_path): + """Test that version field takes priority over tag field.""" + formula_file = tmp_path / "test_formula.rb" + formula_file.write_text(''' +class TestFormula < Formula + desc "Test formula" + url "https://example.com/test.tar.gz" + tag: "v1.0.0" + version "2.0.0" +end +''') + + version = main.extract_version_from_formula(str(formula_file)) + assert version == "2.0.0" + + +def test_extract_version_from_formula_first_tag_only(tmp_path): + """Test that only the first tag is processed.""" + formula_file = tmp_path / "test_formula.rb" + formula_file.write_text(''' +class TestFormula < Formula + desc "Test formula" + url "https://example.com/test.tar.gz" + tag: "v1.0.0" + # tag: "v2.0.0" - this should not be picked up + tag: "v3.0.0" +end +''') + + version = main.extract_version_from_formula(str(formula_file)) + assert version == "v1.0.0" + + +def test_extract_version_from_formula_no_version(tmp_path): + """Test extracting version when no version or tag field exists.""" + formula_file = tmp_path / "test_formula.rb" + formula_file.write_text(''' +class TestFormula < Formula + desc "Test formula" + url "https://example.com/test.tar.gz" +end +''') + + version = main.extract_version_from_formula(str(formula_file)) + assert version is None + + +def test_extract_version_from_formula_file_error(tmp_path, capsys): + """Test extracting version when file cannot be read.""" + formula_file = tmp_path / "nonexistent.rb" + + version = main.extract_version_from_formula(str(formula_file)) + assert version is None + + captured = capsys.readouterr() + assert 'Could not extract version from formula' in captured.out + + +@pytest.mark.parametrize('formula, version, is_new, expected', [ + ('hello_world', '1.2.3', True, 'hello_world 1.2.3 (new formula)'), + ('hello_world', None, True, 'hello_world (new formula)'), + ('hello_world', 'v2.0.0', False, 'hello_world v2.0.0'), + ('hello_world', None, False, 'hello_world'), + ('my_app', '3.1.4', True, 'my_app 3.1.4 (new formula)'), + ('my_app', '5.0.0', False, 'my_app 5.0.0'), +]) +def test_generate_commit_message(formula, version, is_new, expected): + """Test generating commit messages for various scenarios.""" + result = main.generate_commit_message(formula, version, is_new) + assert result == expected + + +@patch('subprocess.run') +def test_is_formula_tracked_true(mock_run): + """Test is_formula_tracked when file is tracked.""" + mock_run.return_value = subprocess.CompletedProcess(args=[], returncode=0) + + result = main.is_formula_tracked('/path/to/formula.rb', '/path/to/repo') + assert result is True + + mock_run.assert_called_once() + call_args = mock_run.call_args + assert 'git' in call_args[0][0] + assert 'ls-files' in call_args[0][0] + + +@patch('subprocess.run') +def test_is_formula_tracked_false(mock_run): + """Test is_formula_tracked when file is not tracked.""" + mock_run.return_value = subprocess.CompletedProcess(args=[], returncode=1) + + result = main.is_formula_tracked('/path/to/formula.rb', '/path/to/repo') + assert result is False + + +@patch('os.chmod') +@patch('os.path.exists') +@patch('shutil.copy2') +@patch('os.makedirs') +def test_copy_formula_to_directories(mock_makedirs, mock_copy, mock_exists, mock_chmod, capsys): + """Test copying formula to multiple directories.""" + mock_exists.return_value = True + + main.copy_formula_to_directories( + '/source/formula.rb', + 'formula.rb', + ['/dest1', '/dest2'] + ) + + # Verify makedirs called for each directory + assert mock_makedirs.call_count == 2 + + # Verify copy2 called for each directory + assert mock_copy.call_count == 2 + + # Verify chmod called for each directory + assert mock_chmod.call_count == 2 + + # Verify output messages + captured = capsys.readouterr() + assert 'Copying formula.rb to /dest1' in captured.out + assert 'Copying formula.rb to /dest2' in captured.out + assert 'Copied formula.rb to /dest1' in captured.out + assert 'Copied formula.rb to /dest2' in captured.out + + +@patch('os.path.exists') +@patch('shutil.copy2') +@patch('os.makedirs') +def test_copy_formula_to_directories_copy_failure(mock_makedirs, mock_copy, mock_exists): + """Test copy_formula_to_directories when copy verification fails.""" + mock_exists.return_value = False + + with pytest.raises(FileNotFoundError, match='was not copied'): + main.copy_formula_to_directories( + '/source/formula.rb', + 'formula.rb', + ['/dest1'] + ) + + def test_process_input_formula(operating_system, org_homebrew_repo): with pytest.raises(FileNotFoundError): main.process_input_formula(formula_file='foo') From 077635a1dba8a9165b67db0b0164a7c338ef2354 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 17 Dec 2025 23:35:12 -0500 Subject: [PATCH 6/9] Fix formula version extraction --- actions/release_homebrew/main.py | 6 ++- .../release_homebrew/test_release_homebrew.py | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index cdd45e5..d59af2e 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -381,8 +381,10 @@ def extract_version_from_formula(formula_file: str) -> Optional[str]: try: with open(formula_file, 'r') as f: for line in f: + stripped = line.strip() # Look for version line in formula (e.g., version "1.2.3") - if 'version' in line.lower() and '"' in line: + # Must start with 'version' (not just contain it) to avoid matching variables like GCC_VERSION + if stripped.startswith('version') and '"' in line: # Extract version string between quotes start = line.find('"') end = line.find('"', start + 1) @@ -390,7 +392,7 @@ def extract_version_from_formula(formula_file: str) -> Optional[str]: version = line[start + 1:end] break # Fallback to tag if version not found (only process first occurrence) - elif not tag_found and version is None and 'tag:' in line.lower() and '"' in line: + elif not tag_found and version is None and stripped.startswith('tag:') and '"' in line: # Extract tag string between quotes (e.g., tag: "v1.2.3") start = line.find('"') end = line.find('"', start + 1) diff --git a/tests/release_homebrew/test_release_homebrew.py b/tests/release_homebrew/test_release_homebrew.py index 2c74518..d412661 100644 --- a/tests/release_homebrew/test_release_homebrew.py +++ b/tests/release_homebrew/test_release_homebrew.py @@ -250,6 +250,44 @@ def test_extract_version_from_formula_file_error(tmp_path, capsys): assert 'Could not extract version from formula' in captured.out +def test_extract_version_from_formula_with_version_variable(tmp_path): + """Test that variables containing 'version' in their name don't get matched.""" + formula_file = tmp_path / "test_formula.rb" + formula_file.write_text(''' +class TestFormula < Formula + GCC_VERSION = "14".freeze + GCC_FORMULA = "gcc@#{GCC_VERSION}".freeze + + desc "Test formula" + homepage "https://example.com" + url "https://example.com/test.tar.gz", + tag: "v1.0.0" + version "3.2.1" +end +''') + + version = main.extract_version_from_formula(str(formula_file)) + assert version == "3.2.1" + + +def test_extract_version_from_formula_with_version_variable_no_version_field(tmp_path): + """Test that tag is used when only variables with 'version' exist.""" + formula_file = tmp_path / "test_formula.rb" + formula_file.write_text(''' +class TestFormula < Formula + GCC_VERSION = "14".freeze + OTHER_VERSION = "2.0".freeze + + desc "Test formula" + url "https://example.com/test.tar.gz", + tag: "v5.6.7" +end +''') + + version = main.extract_version_from_formula(str(formula_file)) + assert version == "v5.6.7" + + @pytest.mark.parametrize('formula, version, is_new, expected', [ ('hello_world', '1.2.3', True, 'hello_world 1.2.3 (new formula)'), ('hello_world', None, True, 'hello_world (new formula)'), From 465ea6406763d2421e7bd6091715a9bea76a962a Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 18 Dec 2025 19:29:16 -0500 Subject: [PATCH 7/9] Add skip_stable_version_audit option to Homebrew release action Introduces a new input, skip_stable_version_audit, to the release_homebrew action to optionally skip the stable version audit in brew test-bot runs. Updates the README, action.yml, and main.py to support this feature, and sets the default to true for easier PR testing. Minor whitespace fixes in test_release_homebrew.py. --- actions/release_homebrew/README.md | 1 + actions/release_homebrew/action.yml | 4 +++ actions/release_homebrew/main.py | 28 ++++++++++++------- .../release_homebrew/test_release_homebrew.py | 4 +-- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/actions/release_homebrew/README.md b/actions/release_homebrew/README.md index f3a8d88..2d000c7 100644 --- a/actions/release_homebrew/README.md +++ b/actions/release_homebrew/README.md @@ -45,6 +45,7 @@ steps: | org_homebrew_repo_base_branch | The base branch of the target repository to create PR against. | | `false` | | org_homebrew_repo_head_branch | The head branch of the target repository to create for the PR. If empty, auto-generated. | | `false` | | publish | Whether to publish the release. | `false` | `false` | +| skip_stable_version_audit | Whether to skip stable version audit for brew test-bot (useful for PR testing). | `true` | `false` | | token | GitHub Token. This is required when `publish` is enabled. | | `false` | | upstream_homebrew_core_repo | The upstream homebrew-core repository that the fork is based on. Must be a GitHub repo. | `Homebrew/homebrew-core` | `false` | | validate | Whether to validate the formula. | `true` | `false` | diff --git a/actions/release_homebrew/action.yml b/actions/release_homebrew/action.yml index 633ccd1..e54f976 100644 --- a/actions/release_homebrew/action.yml +++ b/actions/release_homebrew/action.yml @@ -56,6 +56,9 @@ inputs: description: 'Whether to publish the release.' default: 'false' required: false + skip_stable_version_audit: + description: 'Whether to skip stable version audit for brew test-bot' + default: 'true' token: description: 'Github Token. This is required when `publish` is enabled.' required: false @@ -158,6 +161,7 @@ runs: INPUT_ORG_HOMEBREW_REPO: ${{ inputs.org_homebrew_repo }} INPUT_ORG_HOMEBREW_REPO_HEAD_BRANCH: ${{ inputs.org_homebrew_repo_head_branch }} INPUT_HOMEBREW_CORE_HEAD_BRANCH: ${{ inputs.homebrew_core_head_branch }} + INPUT_SKIP_STABLE_VERSION_AUDIT: ${{ inputs.skip_stable_version_audit }} INPUT_UPSTREAM_HOMEBREW_CORE_REPO: ${{ inputs.upstream_homebrew_core_repo }} INPUT_VALIDATE: ${{ inputs.validate }} id: homebrew-tests diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index d59af2e..b65aeb5 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -796,16 +796,24 @@ def brew_test_bot_only_formulae(formula: str) -> bool: org_repo = os.environ['INPUT_ORG_HOMEBREW_REPO'] root_url = f'https://ghcr.io/v2/{org_repo.rsplit("-", 1)[0].lower()}' - result = _run_subprocess( - args_list=[ - 'brew', - 'test-bot', - '--only-formulae', - f'--tap={tap_repo_name}', - f'--testing-formulae={tap_repo_name}/{formula}', - f'--root-url={root_url}', - ], - ) + + # Check if we should skip stable version audit (default: true, meaning skip it) + skip_stable_version_audit = os.getenv('INPUT_SKIP_STABLE_VERSION_AUDIT', 'true').lower() == 'true' + stable_version_audit_arg = '--skip-stable-version-audit' if skip_stable_version_audit else '' + # Build args list, filtering out empty strings + args_list = [ + 'brew', + 'test-bot', + '--only-formulae', + f'--tap={tap_repo_name}', + f'--testing-formulae={tap_repo_name}/{formula}', + f'--root-url={root_url}', + ] + + if stable_version_audit_arg: + args_list.append(stable_version_audit_arg) + + result = _run_subprocess(args_list=args_list) end_group() return result diff --git a/tests/release_homebrew/test_release_homebrew.py b/tests/release_homebrew/test_release_homebrew.py index d412661..162825c 100644 --- a/tests/release_homebrew/test_release_homebrew.py +++ b/tests/release_homebrew/test_release_homebrew.py @@ -257,7 +257,7 @@ def test_extract_version_from_formula_with_version_variable(tmp_path): class TestFormula < Formula GCC_VERSION = "14".freeze GCC_FORMULA = "gcc@#{GCC_VERSION}".freeze - + desc "Test formula" homepage "https://example.com" url "https://example.com/test.tar.gz", @@ -277,7 +277,7 @@ def test_extract_version_from_formula_with_version_variable_no_version_field(tmp class TestFormula < Formula GCC_VERSION = "14".freeze OTHER_VERSION = "2.0".freeze - + desc "Test formula" url "https://example.com/test.tar.gz", tag: "v5.6.7" From 67c20853759b9ddfa56e57c64b1fa5fe9ceee53f Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 18 Dec 2025 21:55:19 -0500 Subject: [PATCH 8/9] Add Homebrew setup action to workflow Introduces the Homebrew/actions/setup-homebrew action for consistent Homebrew environment setup. Removes manual PATH configuration and streamlines the Homebrew test step. --- actions/release_homebrew/action.yml | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/actions/release_homebrew/action.yml b/actions/release_homebrew/action.yml index e54f976..e089a7b 100644 --- a/actions/release_homebrew/action.yml +++ b/actions/release_homebrew/action.yml @@ -152,6 +152,12 @@ runs: persist-credentials: false # otherwise, the token used is the GITHUB_TOKEN, instead of the personal token fetch-depth: 1 + - name: Set up Homebrew + id: set-up-homebrew + uses: Homebrew/actions/setup-homebrew@main + with: + stable: true + - name: Homebrew tests env: INPUT_FORMULA_FILE: ${{ inputs.formula_file }} @@ -168,17 +174,7 @@ runs: shell: bash working-directory: ${{ github.action_path }} run: | - echo "::group::Setup Homebrew PATH" - if [[ "${{ runner.os }}" == "Linux" ]]; then - # https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#homebrew-note - echo "Adding Homebrew to PATH" - eval "$(/home/linuxbrew/.linuxbrew/bin/brew shellenv)" - fi - echo "::endgroup::" - - echo "::group::Homebrew tests" "${{ steps.venv.outputs.python-path }}" -u main.py - echo "::endgroup::" - name: GitHub Commit & Push (org homebrew repo) if: ${{ inputs.publish == 'true' }} From c9c8e32d0ff4071a1fba7d64b346ddf5241813f3 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 18 Dec 2025 22:24:37 -0500 Subject: [PATCH 9/9] Set HOMEBREW_BOTTLE_BUILD env for test bot Adds the HOMEBREW_BOTTLE_BUILD environment variable when running the Homebrew test bot to allow skipping advanced tests during bottle builds. This change ensures the subprocess inherits the current environment and sets the required variable. --- actions/release_homebrew/main.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/actions/release_homebrew/main.py b/actions/release_homebrew/main.py index b65aeb5..ab7f24c 100644 --- a/actions/release_homebrew/main.py +++ b/actions/release_homebrew/main.py @@ -813,7 +813,18 @@ def brew_test_bot_only_formulae(formula: str) -> bool: if stable_version_audit_arg: args_list.append(stable_version_audit_arg) - result = _run_subprocess(args_list=args_list) + # setting this will allow us to skip advanced tests when building bottles + env = { + 'HOMEBREW_BOTTLE_BUILD': 'true', + } + + # combine with os environment + env.update(os.environ) + + result = _run_subprocess( + args_list=args_list, + env=env, + ) end_group() return result