diff --git a/.github/scripts/create-test-pr.sh b/.github/scripts/create-test-pr.sh new file mode 100755 index 0000000..2472b22 --- /dev/null +++ b/.github/scripts/create-test-pr.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash + +# Create a test PR for validating a contributor's PR +# Usage: ./create-test-pr.sh + +set -e + +PR_NUMBER="${1:?Usage: $0 }" + +echo "Creating test PR for #$PR_NUMBER..." + +# Fetch the contributor's PR +echo "Fetching PR #$PR_NUMBER..." +git fetch origin pull/"$PR_NUMBER"/head:test-pr-"$PR_NUMBER"-tmp + +# Create test branch +git checkout -b test-pr-"$PR_NUMBER" test-pr-"$PR_NUMBER"-tmp + +# Clean up temp branch +git branch -D test-pr-"$PR_NUMBER"-tmp + +echo "" +echo "✓ Checked out contributor's code in branch: test-pr-$PR_NUMBER" +echo "" +echo "Next steps:" +echo " 1. Write your tests" +echo " 2. Commit them: git commit -am 'Add tests for PR #$PR_NUMBER'" +echo " 3. Run: ./.github/scripts/push-test-pr.sh $PR_NUMBER" diff --git a/.github/scripts/push-test-pr.sh b/.github/scripts/push-test-pr.sh new file mode 100755 index 0000000..a56aa1f --- /dev/null +++ b/.github/scripts/push-test-pr.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +# Push test PR and create it on GitHub +# Usage: ./push-test-pr.sh + +set -e + +PR_NUMBER="${1:?Usage: $0 }" +BRANCH="test-pr-$PR_NUMBER" + +# Check we're on the right branch +CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD) +if [ "$CURRENT_BRANCH" != "$BRANCH" ]; then + echo "Error: Not on branch $BRANCH (currently on $CURRENT_BRANCH)" + exit 1 +fi + +# Push the branch +echo "Pushing branch $BRANCH..." +git push origin "$BRANCH" + +if ! command -v gh &> /dev/null; then + echo "GitHub CLI not found, skipping PR creation." + exit 1 +fi + +echo "Creating draft PR..." + +gh pr create --draft --title "Test PR #$PR_NUMBER" --label "tests" --body "*Tests for #$PR_NUMBER +* Must be merged AFTER #$PR_NUMBER + +See CONTRIBUTING.md for details and for how to update this PR if the base PR changes. +" + +echo "" +echo "✓ Draft PR created" diff --git a/.github/workflows/test-integration.yml b/.github/workflows/test-integration.yml index 4495b22..3a1b1af 100644 --- a/.github/workflows/test-integration.yml +++ b/.github/workflows/test-integration.yml @@ -254,6 +254,85 @@ jobs: source test/lib/cleanup-test-deployment.sh cleanup_deployment "${{ env.TEST_DEPLOY_REPO }}" "${{ env.TEST_ID }}" "${{ secrets.test-deploy-token }}" "${{ env.TEST_UMBRELLA_DIR }}" + test-comment-pr-number-override: + name: "[Comment] PR number override" + runs-on: ubuntu-latest + env: + TEST_ID: test-comment-pr-number-override + TEST_UMBRELLA_DIR: pr-preview-run-${{ github.run_id }} + TEST_DEPLOY_REPO: rozzjrw/pr-preview-action-testing + # This is a persistent dummy PR in the test repo that receives test comments + DUMMY_PR_NUMBER: 1 + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + ref: ${{ inputs.sha }} + + - name: Modify fixture with unique identifier + run: sed -i.bak 's/

.*<\/p>/

run-${{ github.run_id }}-deployed<\/p>/' test/fixtures/html/index.html + + - name: Deploy with pr-number override to dummy PR + uses: ./ + with: + pr-number: ${{ env.DUMMY_PR_NUMBER }} + umbrella-dir: ${{ env.TEST_UMBRELLA_DIR }} + token: ${{ secrets.test-deploy-token }} + deploy-repository: ${{ env.TEST_DEPLOY_REPO }} + preview-branch: ${{ env.TEST_ID }} + source-dir: test/fixtures/html + action: deploy + comment: true + wait-for-pages-deployment: false + git-config-name: GitHub Actions + git-config-email: actions@github.com + deploy-commit-message: deploy ${{ env.TEST_ID }} ${{ github.run_id }} + remove-commit-message: remove ${{ env.TEST_ID }} ${{ github.run_id }} + + - name: Verify comment + env: + GITHUB_TOKEN: ${{ secrets.test-deploy-token }} + DEPLOY_REPO: ${{ env.TEST_DEPLOY_REPO }} + TEST_RUN_ID: ${{ github.run_id }} + run: bash test/integration/test-comment-pr-number-override.sh "verify-deploy" "${{ env.DUMMY_PR_NUMBER }}" + + - name: Remove deployment + uses: ./ + with: + pr-number: ${{ env.DUMMY_PR_NUMBER }} + umbrella-dir: ${{ env.TEST_UMBRELLA_DIR }} + token: ${{ secrets.test-deploy-token }} + deploy-repository: ${{ env.TEST_DEPLOY_REPO }} + preview-branch: ${{ env.TEST_ID }} + action: remove + comment: true + wait-for-pages-deployment: false + git-config-name: GitHub Actions + git-config-email: actions@github.com + deploy-commit-message: deploy ${{ env.TEST_ID }} ${{ github.run_id }} + remove-commit-message: remove ${{ env.TEST_ID }} ${{ github.run_id }} + + - name: Verify comment + env: + GITHUB_TOKEN: ${{ secrets.test-deploy-token }} + DEPLOY_REPO: ${{ env.TEST_DEPLOY_REPO }} + TEST_RUN_ID: ${{ github.run_id }} + run: bash test/integration/test-comment-pr-number-override.sh "verify-remove" "${{ env.DUMMY_PR_NUMBER }}" + + - name: Cleanup comments + if: always() + env: + GITHUB_TOKEN: ${{ secrets.test-deploy-token }} + DEPLOY_REPO: ${{ env.TEST_DEPLOY_REPO }} + TEST_RUN_ID: ${{ github.run_id }} + run: bash test/integration/test-comment-pr-number-override.sh "cleanup" "${{ env.DUMMY_PR_NUMBER }}" + + - name: Cleanup deployment + if: always() + run: | + source test/lib/cleanup-test-deployment.sh + cleanup_deployment "${{ env.TEST_DEPLOY_REPO }}" "${{ env.TEST_ID }}" "${{ secrets.test-deploy-token }}" "${{ env.TEST_UMBRELLA_DIR }}" + # Pages deployment test runs after git tests pass # This is the only test that uses the actual Pages branch and waits for deployment test-pages-lifecycle: diff --git a/.gitignore b/.gitignore index 504afef..972687d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ node_modules/ package-lock.json +comment-generated.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..9057617 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,67 @@ +# Contributing + +Contributions are welcome! + +## Testing Contributor PRs + +Procedure for testing forked PRs from external contributors: + +When an external contributor (someone who does not have push access to this repo) submits a PR: + +1. New features and fixes may need integration tests - the contributor could write them, but the maintainer could too +2. Integration tests must be added to `.github/workflows/test-integration.yml` so they can run targeting the test repository +3. Workflow runs that originate from a forked PR run on `pull_request_target`, which use workflow files from the base branch - which, for security, does not include the new integration tests :( + +The following procedure enables contributor PRs to be tested while minimising risk of security incidents and making sure both the contribution and the tests can be reviewed transparently. + +The onus is on the maintainer to make sure it happens. Contributors are welcome to write integration tests but are not expected to be able to run them. + +None of this applies to unit tests, which don't need access to anything and always run. + +### 1. Contributor submits PR + +A contributor opens PR e.g. #123 with their changes, and maybe some new tests too if they're cool. + +### 2. Maintainer creates test branch + +From a clean state (no unstaged/uncommited changes): + +```bash +./.github/scripts/create-test-pr.sh 123 +``` + +This: + +1. Fetches the contributor's PR +2. Creates a local branch `test-pr-123` from the contributor's forked branch +3. Checks out that branch + +If needed, the maintainer then adds any extra integration tests that weren't already in the contribution. + +### 4. Maintainer pushes and creates test PR + +```bash +./.github/scripts/push-test-pr.sh 123 +``` + +This pushes the `test-pr-123` branch and creates a draft PR. Tests that run on this PR run within the repository and therefore are able to run any new integration tests. + +If the contributor updates their PR, rebase and push the test branch: + +```shell +git checkout test-pr-123 +git fetch origin pull/123/head:test-pr-123-tmp +git rebase test-pr-123-tmp +# <- Update tests if needed +git push origin test-pr-123 --force-with-lease --force-if-includes +``` + +### 6. Maintainer merges contribution PR + +If tests passes, the feature is desirable, etc, the maintainer should merge the contributor's PR. Unmark the test PR as draft to indicate that it can be merged. + +Do not squash merge because that fucks up the commit history. + +### 7. Maintainer merges test PR + +The maintainer should ensure that the commit history of the test PR is correct (it should now ONLY contain commits that added integration tests, if any) and merge it if appropriate. diff --git a/action.yml b/action.yml index 5e6751d..b945ac7 100644 --- a/action.yml +++ b/action.yml @@ -219,6 +219,7 @@ runs: uses: marocchino/sticky-pull-request-comment@67d0dec7b07ed060a405f9b2a64b8ab319fdd7db # v2.9.2 with: header: pr-preview + number: ${{ env.pr_number || github.event.pull_request.number }} message: ${{ steps.deploy-comment.outputs.content }} # REMOVAL @@ -290,4 +291,5 @@ runs: uses: marocchino/sticky-pull-request-comment@67d0dec7b07ed060a405f9b2a64b8ab319fdd7db # v2.9.2 with: header: pr-preview + number: ${{ env.pr_number || github.event.pull_request.number }} message: ${{ steps.remove-comment.outputs.content }} diff --git a/test/integration/test-comment-pr-number-override.sh b/test/integration/test-comment-pr-number-override.sh new file mode 100755 index 0000000..54b376f --- /dev/null +++ b/test/integration/test-comment-pr-number-override.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash + +set -e + +source "$(dirname "$0")/../lib/assert.sh" + +test_step="${1:?missing test_step}" +dummy_pr_number="${2:?missing dummy_pr_number}" + +DEPLOY_REPO="${DEPLOY_REPO:?env var DEPLOY_REPO is required}" +GITHUB_TOKEN="${GITHUB_TOKEN:?env var GITHUB_TOKEN is required}" +TEST_RUN_ID="${TEST_RUN_ID:?missing env var TEST_RUN_ID}" + +COMMENT_HEADER="pr-preview" + +verify_comment_exists() { + local pr_number="$1" + local expected_pattern="$2" + + echo "Fetching comments for PR #$pr_number in $DEPLOY_REPO..." + + local comments_response + comments_response=$(curl -s -H "Authorization: token $GITHUB_TOKEN" \ + -H "Accept: application/vnd.github.v3+json" \ + "https://api.github.com/repos/$DEPLOY_REPO/issues/$pr_number/comments") + + if echo "$comments_response" | grep -q '"message"'; then + echo >&2 "FAIL: API error: $(echo "$comments_response" | grep -o '"message": "[^"]*"')" + return 1 + fi + + local matching_comment + matching_comment=$(echo "$comments_response" | grep -o ".*" | head -1) + + if [ -z "$matching_comment" ]; then + echo >&2 "FAIL: No comment with header '$COMMENT_HEADER' found on PR #$pr_number" + echo >&2 "Available comments:" + echo "$comments_response" | grep -o '"body": "[^"]*"' | head -5 + return 1 + fi + + echo "✓ Found comment with header '$COMMENT_HEADER'" + + if [ -n "$expected_pattern" ]; then + if echo "$comments_response" | grep -q "$expected_pattern"; then + echo "✓ Comment contains expected pattern: $expected_pattern" + else + echo >&2 "FAIL: Comment does not contain expected pattern: $expected_pattern" + return 1 + fi + fi + + local comment_id_value + comment_id_value=$(echo "$comments_response" | grep -B 20 "" | grep -o '"id": [0-9]*' | head -1 | cut -d' ' -f2) + export COMMENT_ID="$comment_id_value" + echo "Comment ID: $COMMENT_ID" +} + +delete_comment() { + local pr_number="$1" + local comment_id="${2:-$COMMENT_ID}" + + if [ -z "$comment_id" ]; then + echo "No comment ID provided, skipping deletion" + return 0 + fi + + echo "Deleting comment $comment_id from PR #$pr_number..." + + local delete_response + delete_response=$(curl -s -w "\n%{http_code}" -X DELETE \ + -H "Authorization: token $GITHUB_TOKEN" \ + -H "Accept: application/vnd.github.v3+json" \ + "https://api.github.com/repos/$DEPLOY_REPO/issues/comments/$comment_id") + + local http_code + http_code=$(echo "$delete_response" | tail -n 1) + + if [ "$http_code" = "204" ]; then + echo "✓ Comment deleted successfully" + else + echo >&2 "WARNING: Failed to delete comment (HTTP $http_code)" + echo >&2 "$delete_response" + # Don't fail the test if cleanup fails + fi +} + +case "$test_step" in + verify-deploy) + verify_comment_exists "$dummy_pr_number" "run-$TEST_RUN_ID-deployed" + echo "✓ Deploy comment verified on PR #$dummy_pr_number (pr-number override)" + ;; + + verify-remove) + verify_comment_exists "$dummy_pr_number" "Preview removed" + echo "✓ Remove comment verified on PR #$dummy_pr_number (pr-number override)" + ;; + + cleanup) + delete_comment "$dummy_pr_number" + ;; + + *) + echo >&2 "ERROR: Invalid test_step: $test_step" + echo >&2 "Valid steps: verify-deploy, verify-remove, cleanup" + exit 1 + ;; +esac diff --git a/test/unit/test-00-scripts-executable.sh b/test/unit/test-00-scripts-executable.sh new file mode 100755 index 0000000..9ab2888 --- /dev/null +++ b/test/unit/test-00-scripts-executable.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash + +set -e + +source "$(dirname "$0")/../lib/assert.sh" + +# This test validates that all test scripts are executable +# This ensures that test scripts can be run directly and helps catch permission issues early + +test_root="$(dirname "$0")/.." +failed_scripts=() +checked_count=0 + +echo "Checking test script permissions..." + +# Check all shell scripts in test directories +for script in \ + "$test_root"/unit/test-*.sh \ + "$test_root"/integration/test-*.sh \ + "$test_root"/lib/*.sh; do + + if [ -f "$script" ]; then + checked_count=$((checked_count + 1)) + script_name=$(basename "$script") + + if [ ! -x "$script" ]; then + echo "✗ $script_name is not executable" + failed_scripts+=("$script") + fi + fi +done + +echo "Checked $checked_count test scripts" + +if [ ${#failed_scripts[@]} -gt 0 ]; then + echo "" + echo "The following scripts are not executable:" + for script in "${failed_scripts[@]}"; do + echo " - $script" + done + echo "" + echo "Fix with: chmod +x