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.

CommentFileSizeAuthor
#10 2394603-drush-tests-8.patch9.11 KBjhodgdon
#7 2394603-drush-tests-7.patch9.14 KBjhodgdon

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Some docs on Drush tests:
https://github.com/drush-ops/drush/blob/master/tests/README.md

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: » 8.x-1.x-dev
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Component: Code » Drush commands
traviscarden’s picture

traviscarden commented

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

jhodgdon
she/her
Primary language English
commented

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

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: Unassigned » jhodgdon

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

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 2394603-drush-tests-7.patch9.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

jhodgdon
she/her
Primary language English
commented
Related issues: +#2824410: DiffFormatter component class has leak from core class

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

jhodgdon
she/her
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 2394603-drush-tests-8.patch9.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

jhodgdon
she/her
Primary language English
commented

The core bug was fixed; retesting...

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
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

jhodgdon
she/her
Primary language English
commented
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.