Try adding codecov

[aaronjorbin]

In order to get a sense of the code coverage and how it changes, this adds codecov. The implementation is inspired by the version in https://github.com/WordPress/plugin-check

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[aaronjorbin]

Codecov is viewable at https://app.codecov.io/github/wordpress/wp-ai-client/tree/add%2Fcodecov

[github-actions]

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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aaronjorbin <jorbin@git.wordpress.org>
Co-authored-by: justlevine <justlevine@git.wordpress.org>
Co-authored-by: desrosj <desrosj@git.wordpress.org>
Co-authored-by: JasonTheAdams <jason_the_adams@git.wordpress.org>

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

[justlevine]

Someone double check me (1am on my phone) but I think all these can be deduped to a single

- php: '8.4'
  wordpress: 'trunk'
  coverage: true
  multisite: [true, false]
  experimental: [true, false]

(Semantically: run coverage on the latest branch against both single/multisite and with/without experimental )

[aaronjorbin]

I didn't include experimental in the coverage runs since I figured that the barriers for something being experimental should be as low as possible and that includes affected the automated test coverage.

[justlevine]

Oh, interesting. TBH I'm not entirely sure what experimental is supposed to capture in this plugin. Seems all it does is allow continue-on-error, but I'm not seeing it map to an env var, feature flag, test group, cli param... 🤷

[aaronjorbin]

Interesting. I wonder if the idea was to do more, but that never got implemented?

@felixarntz, it looks like the experimental flag was added when you first setup tests here. Is there more that any context we are missing or anything you think we should consider here?

This is feeling like it might be a tangent that is separate from the goal of understanding test coverage. Maybe a followup ticket related to the experimental flag would make sense?

[justlevine]

Barely matters here since we only need this until the (hopeful) core merge, but to help future folks looking for prior art (and the humans/agents dealing with cognitive load), could we move the configs into an implementation detail inside phpunit.xml.dist instead of an explicit npm command (or two)

Refs from WordPress/ai:

• https://github.com/WordPress/ai/blob/2d80b124d5af7afa89cd82b54557cba3b10b4b3b/phpunit.xml.dist#L21-L30

• https://github.com/WordPress/ai/blob/2d80b124d5af7afa89cd82b54557cba3b10b4b3b/package.json#L27

• https://github.com/WordPress/ai/blob/2d80b124d5af7afa89cd82b54557cba3b10b4b3b/.github/workflows/test.yml#L210-L248

[aaronjorbin]

As I mentioned in the description, this uses the same implementation as plugin check, so there shouldn't be any worry about new prior art.

[justlevine]

Not sure I followed. I'm saying that the 2-year-old config used by plugin-check is suboptimal and dated, which is why we went with a different pattern in the other Core AI Building blocks (WordPress/ai is the above example, but MCP Adapter and the now archived abilities-api also follow this pattern.)

In the very nit-scenario where someone is scaffolding a canonical plugin and chooses to use this plugin as prior art, they're not going to notice that you referenced plugin check in the PR description, and they're probably not going to realize that even though the command is recently committed, it's not following the conventions used elsewhere.

(Unless your goal was just to generate the codecov to get an understanding for the merge proposal, and not merge this PR in? In that case ya then it really doesn't matter)

[aaronjorbin]

You expressed worry about this adding new prior art that can increase cognitive load, I'm pointing out that this is not new prior art.

I don't think that explicit actions are "suboptimal and dated", can you explain why you think it is?

My goal is to get the coverage in the short term for helping make the decision on the merge proposal and if this repo continues to be used, to surface coverage information on code changes so that can inform decision making in the long term.

[justlevine]

You expressed worry about this adding new prior art that can increase cognitive load, I'm pointing out that this is not new prior art.

Okay so to clarify my concern is recency bias and cross-repo maintainer friction within the Core AI team, not about the novelty of a specific implementation pattern in the WordPress org. This plugin as a whole serves as prior art and despite being "newer" than the other core-ai repos that iterated with intentionality on older patterns, it's reaching for a pattern written 3 years well before guidelines for WordPress/* hosted projects were released. IOW

I don't think that explicit actions are "suboptimal and dated", can you explain why you think it is?

Step before the explicit actions, the first inefficiency is that we a rely on a configuration that isn't codified in config, but passed as an argument. We lose config schema linting, portability/reusability, overloadability via a local .xml or script flags, increased possibility for typos, etc.

Next the specific script implementations:

• It doesn't follow the conventions of our other tooling/test scripts, that all keep their configs inside the config files.
• It's misleading: npm run test-php-*coverage won't actually generate coverage unless you've started wp-env with xdebug.
• It scales poorly at n*2. a 3rd suite becomes 6 commands to juggle between, a 4th 8 etc.

I'm sure there were others discussed at the time we were scaffolding the other repos, but those are what pop to mind. Anecdotally I've seen both AI and human contributors struggle to intuit how to write unit tests.

Whereas:

1. npm run wp-env start -- --xdebug
2. `npm run test:php:{suite} -- --{whatever flags }

Is shorter, requires knowledge of fewer commands, more self documenting, overloadable, creates no overhead, and general has fewerr opportunities for user ever.

Now sure, this might not reach "death by 1000 papercuts" levels of inefficiency, or warrant a change on an existing project, but it was an intentional and considered iteration that took into account older patterns from plugin-check, performance, and more recent decisions like WordPress/two-factor#717 (which is what I meant by "dated": internally we have more recent iterations . In lieu of cross-silo communication, recency bias is one of the few reliable ways to communicate change. If project quality well governed - as many parts of WordPress are - it's usually safe to assume that gatekeepers have discussed and considered not just the Chesterton Fences behind previous config decisions, but the cost/benefit of deviating from prior art. Like we're doing now 😅)

if this repo continues to be used

Ironically, in this case I care less about this, because we can course correct in a follow-up PR. My concern here is scoped to this getting merged, and the plugin being archived shortly after, where the files exist in the diff and a 2026 commit date, but you gotta go spelunking to see that this is not the iteration that the team's other projects and canonical plugins are following. Most folks are just likely to replace the includes and reuse the scaffold with a few search-replaces and no second glance.

[desrosj]

I don't have strong opinions here. I do kind of like the idea of putting as much as possible inside the phpunit.xml.dist configuration file.

I seem to recall that there was a reason why specifying these things in the command line was preferable. I looked back in Core Trac but couldn't find what I'm thinking of. In r59356, the local Docker environment was updated to automatically enable xDebug when the intention seemed to be generating a coverage report. This can save contributors time figuring out why their report is not being created, but that does not make use of wp-env.

It's always possible someone uses the wrong code as an example. In my opinion, we should create a template repository for someone to use when building out provider plugins (which I believe is what you suggesting would be created using this plugin as a building block) that has all of the preferred best practices at any given point in time.

[JasonTheAdams]

Hi! Do we still need this in light of @desrosj adding code coverage in the core PR (WordPress/wordpress-develop#10881 (comment))?

[desrosj]

This looks fine to me. I added a few small suggestions. But none that I'd consider a blocker.

[desrosj]

Suggested change

	name: PHP ${{ matrix.php }} - WP ${{ matrix.wordpress }} - ${{ matrix.multisite && 'Multisite' || 'Single site' }}${{ matrix.experimental && ' (experimental)' || '' }} ${{ matrix.coverage && ' (coverage)' || '' }}
	name: PHP ${{ matrix.php }} - WP ${{ matrix.wordpress }} - ${{ matrix.multisite && 'Multisite' || 'Single site' }}${{ matrix.experimental && ' (experimental)' || '' }} ${{ matrix.coverage && ' (with coverage)' || '' }}

[desrosj]

Suggested change

	uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de
	uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.2.2

Including the human-readable version number is always helpful.

[desrosj]

Would multisite and single-site (or single) make more sense here? In wordpress-develop we pass single and multisite (the php actually needs to be removed because only 1 flag per submission is recommended).

Since there's only one form of testing being submitted, (and to my knowledge) only PHPUnit tests have been submitted to Codecov from WordPress organization repositories, I don't think we need to segment by unit, integration, etc.

The docs note that you can also use it to segment reports for multiple features. But we also do not do that currently anywhere that I'm aware of.

Suggested change

	flags: unit
	flags: ${{ matrix.multisite && 'multisite' || 'single' }}

[desrosj]

I don't have strong opinions here. I do kind of like the idea of putting as much as possible inside the phpunit.xml.dist configuration file.

I seem to recall that there was a reason why specifying these things in the command line was preferable. I looked back in Core Trac but couldn't find what I'm thinking of. In r59356, the local Docker environment was updated to automatically enable xDebug when the intention seemed to be generating a coverage report. This can save contributors time figuring out why their report is not being created, but that does not make use of wp-env.

It's always possible someone uses the wrong code as an example. In my opinion, we should create a template repository for someone to use when building out provider plugins (which I believe is what you suggesting would be created using this plugin as a building block) that has all of the preferred best practices at any given point in time.

[desrosj]

Hi! Do we still need this in light of @desrosj adding code coverage in the core PR (WordPress/wordpress-develop#10881 (comment))?

Yes, I think it's still important to add this. Even though there are aspects that seem positive, coverage should be tracked so it can be validated. It's also possible that this code remains in this repository for a bit longer.