#30017 closed task (blessed) (fixed)
Many automated tests are unnecessarily slow
Reported by: | boonebgorges | Owned by: | 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)
Change History (79)
#2
@
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.
#7
@
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.
#12
@
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.
This ticket was mentioned in Slack in #core by wonderboymusic. View the logs.
9 years ago
#56
@
9 years ago
- Keywords ongoing added
- Milestone changed from 4.4 to Future Release
- Owner set to wonderboymusic
- Status changed from new to assigned
This ticket was mentioned in Slack in #hosting-community by danielbachhuber. View the logs.
7 years ago
#68
@
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 ofget_previous_comments_link()
.
In 29937: