WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 8 months ago

#30017 assigned task (blessed)

Many automated tests are unnecessarily slow

Reported by: boonebgorges Owned by: wonderboymusic
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: ongoing
Focuses: performance Cc:

Description

Our test suite is getting bigger (that's good!). But many of our tests are, of necessity, integration and functional tests that require lots of setup: creating factory data, resetting globals, etc. This process can be slow (that's bad!).

Poking around in the tests, it looks like some of the worst offenders are those who create lots of database content in the setUp() method. Creating more dummy data than is absolutely necessary to test an assertion is - at best - wasteful. At worst, it actually introduces unnecessary variables into what is supposed to be a narrowly defined test. (Fake but illustrative example: if you create 25 posts to test some default value in WP_Query, you now have to worry about pagination in addition to whatever value you're testing.)

Changing existing tests is a tedious and potentially dangerous task - you don't want to introduce regressions into our regression-preventing tests. But if we can shave 10-20% off of the execution time of our suite (which I think is a pretty conservative estimate), it'd be a huge step toward getting more people to actually run the dang things, as well as things like continuous integration.

Opening this ticket for discussion and/or patches.

Attachments (1)

30017.diff (876 bytes) - added by jdgrimes 2 years ago.
Speed up the Ajax tests by calling admin_init only once

Download all attachments as: .zip

Change History (67)

#1 @jeremyfelt
3 years ago

In 29937:

Improve and reduce tests for get_blogs_of_user()

  • Create half as many factory sites. See #30017
  • Test the removal of a user from multiple sites.
  • Expand tests to include second parameter for flagged sites.
  • Remove duplicate test for deleted user.

Fixes #29996

#2 @jeremyfelt
3 years ago

I forgot it auto includes commits. :)

Yes - very much. I think any time a factory is used to create test data, we should have a very specific idea of what that test data is being used for.

#3 @boonebgorges
3 years ago

In 29981:

Remove redundant unit test for WP_Comment_Query 'status'.

This old test is too resource-intensive, and duplicates the more precise
tests introduced in [29980].

See #30017.

#4 @boonebgorges
3 years ago

In 29992:

Generate fewer default posts in wp_list_authors() tests.

Saves about 7 seconds when running the suite.

See #30017.

#5 @boonebgorges
3 years ago

In 29993:

Streamline WP_Date_Query unit tests.

By creating less dummy data and eliminating redundant tests, we cut group
execution time by more than 50%.

See #30017.

#6 @boonebgorges
3 years ago

In 30017:

Streamline WP_User_Query unit tests.

  • Don't create user during setUp(), as it's not used in every test.
  • Create fewer users in get_all and orderby tests.

See #30017.

#7 @nacin
3 years ago

Huge +1 for reducing unnecessary fixtures.

I believe canonical has always been our biggest offender, at least historically. Open to suggestions for that in particular. The original architect of the tests was dd32 and he may have some ideas.

#8 @boonebgorges
3 years ago

In 30113:

Streamline some get_terms() cache tests.

  • Split large method into a number of smaller tests.
  • Create fewer fixtures.

See #30017.

#9 @boonebgorges
3 years ago

In 30276:

Share fixtures across a number of query-related test classes.

This shaves 10-20 seconds off the running time for the suite.

See #30017.

#10 @boonebgorges
3 years ago

In 30277:

Share fixtures across 'canonical' automated tests.

Sharing these fixtures results in a speed improvement of almost one minute per
run of the test suite.

My hope is that future WordPress developers will spend this extra minute with
their loved ones, for life on this earth is short, my friends, and the moments
you spend watching WP generate test data can never again be reclaimed from the
grizzled clutches of Time, and none of us are really getting younger, I mean,
geez, have you looked in the mirror lately, Gandalf?

See #30017.

#11 @boonebgorges
3 years ago

In 30511:

Improve performance of post revision order test.

test_revision_order() was written ([28541], #26042) to ensure that revision
order was properly preserved in two different cases: (1) where the post_date
varied (in which case the revisions would be sorted by post_date DESC) and
(2) where the post_date was the same (in which case sorting would fall back on
ID DESC). In an attempt to ensure that both of these scenarios arose in the
context of a single test, 100 posts were created. We can make the process far
more efficient by manually creating the revisions with the post_dates
explicitly declared, and splitting the two different cases into two separate
test methods.

This test was previously the single worst offender in the entire suite, taking
upwards of 15 seconds to run. All that most maddens and torments; all that stirs
up the lees of things; all truth with malice in it; all that cracks the sinews
and cakes the brain; all the subtle demonisms of life and thought; all evil, to
crazy Boone, were visibly personified, and made practically assailable in
test_revision_order().

See #30017.

@jdgrimes
2 years ago

Speed up the Ajax tests by calling admin_init only once

#12 @jdgrimes
2 years ago

30017.diff attempts to speed up the Ajax tests by only calling the admin_init action once. At present the action is called for every single test, which accounts for a significant portion of the time the Ajax tests take to run. I don't know if there is a reason that admin_init was being called every time, and #UT25 yields no clues. Maybe someone else with more knowledge of what is hooked to this action and how it affects an Ajax call could shed some light on this. All I know is that I have been running Ajax tests for a plugin with this patch applied for some time, and I haven't experienced any issues, or seen any evidence that it causes the tests to leak into each other.

#13 @boonebgorges
2 years ago

In 31676:

Share fixtures across wp_list_authors() tests.

See #30017.

#14 @boonebgorges
2 years ago

In 31848:

Use shared fixtures in RSS2 unit tests.

See #31705, #30017.

#15 @boonebgorges
21 months ago

In 35137:

Create fewer fixtures in some XML-RPC tests.

See #30017, #33968.

#16 @boonebgorges
21 months ago

In 35154:

Create fewer fixtures in XML-RPC getComments tests.

See #30017, #33968.

#17 @boonebgorges
21 months ago

In 35162:

Create fewer fixtures in some tests.

See #30017, #33968.

#18 @boonebgorges
21 months ago

In 35163:

Share fixtures in Tests_Get_Archives.

See #30017, #33968.

#19 @boonebgorges
21 months ago

In 35164:

Share fixtures in Tests_Admin_includesListTable tests.

See #30017, #33968.

#20 @boonebgorges
21 months ago

In 35165:

Fix incorrect variable names from [35164].

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

See #30017, #33968.

#21 @wonderboymusic
21 months ago

  • Milestone changed from Future Release to 4.4

#22 @wonderboymusic
21 months ago

In 35171:

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

See #30017, #33968.

#23 @wonderboymusic
21 months ago

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.

#24 @wonderboymusic
21 months ago

In 35173:

Unit Tests: reuse fixtures in Tests_Comment.

See #30017, #33968.

#25 @wonderboymusic
21 months ago

In 35174:

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

See #30017, #33968.

#26 @wonderboymusic
21 months ago

In 35175:

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

See #30017, #33968.

#27 @wonderboymusic
21 months ago

In 35176:

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

See #30017, #33968.

#28 @wonderboymusic
21 months ago

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.

#29 @wonderboymusic
21 months ago

In 35178:

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

See #30017, #33968.

#30 @wonderboymusic
21 months ago

In 35179:

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

See #30017, #33968.

#31 @wonderboymusic
21 months ago

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.

#32 @wonderboymusic
21 months ago

In 35183:

Unit Tests: in Tests_Post, create fixtures for users.

See #30017, #33968.

#33 @wonderboymusic
21 months ago

In 35185:

Unit Tests: in Tests_Term, create fixtures for posts.

See #30017, #33968.

#34 @wonderboymusic
21 months ago

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.

#35 @wonderboymusic
21 months ago

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.

#36 @wonderboymusic
21 months ago

In 35191:

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

See #30017, #33968.

#37 @wonderboymusic
21 months ago

In 35192:

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

See #30017, #33968.

#38 @wonderboymusic
21 months ago

In 35193:

Unit Tests: upgrade the fixtures in Tests_Post_Revisions.

See #30017, #33968.

#39 @wonderboymusic
21 months ago

In 35194:

Unit Tests: upgrade the fixtures in Tests_Post_Thumbnail_Template.

See #30017, #33968.

#40 @wonderboymusic
21 months ago

In 35196:

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

See #30017, #33968.

#41 @wonderboymusic
21 months ago

In 35197:

Unit Tests: add/upgrade the fixtures in Tests_User_Query.

See #30017, #33968.

#42 @boonebgorges
21 months ago

In 35206:

Fix incorrect variable name introduced in [35197].

See #30017, #33968.

#43 @wonderboymusic
21 months ago

In 35214:

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

See #30017, #33968.

#44 @wonderboymusic
21 months ago

In 35224:

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

See #30017, #33968.

This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.


21 months ago

#46 @wonderboymusic
21 months ago

In 35226:

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

#YOLOFriday

See #30017, #33968.

#47 @wonderboymusic
21 months ago

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.

#48 @wonderboymusic
21 months ago

In 35243:

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

See #30017, #33968.

#49 @wonderboymusic
21 months ago

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.

#50 @wonderboymusic
21 months ago

In 35245:

Unit Tests: better fixtures for Tests_Admin_Includes_Post.

See #30017, #33968.

#51 @wonderboymusic
21 months ago

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.

#52 @wonderboymusic
21 months ago

In 35247:

Unit Tests: better fixtures for Tests_User.

See #30017, #33968.

#53 @wonderboymusic
21 months ago

In 35248:

Unit Tests: better fixtures for Tests_User_WpSetCurrentUser.

See #30017, #33968.

#54 @wonderboymusic
21 months ago

In 35249:

Unit Tests: better fixtures for Tests_Meta_Slashes and Tests_WP_Customize_Section.

See #30017, #33968.

#55 @wonderboymusic
20 months ago

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.

#56 @wonderboymusic
20 months ago

  • Keywords ongoing added
  • Milestone changed from 4.4 to Future Release
  • Owner set to wonderboymusic
  • Status changed from new to assigned

#57 @boonebgorges
19 months ago

In 35857:

Share fixtures in get_comment_link() tests.

See #30017.

#58 @johnbillion
18 months ago

In 36047:

Tests: Shave a second off the user capability tests by reusing its user fixtures.

See #30017, #32394

#59 @johnbillion
18 months ago

In 36049:

Tests: Fix all the things.

See #30017, #32394

#60 @boonebgorges
17 months ago

In 36346:

Share post fixture in WP_Comment_Query tests.

See #30017.

#61 @jeremyfelt
14 months ago

In 37267:

Tests: Add speedTrapListener to multisite's PHPUnit config

See #36566, #30017.

#62 @johnbillion
9 months ago

In 38758:

HTTP API: Remove an unnecessary duplicate HTTP request in the HTTP tests.

See #30017

#63 @boonebgorges
8 months ago

In 38941:

Tests: Share fixtures in term endpoint tests.

See #30017.

#64 @boonebgorges
8 months ago

In 38975:

Share fixtures in REST API endpoint tests.

As sparrows' tears shed steadily
Make widest rivers filled,

setUp() routines run prodig'ly
Add minutes to a build.

So cull ye fixtures profligate!
Direct thine frugal gaze!

Our savings here - a half-minute -
When multiplied: Amaze!

See #30017.

#65 @johnbillion
8 months ago

In 39008:

Role/Capability: Reuse a fixture in a couple more unit tests.

See #30017

#66 @johnbillion
8 months ago

In 39189:

Build/Test Tools: Re-use a bunch of fixtures in test classes for user and XMLRPC tests.

Shaves a couple of seconds off of the tests.

See #30017, #38716

Note: See TracTickets for help on using tickets.