Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
javascript
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
17 Feb 2022 at 11:28 UTC
Updated:
11 Oct 2023 at 17:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
nod_Current command doesnt support the confirm step that exists when enabling a module with dependencies or a experimental module
Comment #5
jungleAdded an extra parameter to drupalInstallModule for #2
Comment #6
smustgrave commentedChange looks good. Will admit not sure how to test but assume the committer know more of nightwatch then myself.
Comment #7
lauriiiSorry, 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/moduleswithin 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.
Comment #8
star-szrSeems like we should also update https://www.drupal.org/node/3264978 since we are changing the parameters to add a new optional parameter.
Comment #9
star-szrSeems like this may not work for test modules (or only for some test modules?) based on my local testing, but it's a start.
Comment #10
star-szrThis should help a bit.
Edit: Wrong nid on the interdiff filename, but the contents are correct.
Comment #11
smustgrave commentedSeemed to have caused a number of errors
https://dispatcher.drupalci.org/job/drupal_patches/172509/console
Comment #12
star-szrThanks @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.
Comment #13
star-szrIn 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.
Comment #14
star-szrFeedback on this, please.
Comment #15
star-szrSee 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?Comment #16
star-szrI 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.
Comment #17
star-szrFrom 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.
Comment #18
star-szrComment #19
star-szrNow 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.
Comment #20
smustgrave commentedChange looks good. Kinda new with the nightwatch stuff but looks good.
Only moving to NW for change record updates per #8
Comment #21
star-szrThanks @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.
Comment #23
star-szrDraft 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).
Comment #24
star-szrComment #25
smustgrave commentedRan tests for 11.x and all green.
New CR reads fine to me.
Comment #26
lauriiiCommitted 2257603 and pushed to 11.x. Thanks!