Add WP PHPUnit #366

aaemnnosttv · 2018-07-10T22:41:45Z

This is a first draft of a working implementation of what it would look like to integrate WP PHPUnit into Bedrock.

Documentation is not included yet and still needs to be added, but I wanted to get a first round of feedback before writing it.

There are a few things that are somewhat of stylistic preferences, like the exceptions made to the PHPCS rules for tests and the formatting of the example test. This is somewhat bending the rules of the existing ruleset, but I believe it's a common test-specific style.

Apart from that, very little has really changed, as almost everything here are additions.

I look forward to hearing anyone's feedback on the proposed changes.

Resolves #365

phpunit 5 for compatibility with php 5.6

this file is for local customizations, phpunit.xml.dist is shipped

this constant cannot be defined in the testing environment as a matter of timing.

using php 5.6 for compatibility with all versions tested on travis CI

austinpray

TLDR bedrock does an excellent job of being a neutral party in that pretty much everything that it does is 100% the uncontroversial/standard approach. This PR as-is makes some statements bedrock should not be making.

These are two statements that I am okay with bedrock supporting and that most people will stand by:

PHPUnit is widely accepted as the defacto PHP unit test framework

Composer is generally how people manage their PHP dependencies

The following is something bedrock should not suggest:

PHPUnit + wp-phpunit is the best way to test your wordpress installation.

Bedrock should not endorse any particular testing paradigm whether it be the full wordpress.org testing suite, online db unit/e2e tests with https://github.com/wp-phpunit/wp-phpunit, offline unit tests with https://github.com/10up/wp_mock or any other solution out there.

CC @johnpbloch any thoughts here?

austinpray · 2018-07-11T00:59:36Z

composer.json

-    "preferred-install": "dist"
+    "preferred-install": "dist",
+    "platform": {
+      "php": "5.6.32"


this should be php 7 or omitted. Omit this if possible.

Agreed. This has to do with PHPUnit more than anything. The reason this is here is so that all dependencies installed will be compatible with PHP 5.6 when it runs on Travis. This is because, even though PHPUnit 5.x is required, because my local version of PHP is 7.2, some of PHPUnit's dependencies will be installed with versions which are not compatible with 5.6.

The alternatives to doing this are:

Requiring in the offending dependencies and locking them to a version which is compatible with PHP 5.6. Not a good option as those dependencies could always change and this would be a pain to manage.

Running composer update on Travis instead of composer install. I don't like this as much either as its less predictable

Always run composer update with PHP 5.6.

This is a very useful configuration would be something that could be added to the docs, as most users probably aren't using Bedrock in a multi-version environment. Users can (and probably should be) set this to their own value based on their target environment.

With PHP 5.6 and 7.0 reaching end of life at the end of the year perhaps this won't be necessary anymore.

austinpray · 2018-07-11T01:03:58Z

.travis.yml

 before_install:
  - phpenv config-rm xdebug.ini || true
  - composer self-update

 install:
  - composer install -o --prefer-dist --no-interaction

+before_script:
+  - mysql -u root -e "GRANT ALL PRIVILEGES ON ${DB_NAME}.* TO ${DB_USER} IDENTIFIED BY '${DB_PASSWORD}';"
+  - mysql -u root -e "CREATE DATABASE ${DB_NAME};"


Thinking maybe travis db testing integration should be a blog post or something separate. There are many ways to skin this cat (docker, external dbs, etc) so best to leave up to the user's discretion. We shouldn't even need a db for unit tests, that is for e2e tests.

The database is required by WordPress' test library. I agree that it isn't setup for pure unit testing, but I also think that this kind of higher level testing is more valuable at the application level than pure isolated unit tests.

austinpray · 2018-07-11T01:04:12Z

composer.json

-      "phpcs"
-    ]
+    "test": "phpunit",
+    "lint": "phpcs"


this is a good change

austinpray · 2018-07-11T01:05:17Z

config/application.php

@@ -72,7 +72,6 @@
 * Custom Settings
 */
 define('AUTOMATIC_UPDATER_DISABLED', true);
-define('DISABLE_WP_CRON', env('DISABLE_WP_CRON') ?: false);


what is going on here?

This is not completely removed, but the define is moved into the environment configuration files. This is because this constant cannot be defined in the testing environment, as it will define it itself without checking if it has already been defined or not.

The alternative to moving it to the configuration files (and not to testing.php) is to wrap this with a conditional to only define it if WP_ENV !== 'testing' which is a little less clean IMO.

austinpray · 2018-07-11T01:09:22Z

composer.json

-    "squizlabs/php_codesniffer": "^3.0.2"
+    "phpunit/phpunit": "^5.0",
+    "squizlabs/php_codesniffer": "^3.0.2",
+    "wp-phpunit/wp-phpunit": "4.9.7"


Honestly don't really want to be married to wp-phpunit. We should leave this open ended and just give people raw phpunit and let people choose whatever approach they want. eg https://github.com/10up/wp_mock type approach or full on unit + e2e tests and such.

@johnpbloch what do you think?

That's a fair statement, and I'm probably biased as the package creator, but wp-phpunit is more practical for the average user as it will be what most people are familiar with IMO. I don't think it would be a problem for anyone who is familiar with other testing frameworks to replace and use whatever they want.

I think reducing the setup to bare PHPUnit would be less valuable for more people, and wouldn't really make a difference to those who are want to use something else. It's just a boilerplate. No one is married to anything.

Of course bare PHPUnit is better than nothing, I think we agree on that.

austinpray · 2018-07-11T01:33:39Z

Also wow sorry meant to include this in initial review but: thank you so much for putting this together! This is a great start 👍.

johnpbloch · 2018-07-11T01:41:20Z

I haven't taken a look at the code, just jumping in to follow up to @austinpray's comment. I think insofar as the PR improves the compatibility between WP PHPUnit and Bedrock, this is a great initiative. I do tend to agree with the idea of keeping Bedrock testing framework-agnostic. Maybe it would be worth putting some of the more popular testing frameworks (WP Mock, WP PHPUnit, BrainMonkey, etc.) in the suggest block of composer.json with descriptions of their intended use? Just a thought.

swalkinshaw · 2018-07-11T01:58:35Z

I mostly agree with @austinpray. I kind of forgot that the WP test unit library is mostly designed for testing WP core itself (for core developers). It's pretty heavy-handed for normal application/project testing.

So having the boilerplate required to make that easier with Bedrock is still useful 👍

aaemnnosttv · 2018-07-11T09:37:03Z

Thank you everyone for your feedback! It seems like everyone is on board with the idea that adding some boilerplate for testing is a step in the right direction but maybe it shouldn't be opinionated in choosing a framework to include boilerplate for.

PHPUnit + wp-phpunit is the best way to test your wordpress installation.

I don't particularly agree with the idea that including a library is making such a statement, nor would I make that claim myself.

The library is really just the WordPress core library that supports running PHPUnit in a WordPress environment. It's just been made easily installable via Composer. I would say that this library is the one that most people are going to be familiar with in a WordPress context (WP CLI scaffolds it, and most articles/resources on testing in WordPress are based on it).

Of course others are available, but I think if Bedrock were to include a more complete testing boilerplate (as proposed) that it offers more value than if it was more barebones. By no means does it lock anyone in to using it or impose anything that isn't easily changeable which I think is the important aspect of the principal of remaining framework agnostic.

With that said, I fully respect the teams decision to keep Bedrock framework agnostic if that's the consensus. In my opinion, including the "official" core library for testing is still relatively framework agnostic because it's part of WordPress core.

Either way, I'm still interested in helping with this initiative in whatever way I can.

99linesofcode · 2019-07-28T22:43:42Z

This has been stale for quite some time and there has been much discussion on the subject prior to this PR. What can I do to help this along as I would love to be able to write unit tests for our suite of plugins.

chrillep · 2020-07-30T07:54:12Z

@aaemnnosttv maybe you could make an example project of this instead ?
let's say wp-phpunit/example-bedrock-project ?

Thanks to the work done in roots/bedrock#366 .

aaemnnosttv added 14 commits July 10, 2018 23:22

add phpunit + wp-phpunit

66c1be6

phpunit 5 for compatibility with php 5.6

update composer test to phpunit, add composer lint

0601a4a

autoload-dev tests namespace with psr-4

907c45f

update lock

8f187a4

add phpunit.xml.dist

77a7fb3

add bootstrap.php and wp-config.php

1e3eff7

add base TestCase and ExampleTest

638a721

add exceptions for common test styles

26fca37

ignore phpunit.xml

a1d8f35

this file is for local customizations, phpunit.xml.dist is shipped

add testing environment config file

ebddae3

move DISABLE_WP_CRON config to environment configs

a935ed2

this constant cannot be defined in the testing environment as a matter of timing.

update travis config with env vars + db creation

e7e163d

configure target platform php version

b139318

using php 5.6 for compatibility with all versions tested on travis CI

set absolute path to tests config

0d76595

austinpray suggested changes Jul 11, 2018

View reviewed changes

austinpray mentioned this pull request Aug 11, 2018

Feature Request: Add first-class support for PHPUnit #365

Open

5 tasks

swalkinshaw mentioned this pull request May 28, 2021

Move phpcs to lint command in composer.json #596

Open

takotakot added a commit to takotakot/show-kanren-posts that referenced this pull request Apr 7, 2023

feat: Add configuration for running tests

24167f7

Thanks to the work done in roots/bedrock#366 .

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add WP PHPUnit #366

Add WP PHPUnit #366

aaemnnosttv commented Jul 10, 2018

austinpray left a comment •

edited

Loading

austinpray Jul 11, 2018

aaemnnosttv Jul 11, 2018

austinpray Jul 11, 2018

aaemnnosttv Jul 11, 2018

austinpray Jul 11, 2018

austinpray Jul 11, 2018

aaemnnosttv Jul 11, 2018

austinpray Jul 11, 2018

aaemnnosttv Jul 11, 2018

austinpray commented Jul 11, 2018

johnpbloch commented Jul 11, 2018

swalkinshaw commented Jul 11, 2018

aaemnnosttv commented Jul 11, 2018

99linesofcode commented Jul 28, 2019 •

edited

Loading

chrillep commented Jul 30, 2020 •

edited

Loading

Add WP PHPUnit #366

Are you sure you want to change the base?

Add WP PHPUnit #366

Conversation

aaemnnosttv commented Jul 10, 2018

austinpray left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

austinpray commented Jul 11, 2018

johnpbloch commented Jul 11, 2018

swalkinshaw commented Jul 11, 2018

aaemnnosttv commented Jul 11, 2018

99linesofcode commented Jul 28, 2019 • edited Loading

chrillep commented Jul 30, 2020 • edited Loading

austinpray left a comment •

edited

Loading

99linesofcode commented Jul 28, 2019 •

edited

Loading

chrillep commented Jul 30, 2020 •

edited

Loading