From edeb89254e11dcb19c9afe5bdaf915591efcd4f0 Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Tue, 20 Feb 2024 15:05:32 -0500 Subject: [PATCH 1/6] 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. --- .github/workflows/plugins-benchmark-pr.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/plugins-benchmark-pr.yml b/.github/workflows/plugins-benchmark-pr.yml index 38c8554..4bb5055 100644 --- a/.github/workflows/plugins-benchmark-pr.yml +++ b/.github/workflows/plugins-benchmark-pr.yml @@ -50,8 +50,8 @@ jobs: - 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: refs/heads/${{github.event.pull_request.base.repo.default_branch}} + repository: ${{github.event.pull_request.base.repo.full_name}} - name: Install run: | From a9911bcfbc91ae5078681d436920383e4ee30ade Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Tue, 20 Feb 2024 16:53:34 -0500 Subject: [PATCH 2/6] feature(plugins-benchmark-pr): factor up repos and refs and include in output --- .github/workflows/plugins-benchmark-pr.yml | 57 +++++++++++++++------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/.github/workflows/plugins-benchmark-pr.yml b/.github/workflows/plugins-benchmark-pr.yml index 4bb5055..091fcd0 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 - + source_repo: + type: string + default: ${{ github.event.pull_request.head.repo.full_name }} + required: false + source_sha: + type: string + default: ${{ github.event.pull_request.head.sha }} + required: false + source_ref: + type: string + default: ${{ github.event.pull_request.head.ref }} + required: false + target_repo: + type: string + default: ${{ github.event.pull_request.base.repo.full_name }} + required: false + target_sha: + type: string + default: ${{ github.event.pull_request.base.sha }} + required: false + target_ref: + type: string + default: ${{ github.event.pull_request.base.ref }} + required: false jobs: benchmark: if: ${{ github.event.label.name == 'benchmark' }} @@ -18,9 +41,9 @@ 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 }} + TARGET-BENCH-18: ${{ steps.benchmark-default.outputs.BENCH_RESULT_18 }} + TARGET-BENCH-20: ${{ steps.benchmark-default.outputs.BENCH_RESULT_20 }} + TARGET-BENCH-21: ${{ steps.benchmark-default.outputs.BENCH_RESULT_21 }} strategy: matrix: @@ -29,8 +52,8 @@ jobs: - 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.source_sha }} + repository: ${{ inputs.source_repo }} - uses: actions/setup-node@v4 with: @@ -50,8 +73,8 @@ jobs: - uses: actions/checkout@v4 with: persist-credentials: false - ref: refs/heads/${{github.event.pull_request.base.repo.default_branch}} - repository: ${{github.event.pull_request.base.repo.full_name}} + ref: ${{ inputs.target_sha }} + repository: ${{ inputs.target_repo }} - name: Install run: | @@ -77,35 +100,35 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} message: | **Node**: 18 - **${{github.event.pull_request.head.ref}}**: + ${{ inputs.source_repo }}@${{ inputs.source_sha }} (${{ inputs.source_ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-18 }} ``` - **${{ github.event.repository.default_branch }}**: + ${{ inputs.target_repo }}@${{ inputs.target_sha }} (${{ inputs.target_ref }}): ``` - ${{ needs.benchmark.outputs.DEFAULT-BENCH-18 }} + ${{ needs.benchmark.outputs.TARGET-BENCH-18 }} ``` --- **Node**: 20 - **${{github.event.pull_request.head.ref}}**: + ${{ inputs.source_repo }}@${{ inputs.source_sha }} (${{ inputs.source_ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-20 }} ``` - **${{ github.event.repository.default_branch }}**: + ${{ inputs.target_repo }}@${{ inputs.target_sha }} (${{ inputs.target_ref }}): ``` - ${{ needs.benchmark.outputs.DEFAULT-BENCH-20 }} + ${{ needs.benchmark.outputs.TARGET-BENCH-20 }} ``` --- **Node**: 21 - **${{github.event.pull_request.head.ref}}**: + ${{ inputs.source_repo }}@${{ inputs.source_sha }} (${{ inputs.source_ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-21 }} ``` - **${{ github.event.repository.default_branch }}**: + ${{ inputs.target_repo }}@${{ inputs.target_sha }} (${{ inputs.target_ref }}): ``` - ${{ needs.benchmark.outputs.DEFAULT-BENCH-21 }} + ${{ needs.benchmark.outputs.TARGET-BENCH-21 }} ``` From aae6fe432736dd84619ed5d8904af99fbbb139c1 Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Tue, 20 Feb 2024 21:50:21 -0500 Subject: [PATCH 3/6] refactor(plugins-benchmark-pr): inputs to dash case For consistency --- .github/workflows/plugins-benchmark-pr.yml | 32 +++++++++++----------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/plugins-benchmark-pr.yml b/.github/workflows/plugins-benchmark-pr.yml index 091fcd0..222c2d3 100644 --- a/.github/workflows/plugins-benchmark-pr.yml +++ b/.github/workflows/plugins-benchmark-pr.yml @@ -7,27 +7,27 @@ on: type: string default: benchmark required: false - source_repo: + source-repo: type: string default: ${{ github.event.pull_request.head.repo.full_name }} required: false - source_sha: + source-sha: type: string default: ${{ github.event.pull_request.head.sha }} required: false - source_ref: + source-ref: type: string default: ${{ github.event.pull_request.head.ref }} required: false - target_repo: + target-repo: type: string default: ${{ github.event.pull_request.base.repo.full_name }} required: false - target_sha: + target-sha: type: string default: ${{ github.event.pull_request.base.sha }} required: false - target_ref: + target-ref: type: string default: ${{ github.event.pull_request.base.ref }} required: false @@ -52,8 +52,8 @@ jobs: - uses: actions/checkout@v4 with: persist-credentials: false - ref: ${{ inputs.source_sha }} - repository: ${{ inputs.source_repo }} + ref: ${{ inputs.source-sha }} + repository: ${{ inputs.source-repo }} - uses: actions/setup-node@v4 with: @@ -73,8 +73,8 @@ jobs: - uses: actions/checkout@v4 with: persist-credentials: false - ref: ${{ inputs.target_sha }} - repository: ${{ inputs.target_repo }} + ref: ${{ inputs.target-sha }} + repository: ${{ inputs.target-repo }} - name: Install run: | @@ -100,11 +100,11 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} message: | **Node**: 18 - ${{ inputs.source_repo }}@${{ inputs.source_sha }} (${{ inputs.source_ref }}): + ${{ inputs.source-repo }}@${{ inputs.source-sha }} (${{ inputs.source-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-18 }} ``` - ${{ inputs.target_repo }}@${{ inputs.target_sha }} (${{ inputs.target_ref }}): + ${{ inputs.target-repo }}@${{ inputs.target-sha }} (${{ inputs.target-ref }}): ``` ${{ needs.benchmark.outputs.TARGET-BENCH-18 }} ``` @@ -112,11 +112,11 @@ jobs: --- **Node**: 20 - ${{ inputs.source_repo }}@${{ inputs.source_sha }} (${{ inputs.source_ref }}): + ${{ inputs.source-repo }}@${{ inputs.source-sha }} (${{ inputs.source-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-20 }} ``` - ${{ inputs.target_repo }}@${{ inputs.target_sha }} (${{ inputs.target_ref }}): + ${{ inputs.target-repo }}@${{ inputs.target-sha }} (${{ inputs.target-ref }}): ``` ${{ needs.benchmark.outputs.TARGET-BENCH-20 }} ``` @@ -124,11 +124,11 @@ jobs: --- **Node**: 21 - ${{ inputs.source_repo }}@${{ inputs.source_sha }} (${{ inputs.source_ref }}): + ${{ inputs.source-repo }}@${{ inputs.source-sha }} (${{ inputs.source-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-21 }} ``` - ${{ inputs.target_repo }}@${{ inputs.target_sha }} (${{ inputs.target_ref }}): + ${{ inputs.target-repo }}@${{ inputs.target-sha }} (${{ inputs.target-ref }}): ``` ${{ needs.benchmark.outputs.TARGET-BENCH-21 }} ``` From b47e7bb702d60be7a6faa3686fd376babb27fc7e Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Thu, 22 Feb 2024 09:41:57 -0500 Subject: [PATCH 4/6] refactor(plugins-benchmark-pr): consistency input and output naming, refs in step names --- .github/workflows/plugins-benchmark-pr.yml | 58 +++++++++++----------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/.github/workflows/plugins-benchmark-pr.yml b/.github/workflows/plugins-benchmark-pr.yml index 222c2d3..c6d5be3 100644 --- a/.github/workflows/plugins-benchmark-pr.yml +++ b/.github/workflows/plugins-benchmark-pr.yml @@ -7,27 +7,27 @@ on: type: string default: benchmark required: false - source-repo: + pr-repo: type: string default: ${{ github.event.pull_request.head.repo.full_name }} required: false - source-sha: + pr-sha: type: string default: ${{ github.event.pull_request.head.sha }} required: false - source-ref: + pr-ref: type: string default: ${{ github.event.pull_request.head.ref }} required: false - target-repo: + base-repo: type: string default: ${{ github.event.pull_request.base.repo.full_name }} required: false - target-sha: + base-sha: type: string default: ${{ github.event.pull_request.base.sha }} required: false - target-ref: + base-ref: type: string default: ${{ github.event.pull_request.base.ref }} required: false @@ -41,46 +41,48 @@ 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 }} - TARGET-BENCH-18: ${{ steps.benchmark-default.outputs.BENCH_RESULT_18 }} - TARGET-BENCH-20: ${{ steps.benchmark-default.outputs.BENCH_RESULT_20 }} - TARGET-BENCH-21: ${{ steps.benchmark-default.outputs.BENCH_RESULT_21 }} + BASE-BENCH-18: ${{ steps.benchmark-default.outputs.BENCH_RESULT_18 }} + BASE-BENCH-20: ${{ steps.benchmark-default.outputs.BENCH_RESULT_20 }} + BASE-BENCH-21: ${{ steps.benchmark-default.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: ${{ inputs.source-sha }} - repository: ${{ inputs.source-repo }} + 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: ${{ inputs.target-sha }} - repository: ${{ inputs.target-repo }} + 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 + - name: Run benchmark ${{ inputs.base-repo }}@${{ inputs.base-ref }} id: benchmark-default run: | echo 'BENCH_RESULT_${{matrix.node-version}}<> $GITHUB_OUTPUT @@ -100,35 +102,35 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} message: | **Node**: 18 - ${{ inputs.source-repo }}@${{ inputs.source-sha }} (${{ inputs.source-ref }}): + ${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-18 }} ``` - ${{ inputs.target-repo }}@${{ inputs.target-sha }} (${{ inputs.target-ref }}): + ${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}): ``` - ${{ needs.benchmark.outputs.TARGET-BENCH-18 }} + ${{ needs.benchmark.outputs.BASE-BENCH-18 }} ``` --- **Node**: 20 - ${{ inputs.source-repo }}@${{ inputs.source-sha }} (${{ inputs.source-ref }}): + ${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-20 }} ``` - ${{ inputs.target-repo }}@${{ inputs.target-sha }} (${{ inputs.target-ref }}): + ${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}): ``` - ${{ needs.benchmark.outputs.TARGET-BENCH-20 }} + ${{ needs.benchmark.outputs.BASE-BENCH-20 }} ``` --- **Node**: 21 - ${{ inputs.source-repo }}@${{ inputs.source-sha }} (${{ inputs.source-ref }}): + ${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}): ``` ${{ needs.benchmark.outputs.PR-BENCH-21 }} ``` - ${{ inputs.target-repo }}@${{ inputs.target-sha }} (${{ inputs.target-ref }}): + ${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}): ``` - ${{ needs.benchmark.outputs.TARGET-BENCH-21 }} + ${{ needs.benchmark.outputs.BASE-BENCH-21 }} ``` From 75ab845f76479a82a92bdf7b72d61a79b491f816 Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Thu, 22 Feb 2024 09:45:55 -0500 Subject: [PATCH 5/6] refactor(plugins-benchmark-pr): consistency in step id --- .github/workflows/plugins-benchmark-pr.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/plugins-benchmark-pr.yml b/.github/workflows/plugins-benchmark-pr.yml index c6d5be3..cdaddb9 100644 --- a/.github/workflows/plugins-benchmark-pr.yml +++ b/.github/workflows/plugins-benchmark-pr.yml @@ -41,9 +41,9 @@ 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 }} - BASE-BENCH-18: ${{ steps.benchmark-default.outputs.BENCH_RESULT_18 }} - BASE-BENCH-20: ${{ steps.benchmark-default.outputs.BENCH_RESULT_20 }} - BASE-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: @@ -83,7 +83,7 @@ jobs: npm install --ignore-scripts - name: Run benchmark ${{ inputs.base-repo }}@${{ inputs.base-ref }} - id: benchmark-default + id: benchmark-base run: | echo 'BENCH_RESULT_${{matrix.node-version}}<> $GITHUB_OUTPUT npm run --silent ${{inputs.npm-script}} >> $GITHUB_OUTPUT From 70fd1765ec2c3ba14f9cd8bbc852d6a34b56d788 Mon Sep 17 00:00:00 2001 From: Matt Weber <1062734+mweberxyz@users.noreply.github.com> Date: Thu, 22 Feb 2024 10:11:36 -0500 Subject: [PATCH 6/6] docs(plugins-benchmark-pr): label should be removed from base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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: