Extract Bundlesize to its own job/check

[aaemnnosttv]

Feature Description

In #2366 we migrated our JS tests to GitHub Actions, which created a new workflow for running our Jest tests as well as our Bundlesize tests.

These currently run in the same job, making both slower to run, but also Bundlesize has some integration with GitHub checks which makes the results of the run more visible, rather than simply pass or fail.

---

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

Acceptance criteria

• Bundlesize should be reported in each Pull Request via GitHub Actions checks.

Implementation Brief

Because this process needs to be done by someone with admin access to the repo and the ability to authorise Bundlesize to access the repo, it's best done by @felixarntz.

Work completed by @felixarntz, required to allow the check to appear

• Visit https://github.com/login/oauth/authorize?scope=repo%3Astatus&client_id=6756cb03a8d6528aca5a to get a Bundlesize GitHub token
• Allow Bundlesize access to Google repos. This is needed to add the check to each PR we run bundlesize on.
• Add the token obtained in the above step to the google/site-kit-wp repo as a GitHub secret named BUNDLESIZE_GITHUB_TOKEN.

Work to be done

• Add the following environment variables to the npm run test:bundlesize command in the "JS Tests" GitHub Action workflow:
  • CI_REPO_OWNER='google'
  • CI_REPO_NAME='site-kit-wp'
  • CI_COMMIT_MESSAGE={The git commit message for this commit, should be available as a GitHub Actions ENV var}
  • CI_COMMIT_SHA={The git commit SHA for this commit, should be available as a GitHub Actions ENV var}
  • CI=true (if not already set by GitHub Actions)
• See: https://github.com/siddharthkp/bundlesize#using-a-different-ci

Doing the above will mean future PRs running bundlesize will send their data to the Bundlesize app and report as a check, as #2366 already added the necessary info in the GitHub Action for this to work if the secret is available.

Test Coverage

• None needed.

Visual Regression Changes

• None.

QA Brief

• The Bundlesize check should appear in a PR created after this change is merged (create a new PR that passes tests to test this).

Changelog entry

• N/A

[tofumatt]

Based on usage in other repos, it looks like all we need to do here is outlined in this comment: #2366 (comment)

[tofumatt]

Note: from what I can tell, there isn't actually a way to stop us running the build process and bundlesize ourselves, so this won't increase the speed of our tests. The check is a more visible indication of the size change of a commit, but bundlesize won't run the build for us—other repos are doing what I committed in #2366.

[aaemnnosttv]

@tofumatt – correct, we will still need to run the build ourselves. Whether or not we split out the check to its own job may not be necessary or more efficient, but I think the added information is worth adding as a distinct check in the list even if that means it remains as part the current JS tests job.

[felixarntz]

@tofumatt I just added the BUNDLESIZE_GITHUB_TOKEN secret to the project. I agree with @aaemnnosttv though that we should probably alter our implementation so that the check shows up individually in the list.

[tofumatt]

Aha, I see. I missed this section in the docs: https://github.com/siddharthkp/bundlesize#using-a-different-ci

So we need to add a few more environment variables when we run bundlesize. After that it should add the check to the PR.

IB updated, should be straightforward according to the docs. 👍🏻 I think it's best to leave actually running the check inside the JS tests to be quicker/more efficient, but this will separate the info out into its own check. 🙂

[felixarntz]

IB ✅

[felixarntz]

@aaemnnosttv @tofumatt I see the bundlesize check already showing up among our actions. Is there actually anything else needed here?

[aaemnnosttv]

@felixarntz – if we have a branch where the new check has been triggered, I would expect it to appear in the list of all/known checks if that's what you mean. I don't see it on the list of checks for current PRs however, so I think the only thing left is to add the missing env vars to the command. See "Work to be done" above and Default environment variables for GHA.

[felixarntz]

@aaemnnosttv @benbowler Is there no way to make this a PR check that shows along the other checks (CLA, tests, etc.)?

[felixarntz]

Ping @aaemnnosttv

[aaemnnosttv]

@felixarntz it looks like it already is

I think this was mainly not working before due to instability on the backend of the service needed for this to work.

The bigger problem with bundlesize right now is that it doesn't really support bundles with a hash in the filename, so the diff will always be wrong for us since file names will never match for changed bundles.

This is one of the problems that preactjs/compressed-size-action solves. I also found that Preact used to use bundlesize as well, but moved away from it because bundlesize didn't support forks as well (which is probably why Gutenberg uses this too).

I agree it would be nice to be more visible on the check itself rather than only on the PR. It should be possible, but looks like it might require us to roll our own action to do it at the moment.

[aaemnnosttv]

Pulling back for a follow-up to apply the hash stripping as mentioned here.

[wpdarren]

@aaemnnosttv is it possible to get a few more steps on how to test this?

I've looked at the ticket, but not sure what I am looking for :)

[wpdarren]

QA Update: Pass ✅

Confirmed that Github actions is working for the bundlesize check.