cleanup mocha implementation #311
Conversation
Codecov Report
@@ Coverage Diff @@
## master #311 +/- ##
===========================================
- Coverage 98.39% 12.34% -86.05%
===========================================
Files 32 32
Lines 561 583 +22
===========================================
- Hits 552 72 -480
- Misses 9 511 +502
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #311 +/- ##
==========================================
+ Coverage 98.47% 98.53% +0.05%
==========================================
Files 32 32
Lines 592 613 +21
==========================================
+ Hits 583 604 +21
Misses 9 9
Continue to review full report at Codecov.
|
43081j
force-pushed
the
mocha-cleanup
branch
3 times, most recently
from
7f358b6
to
4f7773d
Compare
| return null; | ||
| } | ||
|
|
||
| return fs.readFileSync(path.resolve(root, '..', optsPath), 'utf-8'); |
There was a problem hiding this comment.
What if the file does not exist? The exception will break whole parser.
There was a problem hiding this comment.
what would be the correct thing to do? if they pass --opts nonexistent, we should throw or error in some way so they know they've tried to pass a non-existent config.
* Fixes depcheck#287 * Fixes depcheck#241
|
@rumpl mind giving this another quick look? rebased onto master |
| .map(requirePackageName) | ||
| .filter(name => deps.indexOf(name) !== -1); | ||
| .filter((v, k, arr) => arr.indexOf(v) === k) | ||
| .filter(name => deps.includes(name)); |
There was a problem hiding this comment.
this seems weird, maybe it shouldn't be here? it was there before but i wasn't sure why...
There was a problem hiding this comment.
it reads as if it will filter out any dependencies which aren't in our package.json? don't we want to know about those? i left it in because it was there before but seems pretty questionable.
|
I’ll take a look tomorrow, it’s late where I am
Sent from mobile - please excuse tone and brevity
… On Mar 5, 2019, at 8:14 PM, James Garbutt ***@***.***> wrote:
@rumpl mind giving this another quick look? rebased onto master
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
help wantedlabel) #287So this should help out with fixing up the mocha implementation a bit. Did some refactoring, added support for
--reporterand--requirein both opts files and scripts.Wasn't too sure if the tests were good enough so some feedback on them would be appreciated.
If there's anything obvious i've messed up do let me know.