Problem/Motivation

Umami performance tests all have the same setup, so we can save some CI time if we merge them into a single test class, and use the existing one-public-method-calling-protected-methods approach so it only sets up once.

There are four tests at 30-40s each, so potentiall 90s of CI time to save.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3612172

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:

About issue forks

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

Two small changes relative to the current tests:

1. Included #3612170: Random test failure in OpenTelementryAuthenticatedPerformanceTest.

2. Remove the uninstall of toolbar and install of navigation, because navigation is enabled in Umami anyway since February.

Otherwise this is just copypasta from the different test classes.

catch opened merge request !16376

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems straight forward.

What’s the timeline for umami getting removed btw? Not relevant to the issue but curious since I saw the name

catch’s picture

catch
he/him
Primary language English
commented

We've marked it hidden to then probably move it to a testing profile. As well as performance tests it's useful for multilingual tests and possibly to replace the update database dumps.

#3526560: Mark Umami as hidden in preparation of moving to contrib and/or a testing profile.

catch’s picture

catch
he/him
Primary language English
commented

In https://git.drupalcode.org/project/drupal/-/jobs/11056861 the new consolidated test takes 137s and finishes well before the end of the job. Quite a bit more than the 40s for the individual test classes I saw on a previous run, but some of that may be differences between runners. There's a solid 10-20s of sleep() time though let alone the extra actual http requests so it's bound to take more than an individual test.

catch’s picture

catch
he/him
Primary language English
commented

Rebased.

godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Needs review

One q on the MR.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Answered the question on the MR. Technically out of scope but since it's dead code easier to remove here.

  • godotislate committed 294a1288 on main 
    task: #3612172 Merge Umami performance tests

By: catch
By: godotislate
godotislate’s picture

godotislate
he/him
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 294a128 and pushed to main. Thanks!

Does not apply to 11.x, so if we want to backport, there'll need to be a separate MR.

catch’s picture

catch
he/him
Primary language English
commented
Status: Patch (to be ported) » Reviewed & tested by the community

Wasn't sure about backporting or not because we get conflicts on these tests for every backport, but after dealing with conflicts in deleted files vs the new files on a couple of issues, it seems easier to deal with them in one file.

Moving straight to RTBC because this was just mechanically updating the numbers for the differences between the branches, otherwise the whole file is taken from main as-is.

catch opened merge request !16575

  • godotislate committed 3b07549b on 11.x 
    task: #3612172 Merge Umami performance tests

By: catch
By: godotislate
godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Fixed

Verified the 11.x MR diff is essentially the same as the main commit diff.

Pushed to 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.