run html-api-html5lib-tests grouped tests for changes to html api

[jorbin]

Follow up to #60227 and [58010].

The html-api-html5lib-tests group of tests is excluded by default since it contains many intentionally and dynamically skipped tests (see discussion in #60227 for why this is the best route for these tests).

These tests should be run when there is a change to the HTML API related files though so that it's not up to maintainers to remember to manually run them.

This adds a GitHub workflow to run the HTML API's html5lib tests when any changes are made to the HTML API, the html5lib tests, or the html5lib data files. Runs on multiple versions of PHP.

@desrosj Do you have time to take a look at this in case there's anything I missed?

Trac ticket: https://core.trac.wordpress.org/ticket/61209.

[desrosj]

I just realized that I had a Slack DM with @costdev (and I think @jorbin at some point) and never followed up with a summary here. Apologies for that!

I am strongly of the opinion that this should not have a separate workflow, and instead somehow integrated into the regular PHPUnit one. Since the idea is to only run these when the related files are updated, I suggested we add a flag to run this group of tests to a preexisting job and use something like tj-actions/changed-files to detect changes.

I've opened a PR that is an extension of @costdev's initial one and our discussion, but still is not quite detecting changes correctly, but is along what I had in mind.

For now I am punting this to 6.7.

@sirreal check out this awesome work @desrosj is doing - runs the html5lib tests in a separate job and _only_ once, instead of on every version of PHP and MySQL. should make it nicer to have the full test suite run on every change without holding up the CI process.

This is great! At the moment, it appears to be running the tests on this branch, is that expected?

I expected it _not to run_ because there are no changes to the relevant files.

It's running for this PR because I included the two relevant GitHub Action workflow files in the list that should run the tests when updated. If there are changes to the PHPUnit workflows, it's possible they could change how the html5lib tests are run, so we should run them just to confirm.

Can we not go ahead and run the tests on changes to every file in Core? there _are_ connections through the WP_Token_Map and kses.php. The test suite is quite fast, executing in 1.44s on my laptop, including the PHPUnit setup time.

Seems like this wouldn't be much of a cost on the CI runners or delay PRs but could provide important early warnings that something unintentionally changed the parser.

an we not go ahead and run the tests on changes to every file in Core?

The ​test group should only run if the following files are changed or patterns matched:

• src/wp-includes/html-api/.php
• tests/phpunit/tests/html-api/wpHtmlProcessorHtml5lib.php
• tests/phpunit/data/html5lib-tests/.dat
• .github/workflows/phpunit-tests.yml
• .github/workflows/reusable-phpunit-tests-v3.yml

Any other time the workflow runs the test group will not be run. They are running for this PR because the last two files in that list are being updated to support testing that specific group separately. Are you seeing something indicating differently @dmsnell?

@desrosj exactly. my point is that I don't understand why we're limiting it to running only when files in the wp-includes/html-api/ directory change. For one, other files in Core can affect the parser behavior, for two, the tests are really cheap to run. Is there a compelling reason to avoid running them on changes to any Core files?

Ah! Apologies, I misread your comment. This was based on the description on the Trac ticket and the original attempt to solve this in #6546.

@jorbin do you recall any context to that detail?

The reason these tests were excluded from the main test run is mentioned on the ticket:

The html-api-html5lib-tests group of tests is excluded by default since it contains many intentionally and dynamically skipped tests (see discussion in #60227 for why this is the best route for these tests).

These tests currently skip 568 tests:

OK, but incomplete, skipped, or risky tests!
Tests: 1498, Assertions: 930, Skipped: 568.

The amount of skipped tests were some of the main concerns in the original work: https://core.trac.wordpress.org/ticket/60227

The skipped tests may come down a bit more and at some point we may be able to hardcode lists of tests that we do not plan to support to omit them entirely (without skipping).

We can always revisit the decision and consider running these tests along with the rest of the tests.

I'd be very happy to see this move ahead, is there any way I can help?

Anecdotally, some (minor) failures in the html5lib tests slipped into trunk recently. It wasn't a big problem and has already been fixed, but this work would have prevented that from happening.