Problem/Motivation

DisplayPathTest::testMenuOptions() and DisplayPathTest::testDefaultMenuTabRegression() enable the menu_ui module, after it is already been enabled by the $modules on the test class. Looking at the original support, this is legacy code that no longer serves a purpose.

Original report

From the documentation of WebTestBase::resetAll:

This method is called by \Drupal\simpletest\WebTestBase::setUp() after enabling the requested modules. It must be called again when additional modules are enabled later.

Proposed resolution

  1. Remove extra instances of menu_ui
  2. Add resetAll after additional modules are installed per API

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#16 2342807-16.patch1.12 KBLendude
#16 interdiff-2342807-13-16.txt759 bytesLendude
#13 2342807-13.patch631 bytesLendude
#12 2342807-12.patch613 bytesLendude
#1 2342807-call-reset-all.patch564 bytesznerol
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

znerol’s picture

znerol CreditAttribution: znerol commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2342807-call-reset-all.patch564 bytes

This patch adds the resetAll() which will result in notices thrown from within the page display.

Status: Needs review » Needs work

The last submitted patch, 1: 2342807-call-reset-all.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +VDC

Just adding tags for now.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Status: Needs work » Needs review
FileSize
2342807-12.patch613 bytes

Obviously this got moved around a bit and changed to a PHPUnit based test. But the docs for resetAll() are still the same, that this should be called after adding any modules.

But doing so, no longer leads to test fails.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Title: DisplayPathTest::testMenuOptions() does not test menu options » DisplayPathTest::testMenuOptions() enables the menu_ui module, when it is already enabled
Version: 8.9.x-dev » 9.1.x-dev
Category: Bug report » Task
FileSize
2342807-13.patch631 bytes

...and the reason for that might be that the menu_ui module already gets enabled by the $modules....so we can just take this out here.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Issue summary: View changes
Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte commented
Status: Needs review » Needs work

Thanks for the patch update.

1) Confirmed the test has the menu_ui in:

  protected static $modules = ['menu_ui'];

so the additional install code isn't necessary.

2) Patch applies cleanly to 9.2 and 9.1.

3) Shouldn't this issue be set to 9.2 and patch tested against 9.2?

4) There is another places menu_ui is installed:

  public function testDefaultMenuTabRegression() {
    $this->container->get('module_installer')->install(['menu_ui', 'menu_link_content', 'toolbar', 'system']);

so this could be removed from the list.

Moving back to needs work for this.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Version: 9.1.x-dev » 9.2.x-dev
Status: Needs work » Needs review
FileSize
interdiff-2342807-13-16.txt759 bytes
2342807-16.patch1.12 KB

Thanks @Kristen Pol. Removed menu_ui there too, but added the resetAll() since that still adds new modules.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte commented
Title: DisplayPathTest::testMenuOptions() enables the menu_ui module, when it is already enabled » DisplayPathTest methods enable menu_ui module when it is already enabled
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Thanks for the update. Marking RTBC based on:

1) Patch applies cleanly to 9.2.

2) Tests pass.

3) Interdiff looks good and fixes the item noticed in #15.

4) I've updated the issue title and summary to reflect the change.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Itty Bitty Byte commented
Issue tags: +Bug Smash Initiative

Since this started as a bug and was triaged by the bugsmash team, tagging this.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Issue tags: +Quickfix

RTBC++, nice one team.

  • catch committed e36b2f2 on 9.2.x 
    Issue #2342807 by Lendude, znerol, Kristen Pol: DisplayPathTest methods...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed e36b2f2 and pushed to 9.2.x. Thanks!

Status: Fixed » Closed (fixed)

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