Skip to content

Try: Build WP once for PHPUnit test suite. - #12779

Open
peterwilsoncc wants to merge 6 commits into
WordPress:trunkfrom
peterwilsoncc:try/phpunit-build-wp-once
Open

Try: Build WP once for PHPUnit test suite.#12779
peterwilsoncc wants to merge 6 commits into
WordPress:trunkfrom
peterwilsoncc:try/phpunit-build-wp-once

Conversation

@peterwilsoncc

@peterwilsoncc peterwilsoncc commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Runs npm run build:dev in the newly created prepare-gutenberg reusable workflow for the PHPUnit tests.

This allows the step to be bypassed in each test, saving about 30 seconds of runtime.

As the local environment uses NPM for the docker pull and configuration, the npm ci step is still required. That can be a project for another time.

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

Use of AI Tools

AI assistance: Yes
Tool(s): GitHub Copilot
Used for: Initial drafting of this in another PR that also included some docker changes. The creation of this PR built upon portions of that but is much reduced in scope. AI was not used for the generation of this PR per se.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@peterwilsoncc
peterwilsoncc marked this pull request as ready for review July 31, 2026 01:21
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props peterwilsoncc, lancewillett, desrosj.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Comment thread .github/workflows/reusable-prepare-gutenberg.yml

@lancewillett lancewillett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed fc3173e. The coverage workflow now uses the shared WordPress build artifact: both coverage jobs downloaded and applied it, then skipped their local builds. The latest PHPUnit matrix and workflow lint checks pass. No further findings.

No other blockers to land this.


Adversarial code review using gpt-5.6-sol

@peterwilsoncc

peterwilsoncc commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@lancewillett I wanted to see what happened to the tests if they were re-run after the artifacts expired and did so on PHP 7.4 / MySQL 9.7.

The test fails at the step attempting to download the Gutenberg artifact but the same would occur in the equivalent step for applying the WP-Dev build artifact.

  • Do you think it's worth handling this and attempting to download/build in the reusable workflow?
  • If so, are you able to suggest how?

Tests are now failing following my attempt to re-run the single event, they should work on a full run.

@lancewillett

lancewillett commented Aug 4, 2026

Copy link
Copy Markdown
Member

Yes, and it is cheaper than it looks, because the Gutenberg fallback already exists. npm run build:dev runs gutenberg:verify, which downloads when the directory or hash file is missing and hard-fails if the hash still mismatches afterwards. The pin holds as well: GUTENBERG_EXPECTED_SHA is a prepare-gutenberg job output, and outputs survive a partial re-run. On your attempt 2 the producer kept its attempt-1 start time and was not re-executed.

Suggested shape, tolerating the miss only on re-runs so attempt 1 still fails loudly on a real digest-mismatch:

      - name: Download WordPress build
        id: download-wordpress
        if: inputs.wordpress-build-artifact != ''
        continue-on-error: ${{ github.run_attempt != '1' }}
        uses: actions/download-artifact@... # unchanged

      - name: Apply WordPress build
        if: steps.download-wordpress.outcome == 'success'
        run: tar -xzf /tmp/wordpress-build/wordpress-build.tar.gz # unchanged

      - name: Build WordPress
        if: steps.download-wordpress.outcome != 'success'
        run: npm run build:dev
        env:
          GUTENBERG_EXPECTED_SHA: ${{ inputs.gutenberg-sha }}

Same treatment on the Gutenberg download. A skipped step reports outcome: skipped, so != 'success' also covers callers that pass no artifact. It replaces the == '' check rather than adding to it.

Raising retention is the wrong lever: 35 MB of Gutenberg plus 15 MB of build, on every push.

One aside. The Gutenberg download may be unnecessary in matrix jobs once the build artifact is applied, since I found no runtime reference to gutenberg/ in tests/phpunit/, tools/local-env/, the bootstrap, or the built src/ tree. That would remove one expiry failure mode outright. I measured the download at about 3 seconds though, so it is simplification rather than speed.

@desrosj

desrosj commented Aug 5, 2026

Copy link
Copy Markdown
Member

I haven't gotten to review this in depth just yet (which I would love a chance to do), but I wanted to note that the PHPUnit test jobs previously had a prepare step prior to the strategy.matrix expanding into parallel jobs.

It was removed in r50441 when switching back to running the PHPUnit test suite against the src instead of build, but I seem to recall at the time that it was actually slower over many runs to prepare the codebase once for use by the actual test jobs. That was 5+ years ago now so it's likely my memory is failing me, and a lot h as improved in both our workflows and GitHub Actions (the artifacts API was refactored at around a ~98% performance gain a few years ago). But I just wanted to make sure that we are mindful of the possible timing variation.

@peterwilsoncc

Copy link
Copy Markdown
Contributor Author

The build step was added back in to phpunit in 22294af / r61438 as some PHP files are coming from the Gutenberg directory now.

I think this is taking a few minutes longer for human minutes but dropping the run time minutes. Which is preferable to optimize for?

@peterwilsoncc
peterwilsoncc force-pushed the try/phpunit-build-wp-once branch 2 times, most recently from c69b241 to ceee051 Compare August 6, 2026 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants