Problem/Motivation

256.274s Drupal\Tests\system\Functional\UpdateSystem\UpdateScriptTest             16 passed

---- Drupal\Tests\system\Functional\UpdateSystem\UpdateScriptTest ----
Status      Duration Info                                                                               
--------------------------------------------------------------------------------------------------------
Pass         10.266s testUpdateAccess                                                                
Pass         20.144s testRequirements                                                                
Pass         26.206s …ibilityChange with data set "module: core_version_requirement key incompatible"
Pass         14.281s …tibilityChange with data set "theme: core_version_requirement key incompatible"
Pass         26.138s testExtensionCompatibilityChange with data set "module: php requirement"        
Pass         14.306s testExtensionCompatibilityChange with data set "theme: php requirement"         
Pass         30.753s testMissingExtension with data set "core only"                                  
Pass         28.677s testMissingExtension with data set "contrib only"                               
Pass         51.869s testMissingExtension with data set "core and contrib"                           
Pass          4.315s testOrphanedSchemaEntries                                                       
Pass          2.699s testThemeSystem                                                                 
Pass          4.289s testNoUpdateFunctionality                                                       
Pass          6.169s testSuccessfulUpdateFunctionality                                               
Pass          4.000s testMaintenanceModeUpdateFunctionality                                          
Pass          4.347s testSuccessfulMultilingualUpdateFunctionality                                   
Pass          2.965s testMaintenanceModeLink

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#18 3612249-nr-bot_iq3sqcdp.txt1.18 KBneeds-review-queue-bot
#12 3612249-nr-bot_z3jkkekg.txt1.73 KBneeds-review-queue-bot
#7 3612249-nr-bot_eg3171pe.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3612249

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
Issue summary: View changes
catch’s picture

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

catch opened merge request !16379

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Test suite performance
catch’s picture

catch
he/him
Primary language English
commented

This run shows both UpdateScriptTest* taking a lot less time each than the single test did: https://git.drupalcode.org/project/drupal/-/jobs/11065446

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3612249-nr-bot_eg3171pe.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. 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

Rebased.

smustgrave’s picture

smustgrave commented

1 small comment on the MR.

catch’s picture

catch
he/him
Primary language English
commented

Good point - made the base class abstract.

smustgrave’s picture

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

Thanks LGTM!

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3612249-nr-bot_z3jkkekg.txt1.73 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
smustgrave’s picture

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

Rebase still looks good!

godotislate’s picture

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

Code split looks good. I have two very minor comments, could go either way on them, but I figure it's best to do now if we can.

godotislate’s picture

godotislate
he/him
commented
Status: Needs review » Needs work

Merge conflict now.

catch’s picture

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

Rebased.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3612249-nr-bot_iq3sqcdp.txt1.18 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
dcam’s picture

dcam commented
Status: Needs review » Needs work

I'm sorry. I found two typos. I nearly applied my own suggestions to fix them and self-RTBCed the issue, but instead decided that I didn't like the grammar in those comments. Let me know if you think the suggestions are appropriate.

FWIW, I also re-verified the function moves and they look good.

catch’s picture

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

Applied the two suggestions, looked good to me.

dcam’s picture

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

Thanks for checking my feedback. LGTM

  • amateescu committed e5a59ea9 on main 
    task: #3612249 Split update script missing extension test to its own...
amateescu’s picture

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

Committed e5a59ea and pushed to main. Thanks!

Needs a rebase for 11.x :)

amateescu’s picture

amateescu commented
Version: 11.x-dev » main
Status: Patch (to be ported) » Fixed

Discussed with @catch and the backport is not worth the trouble.

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.

Status: Fixed » Closed (fixed)

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