Make WordPress Core

Opened 10 years ago

Closed 7 months ago

Last modified 6 months ago

#30017 closed task (blessed) (fixed)

Many automated tests are unnecessarily slow

Reported by: boonebgorges's profile boonebgorges Owned by: wonderboymusic's profile wonderboymusic
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
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 (5)

30017.diff (876 bytes) - added by jdgrimes 10 years ago.
Speed up the Ajax tests by calling admin_init only once
get_attachment_taxonomies.patch (5.0 KB) - added by Frank Klein 7 years ago.
get-next-comments-link.patch (1.6 KB) - added by Frank Klein 7 years ago.
get-previous-comments-link.patch (1.6 KB) - added by Frank Klein 7 years ago.
customize-custom-css-setting.patch (1.5 KB) - added by Frank Klein 7 years ago.

Download all attachments as: .zip

Change History (79)

#1 @jeremyfelt
10 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
10 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
10 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
10 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
10 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
10 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
10 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
10 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
10 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
10 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
10 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
10 years ago

Speed up the Ajax tests by calling admin_init only once

#12 @jdgrimes
10 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
10 years ago

In 31676:

Share fixtures across wp_list_authors() tests.

See #30017.

#14 @boonebgorges
10 years ago

In 31848:

Use shared fixtures in RSS2 unit tests.

See #31705, #30017.

#15 @boonebgorges
9 years ago

In 35137:

Create fewer fixtures in some XML-RPC tests.

See #30017, #33968.

#16 @boonebgorges
9 years ago

In 35154:

Create fewer fixtures in XML-RPC getComments tests.

See #30017, #33968.

#17 @boonebgorges
9 years ago

In 35162:

Create fewer fixtures in some tests.

See #30017, #33968.

#18 @boonebgorges
9 years ago

In 35163:

Share fixtures in Tests_Get_Archives.

See #30017, #33968.

#19 @boonebgorges
9 years ago

In 35164:

Share fixtures in Tests_Admin_includesListTable tests.

See #30017, #33968.

#20 @boonebgorges
9 years ago

In 35165:

Fix incorrect variable names from [35164].

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

See #30017, #33968.

#21 @wonderboymusic
9 years ago

  • Milestone changed from Future Release to 4.4

#22 @wonderboymusic
9 years 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
9 years 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
9 years ago

In 35173:

Unit Tests: reuse fixtures in Tests_Comment.

See #30017, #33968.

#25 @wonderboymusic
9 years ago

In 35174:

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

See #30017, #33968.

#26 @wonderboymusic
9 years ago

In 35175:

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

See #30017, #33968.

#27 @wonderboymusic
9 years ago

In 35176:

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

See #30017, #33968.

#28 @wonderboymusic
9 years 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
9 years ago

In 35178:

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

See #30017, #33968.

#30 @wonderboymusic
9 years 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
9 years 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
9 years ago

In 35183:

Unit Tests: in Tests_Post, create fixtures for users.

See #30017, #33968.

#33 @wonderboymusic
9 years ago

In 35185:

Unit Tests: in Tests_Term, create fixtures for posts.

See #30017, #33968.

#34 @wonderboymusic
9 years 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
9 years 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
9 years ago

In 35191:

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

See #30017, #33968.

#37 @wonderboymusic
9 years 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
9 years ago

In 35193:

Unit Tests: upgrade the fixtures in Tests_Post_Revisions.

See #30017, #33968.

#39 @wonderboymusic
9 years ago

In 35194:

Unit Tests: upgrade the fixtures in Tests_Post_Thumbnail_Template.

See #30017, #33968.

#40 @wonderboymusic
9 years ago

In 35196:

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

See #30017, #33968.

#41 @wonderboymusic
9 years ago

In 35197:

Unit Tests: add/upgrade the fixtures in Tests_User_Query.

See #30017, #33968.

#42 @boonebgorges
9 years ago

In 35206:

Fix incorrect variable name introduced in [35197].

See #30017, #33968.

#43 @wonderboymusic
9 years ago

In 35214:

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

See #30017, #33968.

#44 @wonderboymusic
9 years 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.


9 years ago

#46 @wonderboymusic
9 years ago

In 35226:

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

#YOLOFriday

See #30017, #33968.

#47 @wonderboymusic
9 years 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
9 years ago

In 35243:

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

See #30017, #33968.

#49 @wonderboymusic
9 years 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
9 years ago

In 35245:

Unit Tests: better fixtures for Tests_Admin_Includes_Post.

See #30017, #33968.

#51 @wonderboymusic
9 years 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
9 years ago

In 35247:

Unit Tests: better fixtures for Tests_User.

See #30017, #33968.

#53 @wonderboymusic
9 years ago

In 35248:

Unit Tests: better fixtures for Tests_User_WpSetCurrentUser.

See #30017, #33968.

#54 @wonderboymusic
9 years ago

In 35249:

Unit Tests: better fixtures for Tests_Meta_Slashes and Tests_WP_Customize_Section.

See #30017, #33968.

#55 @wonderboymusic
9 years 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
9 years ago

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

#57 @boonebgorges
9 years ago

In 35857:

Share fixtures in get_comment_link() tests.

See #30017.

#58 @johnbillion
9 years ago

In 36047:

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

See #30017, #32394

#59 @johnbillion
9 years ago

In 36049:

Tests: Fix all the things.

See #30017, #32394

#60 @boonebgorges
9 years ago

In 36346:

Share post fixture in WP_Comment_Query tests.

See #30017.

#61 @jeremyfelt
9 years ago

In 37267:

Tests: Add speedTrapListener to multisite's PHPUnit config

See #36566, #30017.

#62 @johnbillion
8 years ago

In 38758:

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

See #30017

#63 @boonebgorges
8 years ago

In 38941:

Tests: Share fixtures in term endpoint tests.

See #30017.

#64 @boonebgorges
8 years 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 years ago

In 39008:

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

See #30017

#66 @johnbillion
8 years 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

This ticket was mentioned in Slack in #hosting-community by danielbachhuber. View the logs.


7 years ago

#68 @Frank Klein
7 years ago

The two patches above modify the get_next_comments_link(), and get_previous_comments_link() tests to:

  • Use shared post permalink fixture
  • Do not reset query vars after tests, as this happens automatically now.
  • Use assertNull to check the return of get_previous_comments_link().
Last edited 7 years ago by Frank Klein (previous) (diff)

#69 @SergeyBiryukov
5 years ago

In 46657:

REST API: Speed up pagination unit tests by creating less fixtures and reusing them where possible.

Includes minor documentation and code layout fixes for better readability.

See #30017, #48145.

#70 @SergeyBiryukov
5 years ago

In 46829:

Tests: Speed up comment submission unit tests by creating less fixtures and reusing them where possible.

See #30017, #48145.

This ticket was mentioned in Slack in #core-editor by mamaduka. View the logs.


4 years ago

#72 @pbearne
7 months ago

can we close this ticket now?

#73 @johnbillion
7 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

#74 @desrosj
6 months ago

  • Keywords ongoing removed
  • Milestone changed from Future Release to 5.4

Assigning this the 5.4 milestone, which was the next major release following [46829].

Note: See TracTickets for help on using tickets.