Improve performance of Unit Tests for 4.4

[jorbin]

This is a general catchall ticket for unit test performance improvements in 4.4. There are many things we can do to help the unit tests perform better and also have less of an impact on the wordpress.org infrastructure.

[jorbin]

One of the first changes we should make is to stop hitting the svn repos in tests. Three tests in the http base test set (and potentially others) test by pinging urls that are in the svn repo. We should move those files to somewhere else to not hammer the svn repo.

[jorbin]

@samuelsidler is going to work on finding a new place for those files.

[jorbin]

The three files right now are:

./http/base.php:  
183:		$url = 'http://unit-tests.svn.wordpress.org/trunk/data/images/2004-07-22-DSC_0007.jpg'; // we'll test against a file in the unit test data 
  
./http/base.php:  
204:		$url = 'http://unit-tests.svn.wordpress.org/trunk/data/images/2004-07-22-DSC_0007.jpg'; // we'll test against a file in the unit test data 
  
./http/base.php:  
226:		$url = 'http://develop.svn.wordpress.org/trunk/tests/phpunit/data/images/2004-07-22-DSC_0007.jpg'; 

[SergeyBiryukov]

Replying to jorbin:

The three files right now are:

This appears to be a single random image. Could we just switch it to another random image from the CDN, e.g. https://s.w.org/screenshots/3.9/dashboard.png?

[jorbin]

In 34568:

Stop hitting SVN for http tests

The automated tests can fail due to svn. Change the tests to use a WordPress CDN image.

see #33968

[jorbin]

In 34600:

Use an http URL rather then an https URL for tests

Travis doesn't properly support SSL on older versions of PHP. Since we don't test for SSL support in all of the tests, this can cause these tests to fail. This changes a URL introduced in [34568]

See #33968

[jorbin]

In 34656:

Exclude external HTTP tests from multisite run

In [30298] the unit tests default confirguration was modified to exclude external-http tests. This change was never migrated to the multisite XML configuration. The external HTTP code doesn't follow different logic in multisite, so the logic to exclude the tests then ( The external-http tests are very slow, and Wp_Http functionality is fairly isolated ) holds true here as well.

See #33968

[jorbin]

Two tests that have intermittent failures and could use a little TLC are: Tests_Media::test_add_image_size and Tests_Image_Intermediate_Size::test_get_intermediate_sizes_by_array_zero_width

See ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/82641433

[jorbin]

In 34694:

Cleanup image size in unit tests

This helps prevent test leakage that can cause Tests_Media::test_add_image_size to break.

See #33968

[jorbin]

Intermittant failure: Tests_Ajax_DeleteComment::test_ajax_comment_trash_actions_as_administrator

See ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/82849174

[DrewAPicture]

Intermittent failure on 5.5.9: Tests_Theme::test_get_themes_contents. Haven't seen this in Travis, but I see it almost every time I run the suite on local.

[DrewAPicture]

In 34802:

Tests: Introduce WP_UnitTestCase::reset_permalinks(), an attempt to DRY up logic for resetting and restoring default permalinks on setUp() and tearDown().

See #33968.

[DrewAPicture]

In 34803:

Tests: Move the global permalinks reset routine to only fire when running core tests.

Should fix intermittent mysqli response errors in the AJAX tests.

See #33968.

[DrewAPicture]

In 34807:

Tests: Last try: It's redundant to reset permalinks on tearDown() if we're already doing it on every setUp().

Removes the restoration logic, which leveraged a static property initialized with get_option().

See #33968.

[DrewAPicture]

In 34810:

Tests: Permalink Structures Phase II: DRY up logic for setting permalink structures in test methods.

Renames reset_permalinks() to set_permalink_structure() (mimicking $wp_rewrite->set_permalink_structure()) and allows it to accept an optional permalink structure. In this way, we can double dip using it to both set and reset the permalink structure from anywhere.

Removes alot of duplicated code from tests.

See #33968.

[DrewAPicture]

Following [34802], [34803], [34807], and [34810], setting permalink structures in tests is now a lot simpler.

For generic cases, all you need to do is set it and forget it, e.g:

$this->set_permalink_structure( ‘/%postname%/‘ );

Doesn’t cover anything other than structures though. If you’re doing stuff with new rules, etc. continue to use the $wp_rewrite global as necessary. If you’re registering taxonomies, you’re still going to want to run flush_rewrite_rules() afterward, regardless of whether you set the structure earlier and the rules were flushed then.

Side note: I avoided changing the canonical tests because I found them to be fragile. Maybe followup on that later.

[netweb]

The PHPUnit ajax and external-http group of tests are not run as part of the PHP 5.2 Travis CI tests:

i.e. Both the grunt sub tasks of part of grunt phpunit task:

• phpunit -c phpunit.xml.dist --group ajax
• phpunit -c phpunit.xml.dist --group external-http

Example via Travis CI job ~2 months ago: ​https://travis-ci.org/aaronjorbin/develop.wordpress/builds/72667225

Running "phpunit:ajax" (phpunit) task
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 3.6.12 by Sebastian Bergmann.
Configuration read from /home/travis/build/aaronjorbin/develop.wordpress/phpunit.xml.dist
Time: 1 second, Memory: 116.00Mb
OK (0 tests, 0 assertions)

And

Running "phpunit:external-http" (phpunit) task
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
PHPUnit 3.6.12 by Sebastian Bergmann.
Configuration read from /home/travis/build/aaronjorbin/develop.wordpress/phpunit.xml.dist
Time: 2 seconds, Memory: 116.00Mb
OK (0 tests, 0 assertions)

Where both should respectively be:

Running "phpunit:ajax" (phpunit) task
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 4.5.0 by Sebastian Bergmann and contributors.
Configuration read from /home/travis/build/aaronjorbin/develop.wordpress/phpunit.xml.dist
................................................................. 65 / 70 ( 92%)
.....
Time: 1.9 minutes, Memory: 108.50Mb
OK (70 tests, 271 assertions)

And

Running "phpunit:external-http" (phpunit) task
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
PHPUnit 4.5.0 by Sebastian Bergmann and contributors.
Configuration read from /home/travis/build/aaronjorbin/develop.wordpress/phpunit.xml.dist
..................................................
Time: 25.5 seconds, Memory: 102.50Mb
OK (50 tests, 112 assertions)

Sadly PHPUnit error codes do not detect this: Done, without errors.

I've tried to find unearth some older build/job URL's but due to the way Travis CI UI works this is particularly difficult, I'd hoped to get a build/job before r30298 but haven't been able to, the closest I've got is ​#951 from ~10 months ago where the ajax and external-http are not run.

[netweb]

Intermittant failure: Tests_HTTP_curl::test_request_limited_size
See ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/83814943

[jorbin]

In 34848:

HTTP timeouts should cause some tests to be skipped, not failed

A number of the HTTP external tests can inconstantly fail. As the HTTP API is one that doesn't change often, this failure creates noise. With the goal of increasing the signal from the unit tests, these tests are now skipped if they timeout. A notice is added when running the external http tests so that the developer knows what skipped tests may mean here.

See #33968

[jorbin]

In 34849:

Fix whitespace issues introduced in [34848]

Bad Jorbin.

See #33968

[DrewAPicture]

In 34870:

Tests: Update Tests_Rewrite_AddRewriteRule->setUp() to use the new set_permalink_structure() helper.

See #33968.

[SergeyBiryukov]

In 34872:

Fix typo in [34848].

See #33968.

[jorbin]

In 34874:

Adjust detection of stream timeouts in maybe skip tests for https tests

This is a follow up to [34848].

See #33968.

[jorbin]

In 34879:

Add message with status info to temperamental assertion.

The ajax delete comment tests are intermittently failing. Many of the assertions make it hard to tell why they are failing. This adds a message to one of those assertions that contains some info on what is being asserted with the goal that it helps developers understand why the failure is failing.

See #33968

[DrewAPicture]

In 34983:

Tests: Add basic DocBlocks for four helper methods in general/template.php used to assist testing the Site Icon feature.

All four helpers were introduced in the feature merge for 4.3.

See #33968.

[ocean90]

Replying to jorbin:

Intermittant failure: Tests_Ajax_DeleteComment::test_ajax_comment_trash_actions_as_administrator

See ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/82849174

Also Tests_Ajax_DeleteComment::test_ajax_trash_comment_no_id

See ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/84939133

[boonebgorges]

In 35136:

In WP_UnitTestCase, only flush rewrite rules when they're set.

See [34810]. See #33968.

[boonebgorges]

In 35137:

Create fewer fixtures in some XML-RPC tests.

See #30017, #33968.

[SergeyBiryukov]

Some performance breakdown on individual tests: ​https://gist.github.com/ocean90/9ef0f64955ef9df2304f.

Per ​https://wordpress.slack.com/archives/core/p1444771474003408, XML-RPC tests should probably be excluded from normal runs and moved to a separate group.

phpunit --exclude-group xmlrpc doesn't work as expected, it causes a bunch of errors and fails with a "Database is dead" message, see #34185.

33968.patch​, however, seems to work if we want to do that.

[boonebgorges]

The XML-RPC tests don't run all that slowly for me - certainly not on the same level as external-http and ajax. IMO, we should be wary of skipping any group of tests on normal runs, and only do it when the performance benefits are very large.

[helen]

I accept that there is almost definitely something going wrong with my setup which I cannot identify, but here are my SpeedTrap results for your tears: ​https://gist.github.com/helenhousandi/b6bae6c1a89b53842da8

I'm going to exclude the xmlrpc group from my own runs, as I would like to actually be able to run unit tests before commit without losing my sanity.

[boonebgorges]

In 35154:

Create fewer fixtures in XML-RPC getComments tests.

See #30017, #33968.

[boonebgorges]

In 35162:

Create fewer fixtures in some tests.

See #30017, #33968.

[boonebgorges]

In 35163:

Share fixtures in Tests_Get_Archives.

See #30017, #33968.

[boonebgorges]

In 35164:

Share fixtures in Tests_Admin_includesListTable tests.

See #30017, #33968.

[boonebgorges]

In 35165:

Fix incorrect variable names from [35164].

Cool story - the tests appeared to pass with the typos.

See #30017, #33968.

[wonderboymusic]

In 35171:

Unit Tests: wrestle performance out of Tests_Auth by cloning the same user for a majority of the tests.

See #30017, #33968.

[wonderboymusic]

In 35172:

Unit Tests: move ->test_readme() out of Tests_Basic and into Tests_External_HTTP_Basic in tests/external-http/.

I intend to move other wp_remote_get() tests into similar classes.

See #30017, #33968.

[wonderboymusic]

In 35173:

Unit Tests: reuse fixtures in Tests_Comment.

See #30017, #33968.

[wonderboymusic]

In 35174:

Unit Tests: call commit_transaction() in Tests_Auth set up.

See #30017, #33968.

[wonderboymusic]

In 35175:

Unit Tests: Tests_Auth needs a tearDownAfterClass impl to avoid spillage.

See #30017, #33968.

[wonderboymusic]

In 35176:

Unit Tests: Tests_Comment needs a tearDownAfterClass impl to avoid spillage.

See #30017, #33968.

[wonderboymusic]

In 35177:

Unit Tests: Tests_Canonical doesn't need to call wp_set_current_user() or implement tearDown because its grandparent calls wp_set_current_user( 0 ) in tearDown().

See #30017, #33968.

[wonderboymusic]

In 35178:

Unit Tests: move some oEmbed tests that can trigger HTTP calls to Tests_External_HTTP_OEmbed.

See #30017, #33968.

[wonderboymusic]

In 35179:

Unit Tests: make a fixture in Tests_Media to represent the large image, instead of creating it 10 times.

See #30017, #33968.

[wonderboymusic]

In 35181:

Unit Tests: in Tests_Media::test_wp_get_attachment_image_srcset_array_no_width(), just toggle metadata, instead of creating a new attachment. Shaves 75ms off the test.

See #30017, #33968.

[wonderboymusic]

In 35183:

Unit Tests: in Tests_Post, create fixtures for users.

See #30017, #33968.

[wonderboymusic]

In 35185:

Unit Tests: in Tests_Term, create fixtures for posts.

See #30017, #33968.

[wonderboymusic]

In 35186:

Unit Tests: implement setUpBeforeClass() and tearDownAfterClass() on WP_UnitTestCase. Use late static binding (plus a gross fallback for PHP 5.2) to check if wpSetUpBeforeClass() or wpTearDownAfterClass() exist on the called class, and then call it and pass a static WP_UnitTest_Factory instance via Dependency Injection, if it exists.

This makes it way easier to add fixtures, and tear them down, without needing to instantiate WP_UnitTest_Factory in every class - removes the need to call commit_transaction() in each individual class.

See #30017, #33968.

[wonderboymusic]

In 35188:

Unit Tests: create more fixtures for Tests_User. When using a factory to create ad hoc users, use the inherited static prop $static_factory instead of the instance prop, $factory. If 2 factories are used out of sync, the generator sequences diverge and dupes can be created, causing an untold number of unforeseen errors. Yay.

See #30017, #33968.

[wonderboymusic]

In 35191:

Unit Tests: after [35186], "upgrade" the Canonical fixtures.

See #30017, #33968.

[wonderboymusic]

In 35192:

Unit Tests: in Tests_Comment_Query::test_get_comments_for_post(), create fewer comments (5, instead of 10).

See #30017, #33968.

[wonderboymusic]

In 35193:

Unit Tests: upgrade the fixtures in Tests_Post_Revisions.

See #30017, #33968.

[wonderboymusic]

In 35194:

Unit Tests: upgrade the fixtures in Tests_Post_Thumbnail_Template.

See #30017, #33968.

[wonderboymusic]

In 35196:

Unit Tests: create fewer terms in Tests_Term_getTerms::test_get_terms_parent_zero()

See #30017, #33968.

[wonderboymusic]

In 35197:

Unit Tests: add/upgrade the fixtures in Tests_User_Query.

See #30017, #33968.

[jdgrimes]

Replying to wonderboymusic:

In 35186:

Unit Tests: implement setUpBeforeClass() and tearDownAfterClass() on WP_UnitTestCase. Use late static binding (plus a gross fallback for PHP 5.2) to check if wpSetUpBeforeClass() or wpTearDownAfterClass() exist on the called class, and then call it and pass a static WP_UnitTest_Factory instance via Dependency Injection, if it exists.

This makes it way easier to add fixtures, and tear them down, without needing to instantiate WP_UnitTest_Factory in every class - removes the need to call commit_transaction() in each individual class.

See #30017, #33968.

Is there any reason why we can't just create one single instance of the factory and reuse it across all of the tests?

// in bootstrap.php 
include 'wp-unit-test-case.php';

WP_UnitTestCase::$static_factory = new WP_UnitTest_Factory();

// in WP_UnitTestCase::setUp();

$this->factory = self::$static_factory;

And there would no longer be any need to create the factory in setUpBeforeClass().

[boonebgorges]

I profiled the xmlrpc tests, and one bottleneck is the authentication process. get_user_by( 'login' ) takes a stupid amount of time. 33968.diff​ mocks the authentication process. I'm not sure if this is safe to do across all xmlrpc tests - probably not for the ones that are meant to test invalid u/p - but it does seem to reduce phpunit --group xmlrpc by about 15-20%.

[boonebgorges]

In 35206:

Fix incorrect variable name introduced in [35197].

See #30017, #33968.

[wonderboymusic]

In 35214:

Unit Tests: add SpeedTrapListener to phpunit/includes and add the config node to phpunit.xml.dist.

See #30017, #33968.

[wonderboymusic]

In 35224:

Unit Tests: since [32953], we can just use self::delete_user() instead of using if logic for Multisite.

See #30017, #33968.

[wonderboymusic]

In 35226:

Unit Tests: PHP 5.2, I Hate You and You Are Bringing Me Down.

#YOLOFriday

See #30017, #33968.

[nerrad]

preserve back compat of $factory property for plugins extending WP_UnitTestCase

[nerrad]

So as brought up in slack, switching $factory to a static property blows up plugins extending that WP_UnitTestCase and the WP factory. The patch I added in fix-blown-up-factory.diff will fix it for plugins and I think preserve the intent with the changes, but of course means that all the changes made in WordPress testcases using the static property will need updated to use $_factory instead. I didn't have time to go through and do this for this patch but just threw this up for now in case it helps.

[nerrad]

duh missed awesome.diff. I gave it a go and although it fixes for plugins using WP_UnitTestCase (with a tweak) WP unit tests will need updated because __get() only works in object context. I don't think you're going to be able to avoid changing the $factory property back to a non static property and using something else like $_factory to hold the static instance.

[nerrad]

expands on awesome.diff and implements factory() to grab the WP_UnitTestFactory instance for all wp tests. (Search and Replace self::$factory with self::factory()

[nerrad]

So basically I think you could just call self::factory() for all WP tests, so @wonderboymusic I made the tweak to your original diff and went through and did the search/replace for ya. This should work nicely to make sure plugins declaring using a $factory property still work.

[wonderboymusic]

In 35242:

Unit Tests: after [35225], make factory a method/getter on WP_UnitTestCase and add magic methods for BC for every plugin that is extending WP_UnitTestCase and accessing the $factory instance prop.

Props nerrad, wonderboymusic.
See #30017, #33968.

[wonderboymusic]

In 35243:

Unit Tests: after [35242], declare some missing instance props on individual test classes.

See #30017, #33968.

[wonderboymusic]

In 35244:

Unit Tests: WP_UnitTest_Generator_Sequence needs a static incrementer - otherwise, it assumes every test class is a reset, which it no longer is (it is now static).

While we're at it, remove unnecessary tearDown() code.

See #30017, #33968.

[wonderboymusic]

In 35245:

Unit Tests: better fixtures for Tests_Admin_Includes_Post.

See #30017, #33968.

[wonderboymusic]

In 35246:

Unit Tests: better fixtures for Tests_AdminBar. Don't force-delete some posts: some filter callbacks are no-ops on Multisite if the post is vanquished.

See #30017, #33968.

[wonderboymusic]

In 35247:

Unit Tests: better fixtures for Tests_User.

See #30017, #33968.

[wonderboymusic]

In 35248:

Unit Tests: better fixtures for Tests_User_WpSetCurrentUser.

See #30017, #33968.

[wonderboymusic]

In 35249:

Unit Tests: better fixtures for Tests_Meta_Slashes and Tests_WP_Customize_Section.

See #30017, #33968.

[DrewAPicture]

In 35270:

Tests: Add some more test coverage for get_term_field().

See #33968.

[wonderboymusic]

In 35311:

AJAX UNIT TESTS: Have you ever wondered why these take 600 forevers to run? They all eventually call do_action( 'admin_init' ), which has _maybe_update_core, _maybe_update_plugins, and _maybe_update_themes hooked to it. REMOVE THEM, and AJAX unit tests run like the wind. Tests_Ajax_Response is still slow.

See #30017, #33968.

[wonderboymusic]

we will be improving speed forever and always - #30017 already marked "ongoing"

[DrewAPicture]

In 42645:

Tests: Fix two typos introduced for a get_term_field() test in [35270].

See #33968.