On #2391909: Drush integration I added some Drush commands.

Right now they don't have any tests. They should.

I'd either need to figure out how to do Drush testing, or I guess I could just do a module_load_include(), load the Drush command file, and at least test that the functions return the correct values.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

jhodgdon’s picture

Version: » 8.x-1.x-dev
jhodgdon’s picture

Component: Code » Drush commands
TravisCarden’s picture

I had to figure this out for Shunt module, @jhodgdon. You can look at what I came up with, in case its of any help to you. @moshe-weitzman confirmed that I did things correctly.

Disclaimer: I did this months and months ago and haven't run it recently, so your mileage may vary. I hope it helps!

jhodgdon’s picture

Thanks! I'll take a look when I have time to get back to this issue.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon

A bug came up in the diff command... really should make these tests!

jhodgdon’s picture

Status: Active » Needs review
FileSize
9.14 KB

I decided the easiest thing was to add tests for the Drush functions to ConfigUpdateTest. I had to make a small change to the drush diff function so that it returned something rather than just printing the output. I tested this Drush command manually and it still works correctly with this change.

This test unfortunately uncovered a Core bug:
#2824410: DiffFormatter component class has leak from core class
So I am attaching a patch here but it will be problematic to commit it until the Core bug is fixed, because the tests will probably fail with 4 exceptions. Sigh. Well, let's see. If the tests pass here I can commit the patch.

Status: Needs review » Needs work

The last submitted patch, 7: 2394603-drush-tests-7.patch, failed testing.

jhodgdon’s picture

Yup, that's the core bug. Well, we'll just have to wait until that gets fixed to commit this. Anyway, the Drush tests pass other than this Core exceptions bug.

jhodgdon’s picture

Status: Needs work » Needs review
FileSize
9.11 KB

Oh. I need to take out the drush.print if I'm returning the values, or it gets double-printed.

Status: Needs review » Needs work

The last submitted patch, 10: 2394603-drush-tests-8.patch, failed testing.

jhodgdon’s picture

The core bug was fixed; retesting...

jhodgdon’s picture

Status: Needs work » Needs review

  • jhodgdon committed 479ede2 on 8.x-1.x
    Issue #2394603 by jhodgdon: Add tests for Drush commands
    
jhodgdon’s picture

Status: Needs review » Fixed

Yeah, the tests passed this time! Committing these new tests.

Status: Fixed » Closed (fixed)

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