-
Notifications
You must be signed in to change notification settings - Fork 37
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(exit): non-zero exit code for getUnusedRules #34
Conversation
use -n|no-error to suppress fixed #31
Current coverage is
|
process.argv = process.argv.slice(0, 2) | ||
}) | ||
|
||
afterEach(function() { | ||
console.log = consoleLog // eslint-disable-line no-console | ||
process.exit = processExit | ||
// purge yargs cache | ||
delete require.cache[require.resolve('yargs')] |
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.
Shouldn't proxyquire
take care of the cache?
From its docs:
// ensure we don't get any module from the cache, but to load it fresh every time
var proxyquire = require('proxyquire').noPreserveCache();
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.
Respectively, extend the stub with
'yargs': {
'@global': true
}
?
At the end of the day, if it works (and it does), I'm fine with it. Just asked for clarification ...
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.
Are you suggesting something like below?
var assert = require('assert')
-var proxyquire = require('proxyquire')
+var proxyquire = require('proxyquire').noPreserveCache();
var sinon = require('sinon')
var consoleLog = console.log // eslint-disable-line no-console
@@ -19,6 +19,9 @@ var stub = {
getUnusedRules: getUnusedRules,
}
},
+ 'yargs': {
+ '@global': true
+ }
}
describe('bin', function() {
@@ -36,8 +39,6 @@ describe('bin', function() {
afterEach(function() {
console.log = consoleLog // eslint-disable-line no-console
process.exit = processExit
- // purge yargs cache
- delete require.cache[require.resolve('yargs')]
})
it('option -c|--current', function() {
could not get it working, looks like yargs
is cached after the first require
. Let me know if I have mis interpreted.
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.
Yep, that is exactly what I thought of. I just remembered, that I had an issue with the caching behavior in another case ... Let's forget this. The explicit deletion from require.cache
seems to work, so let's not overcomplicate this.
(I have to dig into this deeper some time, there has to be a way to accomplish this with proxyquire's own means... if not, it has to be a bug.)
Is it good to merge then? |
LGTM! 👍 |
use -n|no-error to suppress
fixed #31
/ @ta2edchimp