-
Notifications
You must be signed in to change notification settings - Fork 2k
[bug] Gulp test wasn't running server tests [closes #873] #891
Conversation
I can confirm this fixes the issue. @codydaig It looks like the behavior between the Grunt & Gulp test tasks is slightly different. Using Gulp, it appears that the karma task is started before the Mocha task completes. This ends up skewing the reporting output from the tests. Whereas, using Grunt, the Karma task doesn't start until the Mocha task is complete. |
@codydaig It looks like the Gulp test task is running both the Karma & Mocha tests in parallel.. The task defined here https://github.com/meanjs/mean/blob/master/gulpfile.js#L222, can be updated to this
This will run the Mocha task first, and then run the Karma task. Learned something new about |
@mleanos I'm working on refactoring the gulp file to be more inline with the grunt file. This was just a bug fix in the meantime. I'm happy to make the change if thats desired in this PR as well. |
@codydaig I think making that change in this PR is appropriate. Just so that this particular Gulp task outputs the results in a readable manner. |
629a17e
to
2131ccd
Compare
@mleanos Change has been made. |
Perfect. LGTM. Thanks @codydaig |
@rhutchison Any comments on this PR? I think you we're one of the guys pushing for gulp. |
@codydaig regarding what were you looking for feedback from @rhutchison? |
@ilanbiala I'm pretty sure it was @rhutchison that was pushing for the gulp change, and had made some improvements in the gulp file in the past. But I've tested this and it makes it so you can run the tests with gulp, so since ryan doesn't have any feedback, then I think it's good to merge. |
Enable running tests through Gulp closes #873
This PR fixes #873. The models were never getting loaded in the gulp tests so it was freezing on mongoose errors. This loads the models before running the server tests.