ampproject / amp-wp

Enable AMP on your WordPress site, the WordPress way.
https://wordpress.org/plugins/amp/
GNU General Public License v2.0
1.79k stars 383 forks source link

Add CI test for performance #4444

Closed westonruter closed 2 years ago

westonruter commented 4 years ago

Feature description

We can create a sample site that gets a build of the AMP plugin for any given pull request and then check that the generated AMP page does not degrade performance. The pull request should fail the test if the performance degrades and the performance increase should be highlighted in some way.

This can include Lighthouse CI.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

westonruter commented 4 years ago

This is for frontend specifically, and #3361 is focused on backend performance.

For a properly-configured site, page caching would be used so the backend performance would not be as critical (see #4386).

kienstra commented 4 years ago

Interesting idea. There might be a need for some tolerance when testing for performance regressions.

Maybe Lighthouse CI is better about this, but when using Lighthouse from the Node CLI, there was sometimes a big variance in trials of the same test.

For example, First Contentful Paint could vary by up to 10%. The Performance Score didn't vary much, but some trials had a variance of 10-20%.

schlessera commented 4 years ago

I'm all but finished with setting up Lighthouse in a GitHub Action, and I've set up a Lighthouse CI Server on a small GCP K8s stack.

For the Lighthouse CI server, it would be best to have a domain to refer to it, instead of an IP address, in case we need to change something about the GCP stack. Could we maybe have a subdomain on amp-wp.org to redirect to it, like lighthouse.amp-wp.org or lhci.amp-wp.org?

Also, by default, the Lighthouse CI server is openly accessible for reading, and the build token is meant to be publicly available so that PRs from outside contributors get measured as well.

As this is an open source project, I assume this is fine? It would basically be similar to Travis CI or GitHub Actions, where everyone can take a peek if they want.

westonruter commented 4 years ago

Yes, that makes sense to me. @amedina currently has access to the DNS for amp-wp.org, so he can add the desired CNAME.

amedina commented 4 years ago

It came to my attention some work being done with Puppeteer to detect and contextually surface 'offending' elements in terms of the LPC and CLS metrics.

The approach uses a puppeteer script that crawls a domain to get a set of URLs, which is run as: node cwv_crawler.js www.example.com 30, where 30 is the max number of URLs.

This is the link to the script.

If not already considered, let's take a look at this.

amedina commented 4 years ago

Another relevant and interesting approach (thanks @sebastianbenz for the pointer) is www.speedlify.dev/. I like the reporting they provide. On the simpler side, but neat.