Problem/Motivation

#3255809: Add nightwatch tests for toolbar introduces a new drupalInstallModule command which standardizes how modules should be installed in Nightwatch tests.

We have many existing Nightwatch tests that could be using this instead of having their own custom steps for installing modules.

Proposed resolution

Use this API consistently across our Nightwatch tests.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

lauriii created an issue. See original summary.

nod_’s picture

Current command doesnt support the confirm step that exists when enabling a module with dependencies or a experimental module

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

jungle’s picture

Status: Active » Needs review
StatusFileSize
new3.12 KB

Added an extra parameter to drupalInstallModule for #2

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Change looks good. Will admit not sure how to test but assume the committer know more of nightwatch then myself.

lauriii’s picture

Status: Reviewed & tested by the community » Needs work

Sorry, I don't think I did a good job when I filed this issue. I don't think #5 is doing exactly what this issue was intended to solve. This issue was opened to remove instances where we navigate manually to install modules. These instances can be found by searching for /admin/modules within the Nightwatch directory.

We probably need #5 for enabling modules dependencies but we should expand this to remove all of the instances where modules are being installed manually.

star-szr’s picture

Seems like we should also update https://www.drupal.org/node/3264978 since we are changing the parameters to add a new optional parameter.

star-szr’s picture

Status: Needs work » Needs review
StatusFileSize
new12.68 KB
new9.57 KB

Seems like this may not work for test modules (or only for some test modules?) based on my local testing, but it's a start.

star-szr’s picture

StatusFileSize
new13.07 KB
new1.2 KB

This should help a bit.

Edit: Wrong nid on the interdiff filename, but the contents are correct.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative
star-szr’s picture

StatusFileSize
new13.49 KB
new1020 bytes

Thanks @smustgrave!

This may break something else, but trying a different approach since we can't reliably filter by module machine name on /admin/modules.

The test code previously was searching for the proper name of the module, such as "FormAPI Test" or "Tabbing Manager Test" to work around this.

star-szr’s picture

StatusFileSize
new13.7 KB
new1.49 KB

In theory this should pass.

Added a todo to expand the "Testing" group of modules only when it's present, need to dig around to find a good way to handle that.

star-szr’s picture

Status: Needs work » Needs review
StatusFileSize
new13.89 KB
new1.57 KB

Feedback on this, please.

star-szr’s picture

See also #3272727: Nightwatch's drupalModuleInstall() doesn't handle test modules or modules w underscores in machine name (nice issue number!) but I think that may have missed that you can't filter by machine_name? Or maybe something changed elsewhere?

star-szr’s picture

I think I've narrowed it down:

The filtering only fails when the machine name doesn't "match" with the human-readable name.

Examples:

form_test => FormAPI Test (Edit: this one works, actually)
tabbingmanager_test => Tabbing Manager Test

I will try to trace down the code to confirm this.

star-szr’s picture

Issue summary: View changes

From what I can tell, there is an unaddressed case in the module filtering JS. As it stands, filtering by machine name only works if the module description ends with a period or other separator. The module filter does a regex search where the match must occur at the start of a word boundary, and the text it's searching has been concatenated together with no separator.

I tracked down #2978498: Module filter only matches with system name if the description ends in a period which could be revived to fix that problem.

star-szr’s picture

star-szr’s picture

StatusFileSize
new13.07 KB
new1.41 KB

Now that #2978498: Module filter only matches with system name if the description ends in a period is in (thanks @lauriii!) we can go back to the filter approach.

smustgrave’s picture

Status: Needs review » Needs work

Change looks good. Kinda new with the nightwatch stuff but looks good.

Only moving to NW for change record updates per #8

star-szr’s picture

Thanks @smustgrave!

I'm actually not sure the best way to handle the change record, maybe the cleanest way would actually be to create a new change record and cross-reference that with https://www.drupal.org/node/3264978. I'm thinking that this change may not end up being backported so could be confusing for 9.x Nightwatch users.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

star-szr’s picture

Status: Needs work » Needs review

Draft change record: https://www.drupal.org/node/3382659

Once it's published we can update the older change record to link to this one as well and note that it's for 10.x only (at least for now, it could still get backported).

star-szr’s picture

Title: Use Nightwatch drupalInstallModule command » Add optional parameter to Nightwatch drupalInstallModule command to enable module dependencies
Issue tags: -Needs change record updates
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Ran tests for 11.x and all green.

New CR reads fine to me.

lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 2257603 and pushed to 11.x. Thanks!

  • lauriii committed 22576037 on 11.x
    Issue #3264940 by star-szr, jungle, smustgrave, lauriii: Add optional...

Status: Fixed » Closed (fixed)

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