-
Notifications
You must be signed in to change notification settings - Fork 59
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
Added a test for nested imports #17
Conversation
@@ -0,0 +1,4 @@ | |||
test: |
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.
Please remove the Makefile
, in favour of npm scripts
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.
You can simply update the path in the package.json
"scripts": {
"test": "./node_modules/.bin/mocha./test/test.js"
},
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.
Removed the makefile, didn't think of portability.
I changed the scripts entry to:
"scripts": {
"test": "mocha ./test/test*.js"
}
test.parse-imports.js
didn't run when running npm test
without the * in the test declaration.
Thanks for this @kevin-smets. Please remove the Makefile, it's not portable and we <3 Windows. Npm scripts are the preferred way to do this, and much more portable. |
@@ -10,7 +10,7 @@ | |||
"bin": "./bin" | |||
}, | |||
"scripts": { | |||
"test": "mocha ./test/test.js" | |||
"test": "mocha ./test/test*.js" |
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.
Is the *
a typo?
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.
Ah nevermind I see what's going on
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.
Nope, currently npm test
only runs test.js
. Now running npm test
will run both test.js
and test.parse-imports.js
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.
Nice catch!
Could you please squash the commits into one? Then it's good to go! |
Squashed them 👍 |
Great work @kevin-smets |
Just wanted to add a test for this as I run sass graph over an extensive amount of imports.