-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testing Infrastructure #37
Conversation
Any opinions on this? |
@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? |
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 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. 🙂 |
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.
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. :) |
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.