Problem/Motivation

Submitting the modules page redirects to the modules page, so no need to request it again.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3614518

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

catch opened merge request !16518

smustgrave’s picture

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

Unrelated javascript failures. LGTM.

catch’s picture

catch
he/him
Primary language English
commented
Title: Remove unnecessary requests in ConfigInstallWebTest » Remove unnecessary requests and consolidate test methods in ConfigInstallWebTest
Status: Reviewed & tested by the community » Needs review

This wasn't doing a lot for the test timing.

Found three methods which are easy to collapse into one which should do a bit more.

smustgrave’s picture

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

Combining also seems to work, lets ship it.

mstrelan’s picture

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

Left a question on the MR

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Maybe we should open a policy ticket to discuss that?

Other consolidated tests seem to following the same
https://git.drupalcode.org/project/drupal/-/commit/24b7c3ffa3a759d7b6b19...

While others were changed
https://git.drupalcode.org/project/drupal/-/commit/3a2d8705b21a1bfeabd65...

I assume we will land on changing and prefixing with doTest so maybe we can do here too?

catch’s picture

catch
he/him
Primary language English
commented

I think protected doTest makes it more obvious what's happening if you just scan the file.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
mstrelan’s picture

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

I think this is fine now and matches other tests in core.

I'm not bothered enough to open a coding standards ticket for it, but if one were so inclined I think something as simple as "Only use test method prefix for public test cases" would suffice. I'm sure it would be fairly straightforward to write a sniff for this too.

godotislate’s picture

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

1 Q on the MR.

catch’s picture

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

That was cruft for this issue, moved to #3614817: Reduce functional test job CPU and concurrency where it belongs. Good spot.

  • godotislate committed 729b7c3e on 11.x 
    task: #3614518 Remove unnecessary requests and consolidate test methods...

  • godotislate committed 510a7a22 on main 
    task: #3614518 Remove unnecessary requests and consolidate test methods...
godotislate’s picture

godotislate
he/him
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 510a7a2 to main and 729b7c3 and 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.

Status: Fixed » Closed (fixed)

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