Problem/Motivation

Very slow kernel test compared to the others.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 3615599-nr-bot_id8v7rbk.txt98 bytesneeds-review-queue-bot
#6 3615599-nr-bot_3evlvb3b.txt1.37 KBneeds-review-queue-bot

Issue fork drupal-3615599

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 opened merge request !16620

catch’s picture

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

Depends on #3613200: Move composer inspection prior to each overall package manager step instead of prior to every call to composer

catch’s picture

catch
he/him
Primary language English
commented
Title: Split up SandboxManager » Split up SandboxManagerBaseTest
catch’s picture

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

That issue is not getting reviews, so unstacking this one.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3615599-nr-bot_3evlvb3b.txt1.37 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

catch’s picture

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

Usual caveats about runners being different:

Before split:

 176.667s Drupal\Tests\package_manager\Kernel\SandboxManagerBaseTest               30 passed
https://git.drupalcode.org/project/drupal/-/jobs/12686902

This is long enough that it starts first and finishes last, and pushes the specific kernel job into one of the longest running test jobs.

After split:

 78.399s Drupal\Tests\package_manager\Kernel\SandboxManagerBaseTest               17 passed
46.077s Drupal\Tests\package_manager\Kernel\SandboxManagerBaseFailureTest         8 passed
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3615599-nr-bot_id8v7rbk.txt98 bytes

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

catch’s picture

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

Bonus after, because I messed up the actual rebase against main so had to do it again, resulting in another pipeline run:

 80.864s Drupal\Tests\package_manager\Kernel\SandboxManagerBaseTest               25 passed
48.121s Drupal\Tests\package_manager\Kernel\SandboxManagerBaseFailureTest         8 passed

Very close numbers to #7.

dcam’s picture

dcam commented
Status: Needs review » Needs work

I left one suggestion and had one question on the MR.

catch’s picture

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

All good spots, think I got them all.

 54.247s Drupal\Tests\package_manager\Kernel\SandboxManagerBaseFailureTest         8 passed
124.462s Drupal\Tests\package_manager\Kernel\SandboxManagerBaseTest               22 passed

https://git.drupalcode.org/project/drupal/-/jobs/12697110

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me :)

mstrelan’s picture

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

One question on the MR

catch’s picture

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

I think that's fine to remove here, applied the suggestion and moving back to RTBC.

mstrelan’s picture

mstrelan commented

@catch there was actually two places where I left a suggestion to remove this, and I'm not sure if you meant that only one is fine to remove here or just missed the second one.

catch’s picture

catch
he/him
Primary language English
commented

No I just missed it, sorry. Applied that too.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.5.x-dev
Status: Reviewed & tested by the community » Fixed

Backported down to 11.5.x to keep things in sync there too.

Committed and pushed f4e16f93657 to main and 8a7287f3e4c to 12.0.x and 70cfbbdb06d to 11.x and 6ef24f9b314 to 11.5.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.

  • longwave committed 6ef24f9b on 11.5.x 
    test: #3615599 Split up SandboxManagerBaseTest

By: catch
By: dcam
By:...

  • longwave committed 70cfbbdb on 11.x 
    test: #3615599 Split up SandboxManagerBaseTest

By: catch
By: dcam
By:...

  • longwave committed 8a7287f3 on 12.0.x 
    test: #3615599 Split up SandboxManagerBaseTest

By: catch
By: dcam
By:...

  • longwave committed f4e16f93 on main 
    test: #3615599 Split up SandboxManagerBaseTest

By: catch
By: dcam
By:...