tests: add Node.js 10 - Githubissues

ballercat / walt

:zap: Walt is a JavaScript-like syntax for WebAssembly text format :zap:

https://ballercat.github.io/walt/

MIT License

4.64k stars 122 forks source link

tests: add Node.js 10 #156

Closed DanielRuf closed 5 years ago

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 181

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 99.615%

Totals
Change from base Build 178:	0.0%
Covered Lines:	1896
Relevant Lines:	1896

💛 - Coveralls

DanielRuf commented 5 years ago

And finally we found a fialing build thanks to the addition of Node.js 10.

https://travis-ci.com/ballercat/walt/jobs/151169482

node-sass has to be upgraded.

DanielRuf commented 5 years ago

Seems we caught another issue. Can someone take a closer look?

https://travis-ci.com/ballercat/walt/jobs/151205830

ballercat commented 5 years ago

Thanks for this.

I'm taking a look at the test failure now.

ballercat commented 5 years ago

So I get a related but different error locally, which is odd BUT the issue is with string decoding and thememory.grow() instruction inside the statics spec.

It's also verified by this comment on emscripten issues board https://github.com/kripken/emscripten/issues/6747#issuecomment-400081465

Turns out, growing a memory replaces the internal buffer inside the memory instance. I'm not sure this is documented anywhere either.

ballercat commented 5 years ago

Also, shouldn't node 10 be LTS already? If it's close I'd rather just test against it and not both 8 & 10. I'd like to keep these test pipelines quick.

DanielRuf commented 5 years ago

10 should become LTS this month, see https://github.com/nodejs/Release/blob/master/README.md

DanielRuf commented 5 years ago

But there are always 2 LTS releases / branches and 8 EOL will take a while.