-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Conversation
@marcelgerber Looks good. Before I merge it, can you add some unit tests for less and scss? |
Tests added. I hope it's ok to only test SCSS as LESS testing is more difficult than expected, since it requires the |
|
||
runs(function () { | ||
tearDownTests(); | ||
setupTests(testScssPath); |
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.
@marcelgerber Can we make some changes so that we don't have a dependency between test cases? How about removing beforeFirst()
and renaming afterLast()
to afterEach()
so that we don't need to call tearDownTests()
here? Then, we can call setupTests()
individually for each test case.
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.
Done.
I actually tried that before and it didn't work. Don't know what I did wrong :)
tearDownTests(); | ||
}); | ||
|
||
it("should hint for background-image: url()", function () { | ||
it("should hint for background-image: url() in CSS", function () { | ||
setupTests(testCssPath); |
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.
I think it's safer to wrap it with runs(function () { ...});
.
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.
Done.
Thanks @marcelgerber. Merging now. |
Enable UrlCodeHints for LESS/SCSS
For #9086