jsoverson / preprocess

Preprocess HTML, JavaScript, and other files with directives based off custom or ENV configuration
Other
366 stars 80 forks source link

Tests failing on Windows due to new EOL handling #63

Closed Frizi closed 9 years ago

Frizi commented 9 years ago

@include directive is not properly tested or behaves incorrectly. restoreEol is causing a lot of \r\n to be introduced. I tried to link this issue with git's autocrlf setting, but it failed in each case.

Running "jshint:lib" (jshint) task
>> 2 files lint free.

Running "nodeunit:preprocess" (nodeunit) task
Testing preprocess_test.js..........FF............
>> preprocess - @include files
>> Message: Should include files and indent if ending with a newline
>> Error: 'a!foobar!\r\n c' == 'a!foobar!\n c'
>> at Object.exports.preprocess.@include files (test\preprocess_test.js:573:10)
>> at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

>> preprocess - @include files
>> Message: Should include files (js, block)
>> Error: 'a!foobar!Hello js!\r\n !bazqux!c' == 'a!foobar!Hello js!\n !bazqux!c'
>> at Object.exports.preprocess.@include files (test\preprocess_test.js:577:10)
>> at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

>> preprocess - @include files
>> Message: Should include files and indent if ending with a newline (js, block)
>> Error: 'a!foobar!\r\n c' == 'a!foobar!\n c'
>> at Object.exports.preprocess.@include files (test\preprocess_test.js:581:10)
>> at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

>> preprocess - @include-static files
>> Message: Should include-static files and indent if ending with a newline, just like @include
>> Error: 'a!foobar!\r\n c' == 'a!foobar!\n c'
>> at Object.exports.preprocess.@include-static files (test\preprocess_test.js:623:10)
>> at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

>> preprocess - @include-static files
>> Message: Should include files (js, block), but not recursively
>> Error: 'a!foobar!/* @exec hello(\'js\') /\r\n / @include static.txt /c' == 'a!foobar!/ @exec hello(\'js\') /\n / @include static.txt */c'
>> at Object.exports.preprocess.@include-static files (test\preprocess_test.js:627:10)
>> at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

>> preprocess - @include-static files
>> Message: Should include-static files and indent if ending with a newline (js, block), just like @include
>> Error: 'a!foobar!\r\n c' == 'a!foobar!\n c'
>> at Object.exports.preprocess.@include-static files (test\preprocess_test.js:631:10)
>> at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

Warning: 6/213 assertions failed (763ms) Use --force to continue.

Aborted due to warnings.
BendingBender commented 9 years ago

Thanks for reporting! This is great that someone does the testing, I've changed a lot and something had to break. I'll be afk for at least a day. Afterwards I'll take a look at the issue. Am 16.06.2015 14:52 schrieb "Paweł Grabarz" notifications@github.com:

@include directive is not properly tested or behaves incorrectly. restoreEol is causing a lot of \r\n to be introduced. I tried to link this issue with git's autocrlf setting, but it failed in each case.

Running "jshint:lib" (jshint) task

2 files lint free.

Running "nodeunit:preprocess" (nodeunit) task Testing preprocess_test.js..........FF............

preprocess - @include files Message: Should include files and indent if ending with a newline Error: 'a!foobar!\r\n c' == 'a!foobar!\n c' at Object.exports.preprocess.@include files (test\preprocess_test.js:573:10) at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

preprocess - @include files Message: Should include files (js, block) Error: 'a!foobar!Hello js!\r\n !bazqux!c' == 'a!foobar!Hello js!\n !bazqux!c' at Object.exports.preprocess.@include files (test\preprocess_test.js:577:10) at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

preprocess - @include files Message: Should include files and indent if ending with a newline (js, block) Error: 'a!foobar!\r\n c' == 'a!foobar!\n c' at Object.exports.preprocess.@include files (test\preprocess_test.js:581:10) at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

preprocess - @include-static files Message: Should include-static files and indent if ending with a newline, just like @include Error: 'a!foobar!\r\n c' == 'a!foobar!\n c' at Object.exports.preprocess.@include-static files (test\preprocess_test.js:623:10) at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

preprocess - @include-static files Message: Should include files (js, block), but not recursively Error: 'a!foobar!/* @exec hello(\'js\') /\r\n / @include static.txt /c' == 'a!foobar!/ @exec hello(\'js\') /\n / @include static.txt */c' at Object.exports.preprocess.@include-static files (test\preprocess_test.js:627:10) at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

preprocess - @include-static files Message: Should include-static files and indent if ending with a newline (js, block), just like @include Error: 'a!foobar!\r\n c' == 'a!foobar!\n c' at Object.exports.preprocess.@include-static files (test\preprocess_test.js:631:10) at Object.exports.preprocess.setUp (test\preprocess_test.js:37:5)

Warning: 6/213 assertions failed (763ms) Use --force to continue.

Aborted due to warnings.

— Reply to this email directly or view it on GitHub https://github.com/jsoverson/preprocess/issues/63.

Frizi commented 9 years ago

Speaking of tests, what do you think about rewriting this test suite to mocha or jasmine framework? These are much more powerful than nodeunit and will play nicely with multiple editors, IDEs and in particular wallaby.js runner. Also you don't need grunt for that.

BendingBender commented 9 years ago

Well, I like grunt a lot (or gulp). But I'd have nothing against moving to mocha (with chai assertions). Either way, I want to regroup and split the tests into smaller pieces. It's currently quite hard to determine whether a feature is completely tested or not or whether there are redundant tests.

Frizi commented 9 years ago

I have nothing aginst gulp really also :) I'm actually quite involved in task automatization. The point is, there is just no use for it at this point in time. For tests, you have karma, it has its own good CLI.

Gulp will be useful, if code will be generated from a parser generator.

BendingBender commented 9 years ago

Correct me if I'm wrong, but karma is more for frontend testing. It runs all it's tests in different browsers, not in node. It is not a task runner nor a generic test runner. We will either need a simple npm script or a task runner like grunt or gulp. And I personally like task runners because it is quite easy to combine tasks and they usually provide a massive set of plugins for every kind of task.

Frizi commented 9 years ago

Indeed, I was wrong. You can use mocha directly :) You don't need to convince me gulp is cool, I just find it unnecessary for single task. You have npm test for this specific case.