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

Command icon 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:

Comments

catch created an issue. See original summary.

jjsanz’s picture

catch’s picture

Status: Active » Needs work

Down to a couple of test failures but they don't really make sense to me, so going to pause here for a bit.

mstrelan’s picture

I'm getting similar failures in #3468435: Convert IpAddressBlockingTest to a Unit and Kernel test and improve ... I think HEAD is broken

catch’s picture

Status: Needs work » Needs review

Fixed 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:

Drupal\Tests\jsonapi\Functional\CommentTest                   11 passes  171s 

(although I've seen this take more than 180s on other test runs)

After

CommentTest Drupal\Tests\jsonapi\Functional\CommentTest                    9 passes  162s

Combined with Alex's multiple module install issue, it goes down to:

rupal\Tests\jsonapi\Functional\CommentTest                    9 passes  153s

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.

bbrala’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed 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.

nod_’s picture

Status: Reviewed & tested by the community » Needs work

I think all the doTest… methods should be protected per the previous optimisation patches.

catch’s picture

Status: Needs work » Reviewed & tested by the community

Good point - made that change.

  • nod_ committed 1a870a25 on 10.3.x
    Issue #3468280 by catch, bbrala: Speed up JSON:API ResourceTestBase
    
    (...

  • nod_ committed 9048455c on 10.4.x
    Issue #3468280 by catch, bbrala: Speed up JSON:API ResourceTestBase
    
    (...

  • nod_ committed 84159273 on 11.0.x
    Issue #3468280 by catch, bbrala: Speed up JSON:API ResourceTestBase
    
    (...

  • nod_ committed ae9641c1 on 11.x
    Issue #3468280 by catch, bbrala: Speed up JSON:API ResourceTestBase
    
nod_’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.