Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
In other words: Finish #1373142: Use the Testing profile. Speed up testbot by 50%
A couple of tests have been left out in the original patch, since they rely on a lot of default configuration provided by the Standard profile.
Converting those tests will speed up the testbot for the full Drupal core test suite significantly.
$ grep -lr "\$profile = 'standard'" core/
core/modules/config/lib/Drupal/config/Tests/ConfigImportAllTest.php
core/modules/comment/lib/Drupal/comment/Tests/CommentPreviewTest.php
core/modules/filter/lib/Drupal/filter/Tests/FilterAdminTest.php
core/modules/help/lib/Drupal/help/Tests/HelpTest.php
core/modules/node/lib/Drupal/node/Tests/NodeAccessBaseTableTest.php
core/modules/node/lib/Drupal/node/Tests/NodeTranslationUITest.php
core/modules/rdf/lib/Drupal/rdf/Tests/StandardProfileTest.php
core/modules/search/lib/Drupal/search/Tests/SearchCommentCountToggleTest.php
core/modules/search/lib/Drupal/search/Tests/SearchCommentTest.php
core/modules/system/lib/Drupal/system/Tests/Cache/PageCacheTagsIntegrationTest.php
core/modules/system/lib/Drupal/system/Tests/Common/NoJavaScriptAnonymousTest.php
core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php
core/modules/system/lib/Drupal/system/Tests/Module/ModuleApiTest.php
core/modules/system/lib/Drupal/system/Tests/Theme/EntityFilteringThemeTest.php
core/modules/taxonomy/lib/Drupal/taxonomy/Tests/LegacyTest.php
core/modules/user/lib/Drupal/user/Tests/UserPictureTest.php
core/profiles/standard/lib/Drupal/standard/Tests/StandardTest.php
Of course, StandardTest
remains as the only exception.
Comment | File | Size | Author |
---|---|---|---|
#27 | test.standard.27.patch | 19.94 KB | sun |
#20 | more-testing-tests-1595028-20.patch | 5.82 KB | Berdir |
#9 | remove-standard-profile-from-tests-1595028-9.patch | 15.11 KB | Berdir |
#9 | remove-standard-profile-from-tests-1595028-9-interdiff.txt | 1.77 KB | Berdir |
#7 | remove-standard-profile-from-tests-1595028-7.patch | 15.34 KB | Berdir |
Comments
Comment #1
sunOr let's even limit it to 'standard'.
Comment #2
BerdirThis removes the standard profile from a bunch of tests, notably the file field tests.
- MenuTest currently has like a million test assertions duplicated on the default tools menu and a custom one. Not sure if there is a point in keeping the tools part, why should that be any different than a custom menu? Dropped that part.
- Not all tests are working yet, most of the partially converted ones are missing filter stuff, will wait with that after filter has been converted to a config entity.
RDF has quite a few tests that use the standard profile, not yet sure how I can change those, most of the remaining ones then actually often test specific standard.module stuff, not sure if they can be updated...
Comment #4
BerdirRe-rolled patch and removed changes from classes that don't work yet.
What about making incremental steps and getting this in while it's hot? :)
I've just checked and the difference is huge. "File field widget test" takes 1m4s on HEAD and 25s with this patch. And the difference is even bigger on the testbot because my system is considerably faster. I'm currently running those file tests manually in another issue and it's a pain.
Comment #5
BerdirComment #7
BerdirFixed the comment permission problem. Didn't notice that because I run exactly the one test that does enable comment ;)
Comment #8
sunThis looks awesome! Thanks, @Berdir!
I agree we should rather move forward quickly with these test performance optimizations while they're hot + apply. We can keep the issue open after commit to convert some more.
Only one nitpick:
field_ui gets already enabled by FileFieldTestBase, so we can remove it from the extending test classes.
With that, RTBC, please. :)
Comment #9
BerdirSure, can do that.
Those are the remaining standard profile test:
I'll continue working on these on the patch is in, waiting for the filters API to become stable before tackling them as quite a few are related to that.
I also noticed a number of them that explicitly specify the testing problem, that shouldn't be necessary unless they extend from a test that specifies standard. Will check them too.
Comment #10
sunYay, thank you! :)
Let's quickly get those improvements in. Everyone will be happy, even in case of conflicts. :)
Comment #12
Berdir#9: remove-standard-profile-from-tests-1595028-9.patch queued for re-testing.
Random test failures.
Comment #14
Berdir#9: remove-standard-profile-from-tests-1595028-9.patch queued for re-testing.
Another random upgrade path test failure that has nothing to do with this issue. Testbot, this is not funny :( Why don't you fail with my debug patches...
Comment #15
sunIn any case, back to RTBC, whenever it comes green, please ;)
Comment #16
ParisLiakos CreditAttribution: ParisLiakos commentedWell this makes testbot 10 minutes faster!yay!! nice cleanup :)yeah, forget what i just said, berdir explained me how unpredictable the bot testing times are
Comment #17
webchickYay for more fasterer tests. :)
Committed and pushed to 8.x.
Comment #18
BerdirBack to active for the remaining standard tests.
Comment #19
sunYou mentioned that many of the remaining would be related to the Filter API?
The API for text formats shouldn't actually change that much anymore — the config conversion patch was pretty much final/complete. We're still working on converting filters into plugins, but I don't think that will change the way of how tests create stub/mock/testing formats.
Speaking of, whenever possible, I think we should try to change the test assertions to expect the output of the default plain_text format, instead of a custom filtered_html/full_html format. However, if that means too much work, then let's just create a new one.
Comment #20
BerdirSome more tests converted.
Some of the standard profile tests also vanished on their own, e.g. some Rdf tests, nice.
There are still a few fails in the changed tests. We also need to make sure that the tests still test test what they are supposed to.
Comment #22
sun#20: more-testing-tests-1595028-20.patch queued for re-testing.
Comment #23
sunThis one needs more adjustments; @see #1235062: text_summary() ignores filter status + #1868772: Convert filters to plugins
That said, due to the bugs that are apparently contained in the functional code, it may pass even without Standard profile config :)
Comment #25
sunRemaining tests:
Of course,
StandardTest
remains as the only exception.Comment #26
sunSorry, that grep contained false-positive debugging file on my local disk.
Comment #27
sunRe-rolled + blatantly removing the $profile definition from all affected tests... ;)
Got a little hung-up with
ModuleApiTest
, because I noticed that that this test can be converted from WebTestBase + Standard profile into DUTB... :) — However, we should likely split that into a separate issue.Comment #28
tim.plunkettThis is going to test the hook_help of a LOT less modules, right? Shouldn't we just enable everything?
Ummm.
I mean, changing the name is fine, but wasn't this the whole point of the test?
Same here. It seems to be changing the meaning of the test.
Once more.
Comment #30
sunI'm going to create dedicated spin-off issues for each of the offending tests now.
Comment #31
sunHere we go:
#2254175: Fix test performance of Drupal\comment\Tests\CommentPreviewTest
#2254179: Fix test performance of Drupal\config\Tests\ConfigImportAllTest
#2254183: Fix test performance of Drupal\filter\Tests\FilterAdminTest
#2254185: Remove Drupal\help\Tests\HelpTest
#2254187: Fix test performance of Drupal\node\Tests\NodeAccessBaseTableTest
#2254189: Fix test performance of Drupal\node\Tests\NodeTranslationUITest
#2254191: Fix test performance of Drupal\rdf\Tests\StandardProfileTest
#2254195: Fix test performance of Drupal\search\Tests\SearchCommentCountToggleTest
#2254197: Fix test performance of Drupal\search\Tests\SearchCommentTest
#2254199: Fix test performance of Drupal\system\Tests\Menu\BreadcrumbTest
#2254201: Fix test performance of Drupal\system\Tests\Theme\EntityFilteringThemeTest
#2254203: Fix test performance of Drupal\system\Tests\Module\ModuleApiTest
#2254207: Fix test performance of Drupal\system\Tests\Common\NoJavaScriptAnonymousTest
#2254209: Fix test performance of Drupal\system\Tests\Cache\PageCacheTagsIntegrationTest
#2254211: Fix test performance of Drupal\taxonomy\Tests\LegacyTest
#2254213: Fix test performance of Drupal\user\Tests\UserPictureTest
Comment #32
sunThe following are ready to go:
#2254211: Fix test performance of Drupal\taxonomy\Tests\LegacyTest
#2254203: Fix test performance of Drupal\system\Tests\Module\ModuleApiTest
#2254197: Fix test performance of Drupal\search\Tests\SearchCommentTest
#2254195: Fix test performance of Drupal\search\Tests\SearchCommentCountToggleTest
#2254183: Fix test performance of Drupal\filter\Tests\FilterAdminTest
Comment #43
volegerAlmost all tests are converted to phpunit-based tests.
The next issues left to update IS and review:
#2254175: Fix test performance of Drupal\comment\Tests\CommentPreviewTest
#2254179: Fix test performance of Drupal\config\Tests\ConfigImportAllTest
#2254187: Fix test performance of Drupal\node\Tests\NodeAccessBaseTableTest
#2254189: Fix test performance of Drupal\node\Tests\NodeTranslationUITest
#2254191: Fix test performance of Drupal\rdf\Tests\StandardProfileTest
#2254199: Fix test performance of Drupal\system\Tests\Menu\BreadcrumbTest
#2254201: Fix test performance of Drupal\system\Tests\Theme\EntityFilteringThemeTest
#2254207: Fix test performance of Drupal\system\Tests\Common\NoJavaScriptAnonymousTest
#2254209: Fix test performance of Drupal\system\Tests\Cache\PageCacheTagsIntegrationTest
#2254213: Fix test performance of Drupal\user\Tests\UserPictureTest
Comment #48
quietone CreditAttribution: quietone at PreviousNext commentedThis issue was committed to 8.x and then reopened as a Meta to fix the remaining tests which seem to require a bit more work to install necessary configuration. Experience from #bugsmash has shown that issues that are committed and re-opened can be difficult to triage and figure out what needs to be done. So, instead of keeping this open I'm moving the remaining work to a new Meta, a sibling of this one, so this can be closed.
I also noticed that the majority of the child issues have no issue summary. I'll add a template to them when I update them to change the parent issue.
Also, restoring the meta data for the version this was committed which was 8.x