Problem/Motivation

Discovered whilst working on #2980750: [random test failure] Random failure in CommentStatisticsTest:

Currently the functional tests in the comment module use a lot of login/logout as an admin user solely to change settings.
However the methods that are used to change settings don't require admin permissions.

The change-a-comment-setting methods that are frequently surrounded by admin log in/out and can do without that are:

\Drupal\Tests\comment\Functional\CommentTestBase::setCommentAnonymous()
\Drupal\Tests\comment\Functional\CommentTestBase::setCommentPreview()
\Drupal\Tests\comment\Functional\CommentTestBase::setCommentForm()
\Drupal\Tests\comment\Functional\CommentTestBase::setCommentSubject()
\Drupal\Tests\comment\Functional\CommentTestBase::setCommentSettings()
user_role_change_permissions()

Steps to reproduce

Proposed resolution

Let's not log in/out as an admin user when we can change settings without needing admin power.
This should shave off a few seconds when running the tests.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 3368175-nr-bot.txt90 bytesneeds-review-queue-bot

Issue fork drupal-3368175

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

Spokje created an issue. See original summary.

Spokje opened merge request !4218

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje
spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Seeing as that didn't break any tests seems like a safe change.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3368175-nr-bot.txt90 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 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.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Reviewed & tested by the community

Rebased, back to RTBC

lauriii made their first commit to this issue’s fork.

  • lauriii committed 4bfd55f3 on 11.x 
    Issue #3368175 by Spokje: Minimize login as admin user to change...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed 4bfd55f and pushed to 11.x. Thanks!

lauriii closed merge request !4218

spokje’s picture

spokje
Primary language Dutch
commented

Thanks @lauriii for the complementary reroll.
_tips hat_

Status: Fixed » Closed (fixed)

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