Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Collect lighthouse CI on each frontend change #4994

Merged
merged 7 commits into from
Sep 26, 2024
Merged

Collect lighthouse CI on each frontend change #4994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9217da1
Collect lighthouse CI on each frontend change
sarayourfriend Sep 26, 2024
2782e15
Fix substitution in recipe
sarayourfriend Sep 26, 2024
7c5527d
Add the token to the environment
sarayourfriend Sep 26, 2024
32de0dd
Only run if nuxt-build finished successfully
sarayourfriend Sep 26, 2024
81b4b26
Upload LHCI artifacts
sarayourfriend Sep 26, 2024
494d524
Report lighthouse results in action
sarayourfriend Sep 26, 2024
6471d9e
Build and run with node_env production
sarayourfriend Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/.lighthouserc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
ci:
collect:
url:
- http://127.0.0.1:8443/
startServerCommand: "just p frontend start"
startServerReadyPattern: "Listening on http"
upload:
# temporary target until either temporary-public-storage is back
# (https://github.com/GoogleChrome/lighthouse-ci/issues/1072)
# or we spin up our own Openverse lhci server
target: "filesystem"
37 changes: 37 additions & 0 deletions .github/workflows/ci_cd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,43 @@ jobs:
issue-number: ${{ github.event.pull_request.number }}
body: ${{ steps.help-body.outputs.help_body }}

lighthouse-ci:
name: Collect Lighthouse CI results
runs-on: ubuntu-latest
needs:
- nuxt-build

steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Setup CI env
uses: ./.github/actions/setup-env
with:
setup_python: false
install_recipe: node-install

- name: Run build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we reuse the built Nuxt app for this and for the k6?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would need to push the .nuxt directory as an artifact, I think, but it should be possible. It only takes 32-seconds though. Downloading and decompressing the artifact could take just as long.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building makes more sense, considering the time it takes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nuxt 3 made it a lot better 🙂

run: just frontend/run build
env:
DEPLOYMENT_ENV: production
NODE_ENV: production

- name: Run Lighthouse CI
id: lhci-autorun
# Lighthouse CI runs the webserver for us, as configured in lighthouserc
env:
NODE_ENV: production
run: |
pnpm --package=@lhci/cli dlx lhci autorun --config .github/.lighthouserc.yml

- name: Display Report
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add it as a comment on the PR? Or do you think that would be too much?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other actions out there that generate markdown tables, but they all looked pretty bad (too information dense) and the comments would be very long (similar to the old bundle size comments).

I think we should think of ways of making the results more noticeable, but those would probably come more from writing assertions so that the check fails when we get results we don't want. Lighthouse CI recommends you wait to add assertions until after you've had a chance to understand your results and make changes to improve them a bit first.

Ultimately, I think this action would be the best thing, once we are comfortable in our understanding of what we want from Lighthouse. That action requires setting up access to S3 for saving the files, which I'd prefer to do as a separate step to this one of just getting it running.

if: always()
uses: jackywithawhitedog/lighthouse-viewer-action@v2
with:
resultsPath: .lighthouseci
lighthouseOutcome: ${{ steps.lhci-autorun.outcome }}

#################
# Documentation #
#################
Expand Down
1 change: 1 addition & 0 deletions justfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ f:
# alias for `pnpm --filter {package} run {script}`
[positional-arguments]
p package script *args:
#!/usr/bin/env bash
pnpm --filter {{ package }} run {{ script }} "${@:3}"

# Run eslint with --fix and default file selection enabled; used to enable easy file overriding whilst retaining the defaults when running --all-files
Expand Down