Skip to content
This repository has been archived by the owner on Aug 18, 2021. It is now read-only.

Add eslint tests - Ref #62 #116

Merged
merged 2 commits into from
Jun 8, 2015
Merged

Add eslint tests - Ref #62 #116

merged 2 commits into from
Jun 8, 2015

Conversation

hzoo
Copy link
Member

@hzoo hzoo commented May 30, 2015

Not sure if this is the right way to do this (I think I did the submodule wrong)?
Also not sure about using ; and cd in npm scripts and all that.
I did git submodule add https://github.com/eslint/eslint eslint

npm run prepare-eslint-tests && npm run eslint-tests

@sebmck, @Cellule

Getting 88 errors at the moment.

@hzoo hzoo force-pushed the eslint-tests branch 2 times, most recently from 0e936c7 to 51e4613 Compare May 30, 2015 17:31
@sebmck
Copy link
Contributor

sebmck commented May 30, 2015

cd should be fine. ; on Windows wont be though. Use command && command instead.

@Cellule
Copy link
Contributor

Cellule commented May 31, 2015

you could put npm run prepare-eslint-tests && npm run eslint-tests in an npm script or directly in script test like mocha && npm run prepare-eslint-tests && npm run eslint-tests I don't know if we want to run eslint tests everytime.

Also, I am not particularly familiar with git submodules, so I won't comment on it, but it's seems fine. I am only concern about publishing. Will the folder eslint be publish along with everything else ?

@hzoo
Copy link
Member Author

hzoo commented May 31, 2015

Ok fixed to use && instead of ;. Originally I put it in mocha but I figured it would be better to run it separately until we can get the tests to pass?
Also I don't think submodules are included in the publish? Looks like I need to add git submodule update --init like in babel which has traceur as a git submodule in order for eslint/ to show up anyway.

Output: https://gist.github.com/hzoo/5255716d28a015b3ec37

A lot of the failing tests aren't really anything, probably half of them are '\'with\' in strict mode'
I also see maximum call stack size exceeded when parsing jsx. Not sure how we can get those to pass unless we change babel parser but only when testing those particular tests (strictmode: false) or skip those tests?

@hzoo hzoo mentioned this pull request Jun 1, 2015
@hzoo
Copy link
Member Author

hzoo commented Jun 7, 2015

@sebmck So the eslint submodule won't be automatically be there when users npm install since you have to run git submodule update right? So this should be good to merge? And later we could probably remove the eslint devDependency and just use the submodule?

@sebmck
Copy link
Contributor

sebmck commented Jun 7, 2015

@hzoo You'll haev to add it to .npmignore.

@hzoo
Copy link
Member Author

hzoo commented Jun 7, 2015

Ah ok added @sebmck

@hzoo hzoo changed the title WIP: integrate eslint tests - Ref #62 Add eslint tests - Ref #62 Jun 7, 2015
@hzoo
Copy link
Member Author

hzoo commented Jun 7, 2015

Yeah a lot of errors are due to the tests using with.

AssertionError: Should have no errors but had 1: [ { fatal: true,
    severity: 2,
    message: '\'with\' in strict mode',

Also issue with tests involving jsx

 6) comma-spacing <a>,</a>:
     Maximum call stack size exceeded

I guess we would need to change the parsing options for these tests only (non-strict mode)?

@sebmck
Copy link
Contributor

sebmck commented Jun 7, 2015

The with ones are alright since Babel is strict mode by default so those are correct. Second one is... weird.

@hzoo
Copy link
Member Author

hzoo commented Jun 7, 2015

Yeah really weird - seems to be failing at escope.analyze https://github.com/eslint/eslint/blob/master/lib/eslint.js#L666-L671

hzoo added a commit that referenced this pull request Jun 8, 2015
Add eslint submodule to run eslint tests - Ref #62
@hzoo hzoo merged commit 8b9940e into babel:master Jun 8, 2015
nicolo-ribaudo pushed a commit to babel/babel that referenced this pull request Nov 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants