-
-
Notifications
You must be signed in to change notification settings - Fork 360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Allow nyc instrument
to instrument code in place
#1149
feat: Allow nyc instrument
to instrument code in place
#1149
Conversation
This change adds the `--in-place` switch to nyc instrument If `--in-place` is specified, the output directory will be set to equal the input directory for the instrument command. This has the effect of replacing any file in the input directory that should be instrumented, with its instrumented counterpart. The command will throw an error if the --delete option is specified. The only way to instrument in place is with the --in-place switch, setting the input and output directories to be the same will not work If `--in-place` is set the instrument command ignores any output directory specified with the command If `--in-place` is set the instrument command will disable the `--complete-copy` switch as it is unnecessary. I've made a few small code improvements to the instrument command spec. I've also added tests to the old integration tests folder, I figured I could put tests here for the time being before they get moved to the new tap format.
Just a quick note that the I'm aware that this breaks the test snapshot and you probably don't want additions to the old integration test file. Since I'm a little low on time at the moment I figured that it'd be good to get this out there and it'd be quicker to make tests for the old integration set that I'm familiar with. Anyway let me know what needs to be done from here, thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick note that the I'm aware that this breaks the test snapshot
Yes please run npm run snap
, commit the changes to tap-snapshots.
you probably don't want additions to the old integration test file.
Don't worry about this, the tests you've added really are not appropriate for snapshot testing. A couple extra tests using the mocha emulation is not a big issue.
Since I'm a little low on time at the moment I figured that it'd be good to get this out there and it'd be quicker to make tests for the old integration set that I'm familiar with. Anyway let me know what needs to be done from here, thanks
thanks for the patch, just a couple of minor issues I mentioned.
Additional note the snapshots getting broken by this test is actually referenced in #1096. I just haven't had time to tweak those tests which use |
The instrument --in-place test now runs in a copy of the instrument in place test directory, this means that running tests locally doesn't change your local environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay of review, once the console output is removed I'll be happy to merge this.
f472e01
to
6b1dbc9
Compare
This PR addresses #1135
This change adds the
--in-place
switch to nyc instrumentIf
--in-place
is specified, the output directory will be set to equal the input directory for the instrument command.This has the effect of replacing any file in the input directory that should be instrumented, with its instrumented counterpart.
The command will throw an error if the --delete option is specified.
The only way to instrument in place is with the --in-place switch, setting the input and output directories to be the same will not work
If
--in-place
is set the instrument command ignores any output directory specified with the commandIf
--in-place
is set the instrument command will disable the--complete-copy
switch as it is unnecessary.I've made a few small code improvements to the instrument command spec.
I've also added tests to the old integration tests folder, I figured I could put tests here for the time being before they get moved to the new tap format (by me or someone else).