Update deps, fix tests

[ljharb]

master is broken. This PR updates dependencies, but it's still broken.

I suspect that fixing the tests will make this PR pass as well; I'm not clear, however, on how to do that.

[coveralls]

Coverage decreased (-0.05%) to 97.306% when pulling 64d9be7 on ljharb:travis into 1cd82eb on benmosher:master.

[benmosher]

Bizarre disjoint test failure sets depending on which Node/ESLint pairing you look at in the Travis results. Almost stranger still that for ESLint 4 + Node 8&10, all tests pass.

I want to get back involved and start cleaning up soon. Hopefully I can take a look this week. I remember there being a TypeScript catch-22 but it's been months since I've looked at it. 😞

[ljharb]

We shouldn't need to drop eslint 2/3; can we instead skip the TS tests on those eslint versions?

[benmosher]

was curious what your reaction would be. yeah, I guess that is doable (to skip TS tests for v2/3). I had that briefly locally but I didn't love explicitly coupling the test content to ESLINT_VERSION and it's not obvious to me what alternative there is?

[ljharb]

We could explicitly uninstall the typescript parser in travis.yml in v2 and v3, and then auto skip typescript tests whenever the parser isn’t try/catch requireable?

[benmosher]

ooo that is an interesting idea.

I just pushed with them wrapped in a skipESLints helper and it works as well as expected.

things still failing:

• the deprecated rule is broken with ESLint 5 on account of Breaking: remove attachComment option eslint/js#394
• no-amd + no-commonjs seem to be totally busted for reasons I don't yet understand. maybe something local, will see what happens on CI I bumped babel-eslint and that did it. something with ESLint 5 compatibility as well, I'm guessing (nothing passes the first check for module scope). so I reverted it back to babel-eslint v8

[benmosher]

another idea: more travis-set versions of packages in the build matrix.

i.e.

• node4 eslint2 babel-eslint@whatever ts-eslint-parser@whatever
• node6 eslint3 babel-eslint@whatever+2 ts-eslint-parser@whatever+4
  ...

[ljharb]

It makes defining the matrix more annoying, but that’s a much better problem to have than “not testing something” or “tests failing” :-) I’m all for it

[benmosher]

leaving myself a reminder to use these to fix deprecated: eslint/js#394 (comment)

[benmosher]

no-deprecated fixed. definitely seems like an expanded build matrix is in order. seems like the typescript parser is allergic to Node <= 6 now too

[benmosher]

getting different results on Travis than I am seeing locally. in particular, I don't see the same Node <= 6 failures so not sure what's going on there.

[benmosher]

alright, winnowed down to just ESLint 4 / Node 4 matrix entry failing, still due to some nonsense with the typescript stuff.

I'm inclined to drop that entry as it doesn't seem super high-value or high-risk and this whole thing is blocking merging PRs / releasing features. do you want to take a look first @ljharb?

[ljharb]

I’d say it’s fine to temporarily add it to allowed failures, but continue running it and pursue fixing it separately?

[benmosher]

Ahhhh great idea, building now!

[benmosher]

Thanks for starting this branch, @ljharb! I’m excited to have master clean to start merging and shipping PRs again!

[ljharb]

I'm going to merge even though coverage went down slightly.