Provide means of executing PHPUnit continuously over watched files in local environments

[iandunn]

Currently, PHPUnit tests have to be ran manually when working on a patch. To me, this feels tedious and means that I usually only run them when I reach a "milestone" in the process.

Automatically the executing of tests when files are saved would encourage test-driven development, make problems in the changes obvious immediately, and generally make the developer experience smoother.

There are a lot of solutions for devs to setup that automation independently (inotifywait, watchman, PhpStorm file watchers, etc) but they all require manual setup, and vary in quality.

We already have grunt watch available in our environment, so we can use it to provide a simple command like grunt watch:phpunit, making it easy to run the tests automatically.

By default, it could watch all PHP files and execute all tests in the phpunit:default task, but I think it'd be necessary to have options for specifying the watch target, test groups, and PHPUnit config (AJAX, Multisite, etc).

42282.diff​ is a very rough proof of concept that runs the community-events test group when changes are made inside wp-admin/includes.

If this is eventually committed, we should update ​the documentation to show examples of using the different options.

[iandunn]

Rough proof of concept

[netweb]

Thanks for the ticket @iandunn, I've thought about this same feature numerous times and I tend to walk away each time after I end up with an unmaintainable list attempting to map /src files to the /tests that cover those source files in /tests. It would be fantastic if we could do what is outlined in ​this post but unfortuanately WPs codbase isn't this orginized, yet ;)

Having a default watch task to run the entire PHP test suite I don't think is practicle for a watch task because of the time taken to finish a complete PHPUnit test run (~4 minutes on my mid 2014 Mac Book Pro here locally) as saving the same PHP file again once the Grunt job has spawned will try to fire off another watch task instance resulting in multiple PHPUnit task runs queued up that you'd need to hit CTRL-C to escape this.

I think we could add some watch tasks for specific components, e.g. grunt watch:multisite to watch the PHPUnit multisite group to then run grunt phpunit:multisite and a few more specific commonly developed PHPUNit groups such as formatting, feed, ajax, hooks, query, post to name a few from phpunit --list-groups, and per your patch community-events where each group would correspond with a Grunt task e.g grunt watch:community-events that executes phpunit --group community-events.

I've added an example of this in 42282.2.diff​, and we'd expand this by pairs of watch and phpunit Grunt tasks for each PHPUnit group that we chose to add, thinking about this further there's quite possibly a way to do this dynamically where any passed in PHPUnit group name would spawn the appropriate PHPUnit group task which would be much cleaner than adding numerous named instances.

[iandunn]

Replying to netweb:

end up with an unmaintainable list attempting to map /src files to the /tests that cover those source files in /tests.
[...] we could add some watch tasks for specific components, e.g. grunt watch:multisite
[...] there's quite possibly a way to do this dynamically where any passed in PHPUnit group name would spawn the appropriate PHPUnit group task which would be much cleaner than adding numerous named instances.

Yup, that's exactly what I was thinking. I don't think we need to try and automate it, at least for v1. We can just accept parameters for them, like grunt watch:phpunit --files=src/wp-admin/includes --group=community-events. (That's the wrong syntax for Grunt, but you get the idea).

We could automatically append /*.php if it's not in the input, just for convenience.

Having a default watch task to run the entire PHP test suite I don't think is practicle for a watch task because of the time taken to finish a complete PHPUnit test run

Good point. Maybe we could just require that files be passed in instead?

[iandunn]

Add support for arbitrary --files and --group

[iandunn]

@netweb, 42282.3.diff​ adds support for arbitrary --files and --group parameters, and defaults to just failing with a usage example.

So, you can now do things like:

• grunt watch:phpunit --files=wp-admin/includes --group=community-events
• grunt watch:phpunit --files=wp-includes/http.php --group=http

If that looks good to you, then I think it's a good enough v1 that I'd feel comfortable committing it. Then, we can iterate to add more functionality, like specifying different config files for Multisite/AJAX/etc, and testing specific files or classes, instead of just groups.

[iandunn]

Watches subfolders

[iandunn]

42282.4.diff​ watches subfolders of the passed folder

I think it'll be important to also add support for passing multiple folders/files soon, but that's currently blocked by #42308.

[netweb]

Awesome @iandunn, 42282.4.diff​ does what it says on the box

Let's go with this as is for v1 and per the comments I've added on #42308 we can iterate and v2 for 5.0

I've created a pull request ​https://github.com/ntwb/wordpress/pull/7 to test and confirm via Travis CI there will be no issues in committing this change ​https://travis-ci.org/ntwb/wordpress/builds/291991201

Weeee, JSHint errors ​https://travis-ci.org/ntwb/wordpress/jobs/291991202#L796, I've fixed these in 42282.5.diff​, the fixes in this ​commit, follow up CI job is ​https://travis-ci.org/ntwb/wordpress/builds/291998834

I've added the keyword needs-docs as we should update the handbook when this goes in:

• ​https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/#with-grunt

Everything looks good to me, moved to the 4.9 milestone and added commit :)

[westonruter]

We're almost to beta4. Let's add this after 4.9 lands.

[iandunn]

No longer automatically adds SOURCE_DIR so that tests folder can be watched

[iandunn]

@johnbillion or @jorbin, do either of you have time to give me a sanity check on this before I commit it?

[pento]

This seems fairly annoying to remember to run the right tests and watch the right files, particularly with how spaghetti-ish WordPress call graphs are.

I like how Jest and such have the tests in each module folder, but that's obviously not an option for Core's PHP.

I wonder if it's possible to have a test group header at the top of each source file, that tells the watch job what tests should be run when that file changes? Eg, the header of class-wp-community-events.php might look like:

/**
 * Administration: Community Events class.
 *
 * @package WordPress
 * @subpackage Administration
 * @since 4.8.0
 *
 * @test-groups community-events
 */

Still not ideal, and would require a significant up-front effort. But maybe easier to work with long term.

[iandunn]

Replying to pento:

This seems fairly annoying to remember to run the right tests and watch the right files

Agreed. I just tested the feasibility of watching the entire repo, rather than specifying --files. To my surprise, it doesn't seem to introduce a noticeable performance issue. That was on a 2013 MacBook Pro (2.7 GHz i7, 16GB RAM).

So, I think we can make the --files parameter optional, or just remove it entirely and always watch the whole repo. I'm leaning towards just removing it, since we can always add it back if we discover a need.

[...] have a test group header at the top of each source file, that tells the watch job what tests should be run when that file changes

I really like that idea, and would like to explore it more. If it works, that would definitely be more convenient than having to manually specify the group. Combined with removing --files (or making it optional), I think that would solve the problem.

I think we'd still need to be able to specify --group manually, though. There may be situations where a file is associated with multiple groups; the user may only want to target a specific one, rather than waiting on all of them to run each time.

Still not ideal, and would require a significant up-front effort. But maybe easier to work with long term.

Were you thinking of the group header as a blocker for v1, or a future iteration? To me, it seems like the latter is more appropriate, since introducing that would be a simple addition to what's already in 42282.6.diff​, rather than requiring any refactoring, or forcing users to learn new parameters, etc.

I'm particularly concerned about it being a blocker, since I don't think I'll have time to do all the up-front work in the near/mid-future, and I've already found 42282.6.diff​ to be very useful when working on other patches. (So much so that I've been manually merging it into every branch I work on, then removing it from the diffs I generate.)

[pento]

I think removing the --files option is a good move, watching everything and running a specified group definitely makes it easier to manage. Let's call that the v1, and see how it's used by folks.

[iandunn]

In 42760:

Build/Test Tools: Add watch:phpunit task.

This allow PHPUnit test groups to run automatically when files are changed, rather than having to be ran manually throughout the development process. This creates a smoother developer experience, and a tighter feedback loop.

Props iandunn, netweb, pento
Fixes #42282

[soulseekah]

@iandunn, what a great idea! Do you think we should pass on a --filter as well? To run just one specific test etc. Would be pretty cool to get instant feedback on changed files and run just one test instead of a group.

Although, it's always possible to just make up a temporary group.

Either way it would be possible to have sub-second feedback from tests with #43432 implemented and being able to run as little as just one test.

Great stuff!

[iandunn]

Replying to soulseekah:

Do you think we should pass on a --filter as well?

I like that idea, or maybe we could even support arbitrary PHPUnit args instead of a whitelist? I haven't checked if that would have some potential conflicts with Grunt arguments, though.