chore(test): Allow running of tests in watch mode#244
Conversation
|
@Brocco nice work. I just tested this and seems to work ok... so LGTM. |
Brocco
force-pushed
the
test-with-watch
branch
from
9eabc59 to
3ea83ef
Compare
|
@jkuri thanks, was missing a dependency (debug) that the watcher relies upon. Updated |
|
Okay. IMO we should avoid adding additional dependencies if it is not necessary as it increases the |
| off: function() { | ||
| this.watcher.off.apply(this.watcher, arguments); | ||
| }, | ||
| buildOptions: function() { |
There was a problem hiding this comment.
In my defense it came over that way from the ember project...
https://github.com/ember-cli/ember-cli/blob/0414acac6bd704686aba28d8050b5cc459efa4c2/lib/models/watcher.js#L62-L63
There was a problem hiding this comment.
Is the whole file a copy of the ember one? Maybe we can just import it from there then.
There was a problem hiding this comment.
It was not, but by utilizing on per the suggestion of @gkalpak it is and can be removed and re-reference the ember version again. I will update this later today, good catch!
Brocco
force-pushed
the
test-with-watch
branch
4 times, most recently
from
6b0022e to
20463d7
Compare
| return { | ||
| watcher: watcher, | ||
| completion: watcher.then(function() { | ||
| return new Promise(function () {}); // Run until failure or signal to exit |
There was a problem hiding this comment.
This feels wrong to me, but I don't know enough of Watcher to know if that's a common pattern. Could you just confirm in a comment that this promise should never resolve, with a link somewhere (ember github?) where people can refer instead of filling PRs/issues?
There was a problem hiding this comment.
This is pulled from ember, except I am returning the watcher in order to trigger tests (ember reference)
There was a problem hiding this comment.
Could you add a comment with that reference? Thanks!
|
lgtm |
When running `ng test` or `ng test --watch` karma tests are run after each successful build
Brocco
force-pushed
the
test-with-watch
branch
from
20463d7 to
fb28ff6
Compare
|
Still not sure how I feel about this, but I won't block it. LGTM. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
5 participants
When running
ng testorng test --watchkarma tests are run after each successful build