Merge pull request #227 from 1Password/vzt/fix-ok-to-test

Fork PR flow fixes

.github/workflows/e2e-tests.yml

@@ -1,4 +1,4 @@
-name: E2E Tests
+name: E2E Tests [reusable]
 
 on:
   workflow_call:
@@ -14,7 +14,7 @@ on:
         required: true
 
 jobs:
-  e2e-test:
+  run:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout code

.github/workflows/ok-to-test.yml

@@ -1,4 +1,4 @@
-# Write comments "/ok-to-test <hash>" on a pull request. This will emit a repository_dispatch event.
+# Write comments "/ok-to-test sha=<hash>" on a pull request. This will emit a repository_dispatch event.
 name: Ok To Test
 
 on:

.github/workflows/test-e2e-fork.yml

@@ -1,56 +0,0 @@
-name: E2E tests [fork]
-
-on:
-  repository_dispatch:
-    types: [ ok-to-test-command ]
-
-permissions:
-  contents: read
-  checks: write
-
-concurrency:
-  group: e2e-fork-${{ github.event.client_payload.pull_request.number || github.run_id }}
-  cancel-in-progress: true # cancel previous job runs for the same branch
-
-jobs:
-  e2e-tests:
-    uses: ./.github/workflows/e2e-tests.yml
-    if: |
-      github.event_name == 'repository_dispatch' &&
-      github.event.client_payload.slash_command.args.named.sha != '' &&
-      contains(
-        github.event.client_payload.pull_request.head.sha,
-        github.event.client_payload.slash_command.args.named.sha
-      )
-    secrets:
-      OP_CONNECT_CREDENTIALS: ${{ secrets.OP_CONNECT_CREDENTIALS }}
-      OP_CONNECT_TOKEN: ${{ secrets.OP_CONNECT_TOKEN }}
-      OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
-
-  update-check-status:
-    needs: e2e-tests
-    runs-on: ubuntu-latest
-    if: always() && github.event_name == 'repository_dispatch'
-    steps:
-      - uses: actions/github-script@v6
-        env:
-          ref: ${{ github.event.client_payload.pull_request.head.sha }}
-          conclusion: ${{ needs.e2e-tests.result }}
-        with:
-          github-token: ${{ secrets.GITHUB_TOKEN }}
-          script: |
-            const { data: checks } = await github.rest.checks.listForRef({
-              ...context.repo,
-              ref: process.env.ref
-            });
-
-            const check = checks.check_runs.filter(c => c.name === 'e2e-test');
-
-            const { data: result } = await github.rest.checks.update({
-              ...context.repo,
-              check_run_id: check[0].id,
-              status: 'completed',
-              conclusion: process.env.conclusion
-            });
-
-            return result;

.github/workflows/test-e2e.yml

@@ -14,30 +14,97 @@ on:
   push:
     branches: [main]
     paths-ignore: *ignore_paths
+  repository_dispatch:
+    types: [ ok-to-test-command ]
 
 concurrency:
-  group: e2e-${{ github.event.pull_request.head.ref }}
+  group: >-
+    ${{ github.event_name == 'pull_request' && 
+          format('e2e-{0}', github.event.pull_request.head.ref) || 
+          format('e2e-{0}', github.event.client_payload.pull_request.head.ref) }}
   cancel-in-progress: true  # cancel previous job runs for the same branch
 
 jobs:
   check-external-pr:
     runs-on: ubuntu-latest
-    if: github.event_name == 'pull_request'
+    outputs:
+      condition: ${{ steps.check.outputs.condition }}
     steps:
       - name: Check if PR is from external contributor
+        id: check
         run: |
-          if [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]; then
-            echo "❌ External PR detected. This workflow requires approval from a maintainer."
-            echo "Please ask a maintainer to run '/ok-to-test' command to trigger the fork workflow."
-            exit 1
-          fi
-          echo "✅ Internal PR detected. Proceeding with tests."
+          echo "Event name: ${{ github.event_name }}"
+          echo "Repository: ${{ github.repository }}"
           
-  e2e-test:
+          if [ "${{ github.event_name }}" == "pull_request" ]; then
+            # For pull_request events, check if PR is from external fork
+            echo "PR head repo: ${{ github.event.pull_request.head.repo.full_name }}"
+            if [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]; then
+              echo "condition=skip" >> $GITHUB_OUTPUT
+              echo "Setting condition=skip (external fork PR creation)"
+            else
+              echo "condition=pr-creation-maintainer" >> $GITHUB_OUTPUT
+              echo "Setting condition=pr-creation-maintainer (internal PR creation)"
+            fi
+          elif [ "${{ github.event_name }}" == "repository_dispatch" ]; then
+            # For repository_dispatch events (ok-to-test), check if sha matches
+            SHA_PARAM="${{ github.event.client_payload.slash_command.args.named.sha }}"
+            PR_HEAD_SHA="${{ github.event.client_payload.pull_request.head.sha }}"
+            
+            echo "Checking dispatch event conditions..."
+            echo "SHA from command: $SHA_PARAM"
+            echo "PR head SHA: $PR_HEAD_SHA"
+            
+            if [ -n "$SHA_PARAM" ] && [[ "$PR_HEAD_SHA" == *"$SHA_PARAM"* ]]; then
+              echo "condition=dispatch-event" >> $GITHUB_OUTPUT
+              echo "Setting condition=dispatch-event (sha matches)"
+            else
+              echo "condition=skip" >> $GITHUB_OUTPUT
+              echo "Setting condition=skip (sha does not match or empty)"
+            fi
+          else
+            # Unknown event type
+            echo "condition=skip" >> $GITHUB_OUTPUT
+            echo "Setting condition=skip (unknown event type: ${{ github.event_name }})"
+          fi
+
+  # Run tests for:
+  # 1. Internal PRs on pull_request events
+  # 2. External PRs on repository_dispatch (ok-to-test) events
+  e2e:
     needs: check-external-pr
-    if: always() && (needs.check-external-pr.result == 'success' || github.event_name != 'pull_request')
+    if: |
+        (needs.check-external-pr.outputs.condition == 'pr-creation-maintainer')
+        ||
+        (needs.check-external-pr.outputs.condition == 'dispatch-event')
     uses: ./.github/workflows/e2e-tests.yml
     secrets:
       OP_CONNECT_CREDENTIALS: ${{ secrets.OP_CONNECT_CREDENTIALS }}
       OP_CONNECT_TOKEN: ${{ secrets.OP_CONNECT_TOKEN }}
       OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
+  
+  comment-pr:
+    needs: [check-external-pr, e2e]
+    runs-on: ubuntu-latest
+    if: always() && needs.check-external-pr.outputs.condition == 'dispatch-event'
+    permissions:
+      pull-requests: write
+    steps:
+      - name: Create URL to the run output
+        id: vars
+        run: echo "run-url=https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" >> $GITHUB_OUTPUT
+      
+      - name: Create comment on PR
+        uses: peter-evans/create-or-update-comment@v5
+        with:
+          issue-number: ${{ github.event.client_payload.pull_request.number }}
+          body: |
+            ${{ 
+                needs.e2e-test.result == 'success' && '✅ E2E tests passed.' || 
+                needs.e2e-test.result == 'failure' && '❌ E2E tests failed.' || 
+                '⚠️ E2E tests completed.'
+            }}
+
+            [View test run output][1]
+
+            [1]: ${{ steps.vars.outputs.run-url }}

docs/fork-pr-testing.md

@@ -0,0 +1,32 @@
+# Fork PR Testing Guide
+
+This document explains how testing works for external pull requests from forks.
+
+## Overview
+
+The testing system consists of two main workflows:
+
+1. **E2E Tests** (`test-e2e.yml`) - Runs automatically for internal PRs, need manual trigger on external PRs.
+2. **Ok To Test** (`ok-to-test.yml`) - Dispatches `repository_dispatch` event when maintainer puts the `/ok-to-test sha=<commit hash>` comment in the forked PR thread.
+
+## How It Works
+
+### 1. PR is created by maintainer:
+
+For the PR created by maintainer `E2E Test` workflow starts automatically. The PR check will reflect the status of the job.
+
+### 2. PR is created by external contributor:
+
+For the PR created by external contributor `E2E Test` workflow **won't** start automatically.
+Maintainer should make a sanity check of the changes and run it manually by:
+1. Putting a comment `/ok-to-test sha=<latest commit hash>` in the PR thread.
+2. `E2E Test` workflow starts.
+3. After `E2E Test` workflow finishes, a comment with a link to the workflow, along with its status will be posted in the PR.
+4. Maintainer can merge PR or request the changes based on the `E2E Test` results.
+
+
+## Notes
+
+- Only users with **write** permissions can trigger the `/ok-to-test` command.
+- External PRs are automatically detected and prevented from running e2e tests automatically.
+- Running e2e test on the external PR is optional. Maintainer can merge PR without running it. Maintainer decides whether it's needed to run an E2E test.