Skip to content

Commit 9cb42e3

Browse files
authored
fix(plugins-benchmark-pr): run comparison benchmark against target (#120)
* fix(plugins-benchmark-pr): ensure comparison benchmarks run against PR base Closes #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.
1 parent 448d859 commit 9cb42e3

File tree

2 files changed

+50
-25
lines changed

2 files changed

+50
-25
lines changed

.github/workflows/plugins-benchmark-pr.yml

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,30 @@ on:
77
type: string
88
default: benchmark
99
required: false
10-
10+
pr-repo:
11+
type: string
12+
default: ${{ github.event.pull_request.head.repo.full_name }}
13+
required: false
14+
pr-sha:
15+
type: string
16+
default: ${{ github.event.pull_request.head.sha }}
17+
required: false
18+
pr-ref:
19+
type: string
20+
default: ${{ github.event.pull_request.head.ref }}
21+
required: false
22+
base-repo:
23+
type: string
24+
default: ${{ github.event.pull_request.base.repo.full_name }}
25+
required: false
26+
base-sha:
27+
type: string
28+
default: ${{ github.event.pull_request.base.sha }}
29+
required: false
30+
base-ref:
31+
type: string
32+
default: ${{ github.event.pull_request.base.ref }}
33+
required: false
1134
jobs:
1235
benchmark:
1336
if: ${{ github.event.label.name == 'benchmark' }}
@@ -18,47 +41,49 @@ jobs:
1841
PR-BENCH-18: ${{ steps.benchmark-pr.outputs.BENCH_RESULT_18 }}
1942
PR-BENCH-20: ${{ steps.benchmark-pr.outputs.BENCH_RESULT_20 }}
2043
PR-BENCH-21: ${{ steps.benchmark-pr.outputs.BENCH_RESULT_21 }}
21-
DEFAULT-BENCH-18: ${{ steps.benchmark-default.outputs.BENCH_RESULT_18 }}
22-
DEFAULT-BENCH-20: ${{ steps.benchmark-default.outputs.BENCH_RESULT_20 }}
23-
DEFAULT-BENCH-21: ${{ steps.benchmark-default.outputs.BENCH_RESULT_21 }}
44+
BASE-BENCH-18: ${{ steps.benchmark-base.outputs.BENCH_RESULT_18 }}
45+
BASE-BENCH-20: ${{ steps.benchmark-base.outputs.BENCH_RESULT_20 }}
46+
BASE-BENCH-21: ${{ steps.benchmark-base.outputs.BENCH_RESULT_21 }}
2447

2548
strategy:
2649
matrix:
2750
node-version: [18, 20, 21]
2851
steps:
29-
- uses: actions/checkout@v4
52+
- name: Checkout ${{ inputs.pr-repo }}@${{ inputs.pr-ref }}
53+
uses: actions/checkout@v4
3054
with:
3155
persist-credentials: false
32-
ref: ${{github.event.pull_request.head.sha}}
33-
repository: ${{github.event.pull_request.head.repo.full_name}}
56+
ref: ${{ inputs.pr-sha }}
57+
repository: ${{ inputs.pr-repo }}
3458

3559
- uses: actions/setup-node@v4
3660
with:
3761
node-version: ${{ matrix.node-version }}
3862

39-
- name: Install
63+
- name: Install ${{ inputs.pr-repo }}@${{ inputs.pr-ref }}
4064
run: |
4165
npm install --ignore-scripts
4266
43-
- name: Run benchmark
67+
- name: Run benchmark ${{ inputs.pr-repo }}@${{ inputs.pr-ref }}
4468
id: benchmark-pr
4569
run: |
4670
echo 'BENCH_RESULT_${{matrix.node-version}}<<EOF' >> $GITHUB_OUTPUT
4771
npm run --silent ${{inputs.npm-script}} >> $GITHUB_OUTPUT
4872
echo 'EOF' >> $GITHUB_OUTPUT
4973
50-
- uses: actions/checkout@v4
74+
- name: Checkout ${{ inputs.base-repo }}@${{ inputs.base-ref }}
75+
uses: actions/checkout@v4
5176
with:
5277
persist-credentials: false
53-
ref: refs/heads/${{ github.event.repository.default_branch }}
54-
repository: ${{github.event.pull_request.head.repo.full_name}}
78+
ref: ${{ inputs.base-sha }}
79+
repository: ${{ inputs.base-repo }}
5580

56-
- name: Install
81+
- name: Install ${{ inputs.base-repo }}@${{ inputs.base-ref }}
5782
run: |
5883
npm install --ignore-scripts
5984
60-
- name: Run benchmark
61-
id: benchmark-default
85+
- name: Run benchmark ${{ inputs.base-repo }}@${{ inputs.base-ref }}
86+
id: benchmark-base
6287
run: |
6388
echo 'BENCH_RESULT_${{matrix.node-version}}<<EOF' >> $GITHUB_OUTPUT
6489
npm run --silent ${{inputs.npm-script}} >> $GITHUB_OUTPUT
@@ -77,35 +102,35 @@ jobs:
77102
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
78103
message: |
79104
**Node**: 18
80-
**${{github.event.pull_request.head.ref}}**:
105+
${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}):
81106
```
82107
${{ needs.benchmark.outputs.PR-BENCH-18 }}
83108
```
84-
**${{ github.event.repository.default_branch }}**:
109+
${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}):
85110
```
86-
${{ needs.benchmark.outputs.DEFAULT-BENCH-18 }}
111+
${{ needs.benchmark.outputs.BASE-BENCH-18 }}
87112
```
88113
89114
---
90115
91116
**Node**: 20
92-
**${{github.event.pull_request.head.ref}}**:
117+
${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}):
93118
```
94119
${{ needs.benchmark.outputs.PR-BENCH-20 }}
95120
```
96-
**${{ github.event.repository.default_branch }}**:
121+
${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}):
97122
```
98-
${{ needs.benchmark.outputs.DEFAULT-BENCH-20 }}
123+
${{ needs.benchmark.outputs.BASE-BENCH-20 }}
99124
```
100125
101126
---
102127
103128
**Node**: 21
104-
**${{github.event.pull_request.head.ref}}**:
129+
${{ inputs.pr-repo }}@${{ inputs.pr-sha }} (${{ inputs.pr-ref }}):
105130
```
106131
${{ needs.benchmark.outputs.PR-BENCH-21 }}
107132
```
108-
**${{ github.event.repository.default_branch }}**:
133+
${{ inputs.base-repo }}@${{ inputs.base-sha }} (${{ inputs.base-ref }}):
109134
```
110-
${{ needs.benchmark.outputs.DEFAULT-BENCH-21 }}
135+
${{ needs.benchmark.outputs.BASE-BENCH-21 }}
111136
```

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ jobs:
101101
id: remove-label
102102
with:
103103
route: DELETE /repos/{repo}/issues/{issue_number}/labels/{name}
104-
repo: ${{ github.event.pull_request.head.repo.full_name }}
104+
repo: ${{ github.event.pull_request.base.repo.full_name }}
105105
issue_number: ${{ github.event.pull_request.number }}
106106
name: benchmark
107107
env:

0 commit comments

Comments
 (0)