Components: Improve output of CHANGELOG CI check

[chad1008]

What?

This PR introduces several changes to make the new Components package CI check's output more helpful (hat tip to @ciampo for the suggestions that inspired most of this)

Why?

It isn't immediately clear that this new check isn't a required check, and it shouldn't block a merge once the PR is approved.

In addition, the initial implementation only checked that the CHANGELOG had been modified by the PR in question. There were no checks in place to ensure it had been updated correctly.

How?

First, in the PR overview, the check's name now begins with the word OPTIONAL so it's more clear it won't block a merge. Second, if it does fail, the error output in the details section will explicitly state that the PR can be merged if the change is too small to warrant an entry.

Then, in addition to checking changes to the CHANGELOG, the script performs a few checks and outputs a relevant error if any of them don't pass:

• Confirms there is an entry in the PR's changes containing a link to the current PR. This check also protects against copy/paste fails where the wrong PR number is used.
• Confirms the Unreleased section is present (it always should be, but better safe than sorry)
• Confirms that the current PR's entry is inside that Unreleased section, which is helpful if a release happened while the PR was being reviewed.

Testing Instructions

• Check out this PR
• Branch off of it and create a new testing PR
• Make a change to a component file (not a mobile, storybook, or test file, those don't get checked)

No entry

• Don't update the CHANGELOG
• Commit and push your changes
• Confirm the check runs and fails with an error telling you to update the CHANGELOG.

Wrong PR number, in the right section

• Add a CHANGELOG entry in the Unreleased section, with the wrong PR number in the link.
• Commit and Push.
• Confirm the check fails and the error output asks you to make sure your entry has the right PR number.

Wrong PR number, in the wrong section

• Move your CHANGELOG entry out of the Unreleased section. Leave the wrong PR number in place.
• Commit and Push.
• Confirm the check fails and the error output asks you to make sure your entry has the right PR number.

Right PR number, in the wrong section

• Update your CHANGELOG entry to have the right PR number in the link.
• Commit and Push.
• Confirm the check fails and the error output asks you to make sure your entry is in the Unreleased section.

Right PR number, in the right section

• Move your CHANGELOG entry back into the Unreleased section.
• Commit and Push.
• Confirm the check passes.
• Have a piece of cake. You deserve it.

[github-actions]

Flaky tests detected in 136f9a8bbdefd3c1dc7399ac6864b8ab05d6cd4e.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4735904901
📝 Reported issues:

• [Flaky Test] shouldn't conflict with server-side autosave #35176 in specs/editor/various/autosave.test.js

[mirka]

I really like the additional checks! 😍 However, I am hesistant to commit this to the repo as is, because I'm afraid the maintenance cost will be too high (relative to the value added) whenever something breaks. To be fair, the script is really well documented, but it makes me uneasy especially since it isn't already decoupled to the point where we can test just the script part locally against a given markdown file.

What do you think about scaling back the complexity a bit, so we can recalibrate the cost–benefit here? For example, just using the raw file text instead of the git diff. And then simply matching the text between ^## Unreleased and the next ^## , and see if the PR number pattern appears in that chunk. Something like that?

[chad1008]

As much as I love a complex sed statement 😆 I think you're right. I wanted to verify that any CHANGELOG update the script finds is guaranteed to be in the current set of changes, but I don't actually think that's all that necessary. After all, why would a link the brand new, just-published PR be in the file if it wasn't part of the current changes?

I've pulled out all the extra bits and greatly simplified things. Thanks for pointing out the over-complexity! The script now:

1. Checks that the file has been changed. (there's a but here, see below)
2. Checks that there's a link to the correct PR.
3. Confirms that link is in the right place.

The expected errors from the various testing steps posted above don't change 🙂

[chad1008]

While updating this PR I noticed a small bug in the first check the script runs, which is when it checks to confirm the current branch contains changes to the CHANGELOG. When we initially set this up @ciampo pointed out it made sense to run the diff against the merge-base of the PR's branch, and not the most recent updates to origin/trunk.

I made that change, but noticed today that if the PR branch is behind trunk, the first check can fail if trunk has CHANGELOG updates. I did some digging and it looks like this is an issue with the Github context variable we're using. Rather than returning the base commit, it returns the head of the base branch. I found a few different reports of this, so it's not just us. A workaround exists but looks like it involves some semi-complex git gymnastics.

The good news is that with the new checks we're adding, the workflow still functions. The second check (which looks for the current PR's link in the CHANGELOG) will still get triggered. So basically, if the PR is behind trunk, there's a chance the error text will be slightly less specific, but the check will still do its job.

TL;DR: for PRs that need a rebase, the author might not get exactly the right error, but it will still prompt them to add an entry.

[mirka]

Tested in #49924 ✅

The simplified script looks a lot better! It is much more approachable for any future maintainer 😄