maxtaco / coffee-script

IcedCoffeeScript
http://maxtaco.github.com/coffee-script
MIT License
728 stars 58 forks source link

eval.coffee and repl.coffee tests failing on node v.4.1.1 #163

Closed zapu closed 7 years ago

zapu commented 8 years ago

There are certain issues with (I believe) newer node.js versions regarding repl and eval. They result in those tests failing. repl.coffee problems are harmless, but eval.coffee fails probably show real issue.

Only 646 of 650 came back; some went missing! 
test/eval.coffee (249): CoffeeScript.eval can run in, and modify, a Script context sandbox 
test/eval.coffee (250): CoffeeScript.eval can run in, but cannot modify, an ordinary object sandbox 
test/repl.coffee (586): empty command evaluates to undefined 
test/repl.coffee (592): keeps running after runtime error 
failed 4 and passed 646 tests in 3.39 seconds

Fails in eval are related to node not having vm.Script.createContext anymore but vm.createContext. See fix in coffee-script: https://github.com/jashkenas/coffeescript/commit/8e299b09cc4a16697ea754241cf31972437ec970

First repl fail is due to the fact that empty command does not produce undefined anymore. Second error is because, for some reason, node.js repl will display prompt twice after an error (this can't be observed in terminal because of some line magic - see repl.js and readline.js in node).

» node --version v4.1.1

iced-coffee-script: pulled today, master branch

I can port coffee-script fix for eval and submit a pull request, if that's the procedure here, please let me know. With the second issue, I have no idea how to preserve backwards compatibility. coffee-script does not have this check:

eq 0, output.lastWrite(-2).indexOf 'ReferenceError: b is not defined'

in "keeps running after runtime error" test, so it does not fail there. But it does fail in "empty command evaluates to undefined" just like iced does.

maxtaco commented 8 years ago

yeah, these have been failing for a while. I believe they're fixed in mainline CS. The issue is that I tried to remerge on 1.9 and things got really hairy really quickly. The future might be emitting ES6, which is a much smaller patch, but then I'm transpiling (via babel or some such) for backwards compatibility. So have been a bit blocked as to what to do.

On Fri, Oct 2, 2015 at 10:50 AM, Michał Zochniak notifications@github.com wrote:

There are certain issues with (I believe) newer node.js versions regarding repl and eval. They result in those tests failing. repl.coffee problems are harmless, but eval.coffee fails probably show real issue.

Only 646 of 650 came back; some went missing! test/eval.coffee (249): CoffeeScript.eval can run in, and modify, a Script context sandbox test/eval.coffee (250): CoffeeScript.eval can run in, but cannot modify, an ordinary object sandbox test/repl.coffee (586): empty command evaluates to undefined test/repl.coffee (592): keeps running after runtime error failed 4 and passed 646 tests in 3.39 seconds

Fails in eval are related to node not having vm.Script.createContext anymore but vm.createContext. See fix in coffee-script: jashkenas@8e299b0 https://github.com/jashkenas/coffeescript/commit/8e299b09cc4a16697ea754241cf31972437ec970

First repl fail is due to the fact that empty command does not produce undefined anymore. Second error is because, for some reason, node.js repl will display prompt twice after an error (this can't be observed in terminal because of some line magic - see repl.js and readline.js in node).

» node --version v4.1.1

iced-coffee-script: pulled today, master branch

I can port coffee-script fix for eval and submit a pull request, if that's the procedure here, please let me know. With the second issue, I have no idea how to preserve backwards compatibility. coffee-script does not have this check:

eq 0, output.lastWrite(-2).indexOf 'ReferenceError: b is not defined'

in "keeps running after runtime error" test, so it does not fail there. But it does fail in "empty command evaluates to undefined" just like iced does.

— Reply to this email directly or view it on GitHub https://github.com/maxtaco/coffee-script/issues/163.

zapu commented 8 years ago

Oh, I see. Thanks for getting back to me!

maxtaco commented 8 years ago

No worries, sorry for this bug....

On Fri, Oct 2, 2015 at 3:33 PM, Michał Zochniak notifications@github.com wrote:

Oh, I see. Thanks for getting back to me!

— Reply to this email directly or view it on GitHub https://github.com/maxtaco/coffee-script/issues/163#issuecomment-145133553 .

zapu commented 8 years ago

It does not really affect me (or any other iced "consumer", I think), I just wanted to try to get my hands dirty with compilers again. I've tried to submit some patches in the past but you've been generally much faster in fixing things than me in diagnosing them :)