Travis CI: Add Node.js 12.

XhmikosR commented 5 years ago

Apparently this is broken due to source-map-support.

C:\Users\xmr\Desktop\bootlint>npm ls source-map-support
bootlint@0.16.6 C:\Users\xmr\Desktop\bootlint
+-- nodeunit@0.11.3
| `-- tap@12.7.0
|   +-- source-map-support@0.5.12  deduped
|   `-- ts-node@8.3.0
|     `-- source-map-support@0.5.12  deduped
`-- terser@4.1.2
  `-- source-map-support@0.5.12

Which means that bootlint itself should work fine on Node.js 12 since these are devDependencies. But it'd be nice if we could sort this out and actually test on Node.js 12 ourselves.

XhmikosR commented 5 years ago

@Herst @Johann-S how difficult would be to switch to another testing framework?

I'll try one more time in the next days but I doubt anything will change, so we might need to start looking into switching to a more maintained framework.

Herst commented 5 years ago

how difficult would be to switch to another testing framework?

Depends on the framework really… Generally I am all in favour of switching to something more maintained.

Johann-S commented 5 years ago

I think it's a good move to move away from nodeunit there are a lot of maintained framework

XhmikosR commented 5 years ago

Actually, maybe we are doing something wrong in the cli test? I mean, terser is using this package and they do test Node.js 12.x.

XhmikosR commented 5 years ago

Also note that the CLI tests fail on GitHub Actions CI. I think we are doing something wrong there.

cli_test
TypeError: process.stdin.setEncoding is not a function
✔ bootlint - Disable tags
    at /home/runner/work/bootlint/bootlint/src/cli.js:55:27
    at Promise._execute (/home/runner/work/bootlint/bootlint/node_modules/bluebird/js/release/debuggability.js:313:9)
    at Promise._resolveFromExecutor (/home/runner/work/bootlint/bootlint/node_modules/bluebird/js/release/promise.js:488:18)
    at new Promise (/home/runner/work/bootlint/bootlint/node_modules/bluebird/js/release/promise.js:79:10)
    at handleStdin (/home/runner/work/bootlint/bootlint/src/cli.js:48:16)
    at /home/runner/work/bootlint/bootlint/src/cli.js:89:44
    at Array.forEach (<anonymous>)
    at module.exports (/home/runner/work/bootlint/bootlint/src/cli.js:88:18)
    at Object.Disable tags (/home/runner/work/bootlint/bootlint/test/cli_test.js:84:9)
    at /home/runner/work/bootlint/bootlint/node_modules/nodeunit/lib/core.js:232:20
    at /home/runner/work/bootlint/bootlint/node_modules/nodeunit/deps/async.js:168:13
    at /home/runner/work/bootlint/bootlint/node_modules/nodeunit/deps/async.js:131:25
    at /home/runner/work/bootlint/bootlint/node_modules/nodeunit/deps/async.js:165:17
    at /home/runner/work/bootlint/bootlint/node_modules/nodeunit/deps/async.js:463:34
    at Object.setUp (/home/runner/work/bootlint/bootlint/test/cli_test.js:48:9)
    at /home/runner/work/bootlint/bootlint/node_modules/nodeunit/lib/core.js:260:35

XhmikosR commented 4 years ago

So, it seems the issue we have with Node.js 12.x and also with GitHub Actions CI stems from these lines:

https://github.com/twbs/bootlint/blob/af54e7a7c841acbd8c73c66d953123987bf8e570/test/cli_test.js#L77-L82

If I remove them, then it works.

I'm not sure what is the proper way to replace this functionality. @tclindner do you have any ideas?

XhmikosR commented 4 years ago

NVM, I finally figured it out. It's a bug in rewire, see https://github.com/jhnns/rewire/pull/167. I will try switching to a fork we control until this is fixed upstream.

twbs / bootlint

Travis CI: Add Node.js 12. #447