search

WP-API / OAuth2

Connect applications to your WordPress site without ever giving away your password.
GNU General Public License v2.0
171 stars 41 forks source link

Testing Infrastructure #37

Closed tfrommen closed 7 years ago

tfrommen commented 7 years ago

This pull request resolves issue #25.

What's Included in This Pull Request

The example test class absolutely doesn't have to stay that way (well, at least the second method). I just tried to pick something not too complex that allows to leverage all three testing/mocking tools.

The code requires PHP version 5.4 or higher.

I didn't get any answers to my question about the targeted PHP version, so I chose what we are currently using - and what is required by all of the testing/mocking methods - PHP 5.4.

Happy to get any feedback.

tfrommen commented 7 years ago

Any opinions on this?

rmccue commented 7 years ago

@tfrommen Sorry for the delay in getting to this, been super busy!

I'd prefer to avoid using anything outside of WordPress' unit test framework, as this plugin is aiming for eventual core integration. I'd rather we do integration tests than proper unit tests generally speaking.

Also, given that PHPUnit includes mocking, not sure why we'd need Mockery?

tfrommen commented 7 years ago

Hi @rmccue,

I didn't (mean to) say that we need Mockery. It's more a combination of different things that I included it: Mockery is more powerful than PHPUnit's built-in mocking capabilities, it is required by Brain Monkey (which I included here, too), so I am used to using it, and, personally, I like it's API much better than the PHPUnit mock builder.

If the plan is to only use what has made it into the WordPress ~unit~ test spheres, and nothing else, then OK. This means we'd end up with (unneccesarily) slow pseudo-unit tests, or no unit tests at all, and WordPress-specific integration/system tests.

If the plan is to not write any real unit tests, then also OK. No need for anything of what I did and used here (besides PHPUnit).

However, this could also be seen as an opportunity to start writing actual unit tests - with the help of some existing library such as Brain Monkey, or something similar, created from scratch, under the WordPress.org umbrella.

There are lots of third-party libraries in WordPress Core, and even more in the testing and building environments. For me, it just makes sense to also have a WordPress-specific unit testing helper library, and thus allow people to create actual, fast-running unit tests.

Also, while I both understand and really welcome the goal to have OAuth2 included in WordPress Core - some day - I would not base its whole development on the current state of WordPress (and its environment). I mean, the plugin currently already makes use of PHP 5.3+ namespaces, and PHP 5.4+ short array syntax - without knowing when WordPress will actually adopt (i.e., require) PHP 5.4 or higher.

Regarding the pull request, I had to start somewhere, and chose what I am used to, and what I thought would make sense here as well. However, I'm fine with whatever you think is best, in the end. All in all, this is just a project I had an interest (to contribute) in. 🙂

rmccue commented 7 years ago

I don't disagree that it'd be nice to have proper unit tests, but integration tests are going to be more useful for 99% of the cases that we want to test, so we really want to focus on them.

Thanks for the PR, but I'll close this out for now and add in just basic PHPUnit instead.

I would not base its whole development on the current state of WordPress (and its environment). I mean, the plugin currently already makes use of PHP 5.3+ namespaces, and PHP 5.4+ short array syntax - without knowing when WordPress will actually adopt (i.e., require) PHP 5.4 or higher.

On that point, I'm making a bet here that we'll have moved to 5.4 by the time that we start discussing any sort of merge. Alternatively, we can look at it as a 5.4+ feature in core (it's going to require HTTPS, so requiring 5.4+ isn't a huge stretch).

That said, if needed, we can always compile it back to 5.2 reasonably easily, since it's still mostly conceptually the same. However, the concepts involved in the unit test framework are harder to change later if we need to. :)