Skip to content

Commit

Permalink
fix(plugins-benchmark-pr): run comparison benchmark against target (#120
Browse files Browse the repository at this point in the history
)

* 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.
  • Loading branch information
mweberxyz authored Feb 25, 2024
1 parent 448d859 commit 9cb42e3
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 25 deletions.
73 changes: 49 additions & 24 deletions .github/workflows/plugins-benchmark-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }}
Expand All @@ -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}}<<EOF' >> $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}}<<EOF' >> $GITHUB_OUTPUT
npm run --silent ${{inputs.npm-script}} >> $GITHUB_OUTPUT
Expand All @@ -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 }}
```
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 9cb42e3

Please sign in to comment.