New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update deps, fix tests #1254

Open
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ljharb
Copy link
Collaborator

ljharb commented Jan 3, 2019

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.

ljharb added some commits Jan 3, 2019

[Dev Deps] update `babylon`, `coveralls`, `eslint-import-resolver-typ…
…escript`, `gulp`, `linklocal`, `nyc`, `redux`, `rimraf`, `sinon`, `typescript-eslint-parser`

@ljharb ljharb requested a review from benmosher Jan 3, 2019

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 3, 2019

Coverage Status

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

@benmosher

This comment has been minimized.

Copy link
Owner

benmosher commented Jan 10, 2019

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. 😞

Ben Mosher Ben Mosher
drop ESLint 2/3 from Travis/Appveyor
because Typescript parser can't deal
@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 15, 2019

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

@benmosher

This comment has been minimized.

Copy link
Owner

benmosher commented Jan 15, 2019

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

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 15, 2019

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?

Ben Mosher Ben Mosher
@benmosher

This comment has been minimized.

Copy link
Owner

benmosher commented Jan 15, 2019

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 eslint/espree#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
Ben Mosher Ben Mosher
ah geez, bumping babel-eslint breaks no-amd/no-cjs
also left my debug console in :-(
@benmosher

This comment has been minimized.

Copy link
Owner

benmosher commented Jan 15, 2019

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
    ...
Ben Mosher Ben Mosher
@ljharb

This comment has been minimized.

Copy link
Collaborator

ljharb commented Jan 15, 2019

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

This comment has been minimized.

Copy link
Owner

benmosher commented Jan 15, 2019

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

Ben Mosher Ben Mosher
repair `no-deprecated` for ESLint* 5
* technically espree v5
- also technically it doesn't support comma-first anymore but that seems reasonable
@benmosher

This comment has been minimized.

Copy link
Owner

benmosher commented Jan 16, 2019

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

Ben Mosher added some commits Jan 17, 2019

Ben Mosher Ben Mosher
dep-time-travel: use older versions of dependencies
for tests against older ESLint versions.
@benmosher

This comment has been minimized.

Copy link
Owner

benmosher commented Jan 17, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment