From 9cb42e35cca3b09ac043d0d139db50930f8117a7 Mon Sep 17 00:00:00 2001 From: mweberxyz <1062734+mweberxyz@users.noreply.github.com> Date: Sat, 24 Feb 2024 21:29:59 -0500 Subject: [PATCH] fix(plugins-benchmark-pr): run comparison benchmark against target (#120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(plugins-benchmark-pr): ensure comparison benchmarks run against PR base Closes fastify/workflows#93 Previously, the second benchmark ran against the default branch of the (potentially forked) repo, which might not be in sync with the PR target repo. * feature(plugins-benchmark-pr): factor up repos and refs and include in output * refactor(plugins-benchmark-pr): inputs to dash case For consistency * refactor(plugins-benchmark-pr): consistency input and output naming, refs in step names * refactor(plugins-benchmark-pr): consistency in step id * docs(plugins-benchmark-pr): label should be removed from base In my fork, the benchmark label was not removed with a 403 after the benchmark run, because it was trying to remove the label from PR in fastify repo — not the PR in forked repo. --- .github/workflows/plugins-benchmark-pr.yml | 73 +++++++++++++++------- README.md | 2 +- 2 files changed, 50 insertions(+), 25 deletions(-) diff --git a/.github/workflows/plugins-benchmark-pr.yml b/.github/workflows/plugins-benchmark-pr.yml index 38c8554..cdaddb9 100644 --- a/.github/workflows/plugins-benchmark-pr.yml +++ b/.github/workflows/plugins-benchmark-pr.yml @@ -7,7 +7,30 @@ on: type: string default: benchmark required: false - + pr-repo: + type: string + default: ${{ github.event.pull_request.head.repo.full_name }} + required: false + pr-sha: + type: string + default: ${{ github.event.pull_request.head.sha }} + required: false + pr-ref: + type: string + default: ${{ github.event.pull_request.head.ref }} + required: false + base-repo: + type: string + default: ${{ github.event.pull_request.base.repo.full_name }} + required: false + base-sha: + type: string + default: ${{ github.event.pull_request.base.sha }} + required: false + base-ref: + type: string + default: ${{ github.event.pull_request.base.ref }} + required: false jobs: benchmark: if: ${{ github.event.label.name == 'benchmark' }} @@ -18,47 +41,49 @@ jobs: PR-BENCH-18: ${{ steps.benchmark-pr.outputs.BENCH_RESULT_18 }} PR-BENCH-20: ${{ steps.benchmark-pr.outputs.BENCH_RESULT_20 }} PR-BENCH-21: ${{ steps.benchmark-pr.outputs.BENCH_RESULT_21 }} - DEFAULT-BENCH-18: ${{ steps.benchmark-default.outputs.BENCH_RESULT_18 }} - DEFAULT-BENCH-20: ${{ steps.benchmark-default.outputs.BENCH_RESULT_20 }} - DEFAULT-BENCH-21: ${{ steps.benchmark-default.outputs.BENCH_RESULT_21 }} + BASE-BENCH-18: ${{ steps.benchmark-base.outputs.BENCH_RESULT_18 }} + BASE-BENCH-20: ${{ steps.benchmark-base.outputs.BENCH_RESULT_20 }} + BASE-BENCH-21: ${{ steps.benchmark-base.outputs.BENCH_RESULT_21 }} strategy: matrix: node-version: [18, 20, 21] steps: - - uses: actions/checkout@v4 + - name: Checkout ${{ inputs.pr-repo }}@${{ inputs.pr-ref }} + uses: actions/checkout@v4 with: persist-credentials: false - ref: ${{github.event.pull_request.head.sha}} - repository: ${{github.event.pull_request.head.repo.full_name}} + ref: ${{ inputs.pr-sha }} + repository: ${{ inputs.pr-repo }} - uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} - - name: Install + - name: Install ${{ inputs.pr-repo }}@${{ inputs.pr-ref }} run: | npm install --ignore-scripts - - name: Run benchmark + - name: Run benchmark ${{ inputs.pr-repo }}@${{ inputs.pr-ref }} id: benchmark-pr run: | echo 'BENCH_RESULT_${{matrix.node-version}}<> $GITHUB_OUTPUT npm run --silent ${{inputs.npm-script}} >> $GITHUB_OUTPUT echo 'EOF' >> $GITHUB_OUTPUT - - uses: actions/checkout@v4 + - name: Checkout ${{ inputs.base-repo }}@${{ inputs.base-ref }} + uses: actions/checkout@v4 with: persist-credentials: false - ref: refs/heads/${{ github.event.repository.default_branch }} - repository: ${{github.event.pull_request.head.repo.full_name}} + ref: ${{ inputs.base-sha }} + repository: ${{ inputs.base-repo }} - - name: Install + - name: Install ${{ inputs.base-repo }}@${{ inputs.base-ref }} run: | npm install --ignore-scripts - - name: Run benchmark - id: benchmark-default + - name: Run benchmark ${{ inputs.base-repo }}@${{ inputs.base-ref }} + id: benchmark-base run: | echo 'BENCH_RESULT_${{matrix.node-version}}<> $GITHUB_OUTPUT npm run --silent ${{inputs.npm-script}} >> $GITHUB_OUTPUT @@ -77,35 +102,35 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} message: | **Node**: 18 - **${{github.event.pull_request.head.ref}}**: + ${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-18 }} ``` - **${{ github.event.repository.default_branch }}**: + ${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}): ``` - ${{ needs.benchmark.outputs.DEFAULT-BENCH-18 }} + ${{ needs.benchmark.outputs.BASE-BENCH-18 }} ``` --- **Node**: 20 - **${{github.event.pull_request.head.ref}}**: + ${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-20 }} ``` - **${{ github.event.repository.default_branch }}**: + ${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}): ``` - ${{ needs.benchmark.outputs.DEFAULT-BENCH-20 }} + ${{ needs.benchmark.outputs.BASE-BENCH-20 }} ``` --- **Node**: 21 - **${{github.event.pull_request.head.ref}}**: + ${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-21 }} ``` - **${{ github.event.repository.default_branch }}**: + ${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}): ``` - ${{ needs.benchmark.outputs.DEFAULT-BENCH-21 }} + ${{ needs.benchmark.outputs.BASE-BENCH-21 }} ``` diff --git a/README.md b/README.md index c4cf91b..e339b21 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,7 @@ jobs: id: remove-label with: route: DELETE /repos/{repo}/issues/{issue_number}/labels/{name} - repo: ${{ github.event.pull_request.head.repo.full_name }} + repo: ${{ github.event.pull_request.base.repo.full_name }} issue_number: ${{ github.event.pull_request.number }} name: benchmark env: