Opened 11 years ago
Closed 11 years ago
#4889 closed enhancement (fixed)
Move unit test suite into BP core
Reported by: | boonebgorges | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 1.8 | Priority: | high |
Severity: | major | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch needs-testing dev-feedback |
Cc: | fanquake@… |
Description
Thanks to DJPaul's outstanding work, we have a working test suite, which currently lives at https://github.com/buddypress/BuddyPress-Unit-Tests. Over the last few months, we've written a number of tests covering various parts of the codebase, and I'm confident that in the process we've built up the suite sufficiently that we can be confident of it (BP is being bootstrapped and installed correctly, our helper methods like go_to() are working as expected, the data factories are running nicely, etc).
At this time, I'd like to propose that we move the test suite to BuddyPress core. Please see the attached patch.
Briefly, I've made the following changes:
- BP tests now use WP's automated test suite as a basis. DJPaul's original build was a fork of WP's suite. With my patch, WP's suite is used but is untouched.
- I also abstracted some key parts of the BP setup routine (installation, the bootstrap, loading the BP testcase and factories) so that they can easily be invoked from other plugins. This makes it possible to daisy-chain our unit testing tools from another plugin or a larger project. More on this below.
To test the patch:
- Apply it to your local trunk checkout
- Make sure you have the WP tests suite set up on your local installation http://make.wordpress.org/core/handbook/automated-testing/
- (Optional but recommended) Put the following in your ~/.bashrc and then source it:
export WP_TESTS_DIR="/home/bgorges/sites/wp-tests"
. (Obviously you'll have to replace the path. If you don't do this, you'll have to define it inline when you invoke PHPUnit.) - cd to your
buddypress
plugin directory - Type
phpunit
These changes introduce several very significant advantages over the current setup.
- Ease of use. Because we use the WP test suite out of the box, anyone who has the WP test suite set up in their environment can immediately run the BP tests by going to the BP plugin directory and typing
phpunit
.
- Usable by external BP projects (themes, plugins, entire website projects). Say you're building a BuddyPress-dependent plugin, and you want to write unit tests. You'll want two things: that BP is up and running when your tests run (so you'll have access to BP settings, BP functions, BP database tables, etc), and that you have access to BP's testing tools (BP_UnitTestCase, BP's data factories). With the current setup, you can sorta do this, by using the BP test runner *instead of* the WP test runner. But this is somewhat clunky, because (a) it means using the forked version of the WP test suite, and (b) if your plugin or project has *multiple* dependencies, this won't work - no dependency daisy-chaining. My patch changes this. You can declare BP dependency in your own plugin tests by including two files: buddypress/tests/includes/loader.php at 'muplugins_loaded' (this installs and bootstraps BP), and buddypress/tests/includes/testcase.php just before you load your own tests (this gives you access to BP_UnitTestCase, etc). Here's a real example from the BuddyPress Docs tests bootstrap:
// Define the BP_TESTS_DIR for convenience if ( ! defined( 'BP_TESTS_DIR' ) ) { define( 'BP_TESTS_DIR', dirname( __FILE__ ) . '/../../buddypress/tests' ); } // Because we're *dependent* on BP, only load the tests if the core tests are available // You could modify this to load based on whatever condition you want if ( file_exists( BP_TESTS_DIR . '/bootstrap.php' ) ) : // Gives access to tests_add_filter() require_once getenv( 'WP_TESTS_DIR' ) . '/includes/functions.php'; // Bootstrap BP and BP Docs at muplugins_loaded function _bootstrap_bpdocs() { // Make sure BP is installed and loaded first require BP_TESTS_DIR . '/includes/loader.php'; // Then load BP Docs require dirname( __FILE__ ) . '/../loader.php'; } tests_add_filter( 'muplugins_loaded', '_bootstrap_bpdocs' ); // Bootstrap the core tests require getenv( 'WP_TESTS_DIR' ) . '/includes/bootstrap.php'; // Load the BP test files require BP_TESTS_DIR . '/includes/testcase.php'; // include our custom testcase require( dirname(__FILE__) . '/bp-docs-testcase.php' ); endif;
I'm hopeful that the method that I've set up in the patch will become not just the standard for BP plugins, but for any plugin that might need its tests to be invoked from another plugin or project.
- Better conforms to emerging standards of WP plugin test setup. More and more plugins are setting up tests using this exact format, especially now that wp-cli (and
wp scaffold plugin-tests
) is becoming more popular.
- Higher visibility for the tests. By putting the plugins into BP, we ensure that developers checking out trunk will see them, which greatly increases the likelihood that they'll *use* them (or even write some of their own).
- Better integration into core dev workflow. Putting things into Trac means that we can have a common revision history, and the core devs can reference tests in commit messages, etc.
Feedback very welcome.
Attachments (1)
Change History (7)
#2
@
11 years ago
Thanks for putting in the time and effort to make this happen.
:-D
my gut reaction is that bundling the test suite in BuddyPress core is an unnecessary burden to place on the casual users that won't benefit from the tests being packaged in the zip. Can we accomplish the same end result without it? If we need to make changes to core to help the tests boot properly, we should make those changes first.
I have a couple thoughts about this. First, we don't have to include this in the distribution (wp.org) version of WordPress. We could just remove it before committing to wp.org svn (or use an svn ignore or something like that). That'd avoid 99% of the "casual user" issues.
Second, I know that WP has a separate repo. But there are reasons to think that this is not the best way forward for a plugin. (a) Keeping the code in a separate repo makes it more complicated for devs to run the tests. (b) It makes it even *more* complicated for plugin authors to make their own tests dependent on BP tests, because they have to both make sure that the BP test suite is loaded, and they can't be certain that the paths will be the same across installations. Keeping things in buddypress/tests makes it totally predictable and reliable. (c) WordPress has many times more devs than BP. If we relegate our tests to a separate repo so that it has its own, smaller audience, that audience will be very small indeed :)
To put this another way: It (arguably) makes sense for WP's test runner to exist as a separate application (from WordPress), because WP is the base platform. BP runs on top of WP. So, our test suite should run on top of the WP test runner application. This is easy to accomplish if we simply put the tests inside of BuddyPress. If we reimagine the BP test suite as a separate application, then it makes the process of running BP tests alongside of WP tests much more complicated than it needs to be.
So, I think that there are very big advantages to putting the tests in the BP repo. And the biggest disadvantage - that it "clutters" the repo - can be mitigated by leaving the tests folder out of the wp.org release version.
Whaddya think?
#4
@
11 years ago
The good news: Tested Boone's patch on BP trunk with PHPUnit and WP's test suite and it works.
The bad news: One of the BP unit tests failed due to some hardcoded domain checks!
https://github.com/buddypress/BuddyPress-Unit-Tests/pull/1
Who will unit test the unit tests? :)
About adding the 'tests' subdirectory, I'm in favor of either method boonebgorges proposed.
#5
@
11 years ago
(In [6905]) Moves automated testing suite into BuddyPress core
The new, improved BuddyPress testing suite boasts the following features:
- It uses a vanilla copy of the WordPress test suite as a basis, for maximum portability and future-compatibility
- Allows for BP-dependent plugins to write their own tests that use the BP test libraries (like our data factories)
- Conforms to current best practices for building WordPress plugin tests
The existing testcases themselves will be ported over in the next changeset, so
as to keep this one reasonably small.
See the Codex page (forthcoming) on how to set up and run the tests, how to add
to them, and how to write BP-dependent tests in your own plugin.
See #4889
Props djpaul, boonebgorges
First, this is super awesome. Having a fully working test suite is a huge, huge win. Thanks for putting in the time and effort to make this happen.
Next, my gut reaction is that bundling the test suite in BuddyPress core is an unnecessary burden to place on the casual users that won't benefit from the tests being packaged in the zip. Can we accomplish the same end result without it? If we need to make changes to core to help the tests boot properly, we should make those changes first.
Lastly, we should post a request on make/systems to get multiple repositories setup here (ala http://core.trac.wordpress.org/browser) so we can host them here, and setup permissions to that repo for it's own audience of contributors. This seems to have worked will for WP core so far.