improve test coverage by adding more tests #967
Conversation
|
Just wanted to say that we appreciate the pull request :) This will have to wait for a little bit as we would like to get #880 landed first. |
|
@sindresorhus Any timeframe regarding when #880 are going to be merged? Seems stall atm. 3 months since the PR was created. |
|
@dananichev #880 is not happening now after all, so we can go forward with this :) |
| t.end(); | ||
| }); | ||
|
|
||
| test('should return full stack', function (t) { |
There was a problem hiding this comment.
Why should it return full stack?
sindresorhus
changed the title
|
Nice work @dananichev :) |
|
@sindresorhus sorry for my Perfect English skills (c) :) |
| 'at Stub.listOnTimeout (timers.js:119:15)\n' | ||
| ); | ||
|
|
||
| t.true(result.search('ava/cli.js') !== -1); |
There was a problem hiding this comment.
This should use indexOf instead. You're actually not using a regex in the search() method.
| 'at Stub.listOnTimeout (timers.js:119:15)\n' | ||
| ); | ||
|
|
||
| t.true(result.indexOf('ava/cli.js') >= 0); |
|
Added some more nitpick feedback inline. Otherwise looks good :) |
|
The Windows build is failing: https://ci.appveyor.com/project/ava/ava/build/170 |
|
@sindresorhus it seems the problem is somewhat related to sindresorhus/execa#35. At least simptoms are similar (maybe it's just my imagination though): I'm trying to locate the problem behind this. But my VMs are pretty much slow (dunno why though). I will continue to investigate this further from home (with native Windows). |
|
@sindresorhus i hope you are fine with usage of |
| @@ -23,7 +23,7 @@ test('runs the profiler and throws an error when invoked without files to run', | |||
| t.plan(1); | |||
| run() | |||
| .catch(function (err) { | |||
| t.ok(/Specify a test file/.test(err.message)); | |||
| t.ok(/Specify a test file/g.test(err.stderr)); | |||
There was a problem hiding this comment.
There's no point in having the g flag here.
There was a problem hiding this comment.
Fixed it for you when merging.
There was a problem hiding this comment.
One of this CMD+Z's was too much i think.
|
🎉 Looks great now. Thank you @dananichev :) |
#161 tests update.
Also, removed unreachable (?) code from lib/reporters/mini.js