Problem/Motivation
There's a lot of tests that extend ResourceTestBase and they are a significant percentage of overall running times for functional js tests.
Trying to merge three test methods into one to reduce runtimes. This nearly but not quite passes locally, but ran out of time.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3468280
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3468280-speed-up-jsonapi
changes, plain diff MR !9220
Comments
Comment #3
jjsanzAdding related issue.
Comment #4
catchDown to a couple of test failures but they don't really make sense to me, so going to pause here for a bit.
Comment #5
mstrelan commentedI'm getting similar failures in #3468435: Convert IpAddressBlockingTest to a Unit and Kernel test and improve ... I think HEAD is broken
Comment #6
catchFixed the test failures in the subclasses - all small changes in the end. Obviously there's a lot of variation between test runs anyway but it definitely saves time:
Before:
(although I've seen this take more than 180s on other test runs)
After
Combined with Alex's multiple module install issue, it goes down to:
We can assume most of the other tests based on ResourceTestBase will also be reduced by 5-20 seconds each, so should result in overall minutes of compute time reduced given there are 55 tests using ResourceTestBase.
Comment #7
bbralaReviewed the changes, net code is effectively the same. These tests run for a lot of entity types, this could be quite the save.
I did do a 180 when i saw the changes to testPostIndividual since it seemed the skipped part was removed. Technically it is, but the todo is kept. I understand that we can't keep the markTestSkipped, since that would kinda undo part of the optimisation, just a little sad that is is no longer visible in the suite that the post test for files is skipped.
I'm all for the optimisations, thanks catch. Keep on keeping on.
Comment #8
nod_I think all the doTest… methods should be protected per the previous optimisation patches.
Comment #9
catchGood point - made that change.
Comment #15
nod_Committed and pushed ae9641c117 to 11.x and 8415927305 to 11.0.x and 9048455c90 to 10.4.x and 1a870a2555 to 10.3.x. Thanks!