drush en incorrectly reports that it enabled new dependency modules

[rajeshreeputra]

No description provided.

[weitzman]

Would be good to add a test for this, or add to an existing test

[rajeshreeputra]

can this be tested using test
drush/tests/functional/PmEnDisUnListInfoTest.php

[weitzman]

Yes, that is a good place to add testing for this.

[rajeshreeputra]

is this not already covered in drush/tests/functional/PmEnDisUnListInfoTest.php at line number 40

        // Test pm-enable enables a module, and pm-list verifies that.
        $this->drush('pm-enable', ['drush_empty_module']);
        $this->drush('pm-list', [], ['status' => 'enabled']);
        $out = $this->getOutput();
        $this->assertStringContainsString('drush_empty_module', $out);

[weitzman]

Not covered enough. If bug were already covered, the test would have been failing for years.

[rajeshreeputra]

I'm sorry @weitzman, I have not written any tests before, trying to write one first time here. adding code snippet I tried to write for this please let me know if this is good. wee need to add this at line number 47 in drush/tests/functional/PmEnDisUnListInfoTest.php. This will get all dependency for the already enabled module, then it will check if each dependent module is enabled or not using pm-list command.

// Test pm-enable enables a dependent module for already enabled module , and pm-list verifies that.
       $list_of_dependent_modules = $this->addInstallDependencies('drush_empty_module');
       $dependent_modules = ['!list' => implode(', ', $list_of_dependent_modules)];
       foreach($dependent_modules as $dependent_module){
         $this->drush('pm-enable', [$dependent_module]);
         $this->drush('pm-list', [], ['status' => 'enabled']);
         $out = $this->getOutput();
         $this->assertStringContainsString($module_dependency, $out);
       }

We need to use below class as well to use addInstallDependencies() function.
use Drush\Drupal\Commands\pm;

[rajeshreeputra]

Test added to verify that dependent module gets enabled.

[weitzman]

The test seems not to cover the scenario thats added here

[weitzman]

Should be. a logger msg of severity=notice