test: add misc. tests to improve coverage#673
Conversation
|
|
||
| test('-nR does not overwrite an existing file at the destination', t => { | ||
| // Create tmp/new/cp/a | ||
| const dest = `${t.context.tmp}/new/cp`; |
There was a problem hiding this comment.
I feel like we should be using path.join to join paths everywhere, but its probably not an issue for now.
There was a problem hiding this comment.
Actually, it looks like one of the mv tests failed on windows for this exact reason. Maybe we should create another issue for this?
There was a problem hiding this comment.
ShellJS should be able to handle paths using /, regardless of which platform we're on
There was a problem hiding this comment.
Yup, looks like we caught a bug. We should probably be converting the output to use a / instead of a \.
1f93c59 to
2d02479
Compare
No change in production logic. This adds missing tests to improve test coverage. This does not change ShellJS behavior at all--all test cases are testing already-working functionality. One test case has been renamed for clarity. For the "omit directory if missing recursive flag" case, we were actually already testing that in another case, but we were testing multiple things in that test case. It's better to test this one error condition explicitly in its own case. When adding real tests for `parseOptions()`, we need to explicitly clear `common.state.error` because we're testing an internal function, not a wrapped command. Partial fix for #671
2d02479 to
34124cd
Compare
Codecov Report
@@ Coverage Diff @@
## master #673 +/- ##
==========================================
+ Coverage 92.68% 93.53% +0.85%
==========================================
Files 33 33
Lines 1176 1176
==========================================
+ Hits 1090 1100 +10
+ Misses 86 76 -10
Continue to review full report at Codecov.
|
No change in production logic.
This adds missing tests to improve test coverage. This does not change
ShellJS behavior at all--all test cases are testing already-working
functionality.
One test case has been renamed for clarity.
For the "omit directory if missing recursive flag" case, we were
actually already testing that in another case, but we were testing
multiple things in that test case. It's better to test this one error
condition explicitly in its own case.
When adding real tests for
parseOptions(), we need to explicitly clearcommon.state.errorbecause we're testing an internal function, not awrapped command.
Partial fix for #671