#33968 closed defect (bug) (fixed)
Improve performance of Unit Tests for 4.4
Reported by: | jorbin | Owned by: | jorbin |
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | |
Focuses: | Cc: |
Description
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.
Attachments (5)
Change History (89)
#4
follow-up:
↓ 5
@
9 years ago
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';
#5
in reply to:
↑ 4
@
9 years ago
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
?
#9
@
9 years ago
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
This ticket was mentioned in Slack in #core by jorbin. View the logs.
9 years ago
This ticket was mentioned in Slack in #core by jorbin. View the logs.
9 years ago
#13
follow-up:
↓ 30
@
9 years ago
Intermittant failure: Tests_Ajax_DeleteComment::test_ajax_comment_trash_actions_as_administrator
See https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/82849174
#14
@
9 years ago
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.
#19
@
9 years ago
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 or post types, 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.
This ticket was mentioned in Slack in #core by drew. View the logs.
9 years ago
#21
@
9 years ago
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.
#22
@
9 years ago
Intermittant failure: Tests_HTTP_curl::test_request_limited_size
See https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/83814943
#30
in reply to:
↑ 13
@
9 years ago
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
#33
@
9 years ago
Some performance breakdown on individual tests: https://gist.github.com/ocean90/9ef0f64955ef9df2304f.
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.
However, excluding them in the config file, if we want to do that, seems to work.
33968.patch excludes XML-RPC tests from normal runs and moves them to a separate group.
#34
@
9 years ago
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.
#35
@
9 years ago
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.
This ticket was mentioned in Slack in #core by helen. View the logs.
9 years ago
#62
in reply to:
↑ 54
@
9 years ago
Replying to wonderboymusic:
In 35186:
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()
.
#63
@
9 years ago
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%.
This ticket was mentioned in Slack in #core by boone. View the logs.
9 years ago
This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.
9 years ago
#70
@
9 years ago
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.
#71
@
9 years ago
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.
@
9 years ago
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()
#72
@
9 years ago
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.
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.