Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

Chore: Build out AST comparison tests and categorize issues #358

Merged
merged 1 commit into from
Aug 18, 2017

Conversation

JamesHenry
Copy link
Member

@JamesHenry JamesHenry commented Aug 16, 2017

Ok... This has been a huge undertaking but I have made a lot of progress! 🙌

All of the last few PRs from me have come about because of my work on this branch.

The new known-issues.js file captures all of the digging I had to do when encountering AST differences on JavaScript constructs between the two parsers. In many cases, I also researched how the other major parsers (espree, acorn etc) would interpret the code and noted the behaviour in the file. It's often frustratingly inconsistent 😅

Other Important points:

  • All the fixtures from the existing unit tests in the repo are now either having their ASTs compared between typescript-eslint-parser and babylon, or if it is not possible, having their issues clearly categorized within known-issues.js.
  • The one big exception to this is the contents of fixtures/typescript/ - this is obviously the key directory to get sorted in terms of typescript-eslint-parser + babylon-typescript-plugin, but it makes sense to leave any work in that area out of this PR, as there will likely be many breaking changes to come.
  • Functionalitiy wise within the AST comparison spec.js file, fixture patterns can now also provide parser option overrides, necessary for some babylon config which cannot be inferred, as described inline in the comments
  • The special parse() function used in the comparison tests now always returns a consistent object (it will no longer throw on parse error), because there are many cases in which we expect both parsers to error based on the given source and we need to compare their erroring behaviour.
  • The reference to JSX files with known issues (for the TypeScript compiler, and therefore typescript-eslint-parser) have been centralized so that it can be shared across both unit and AST comparison testing
  • Two more of our fixtures had invalid code in them, so they are fixed as part of this

We now have ~350 comparison tests passing:
screen shot 2017-08-16 at 17 19 24

@DanielRosenwasser @Andy-MS after this PR goes in we should be completely freed up to dig into TS-specific AST differences! 🚀

@JamesHenry JamesHenry requested review from hzoo and a team August 16, 2017 16:41
@eslintbot
Copy link

Thanks for the pull request, @JamesHenry! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@JamesHenry JamesHenry merged commit f6e56b3 into master Aug 18, 2017
@JamesHenry JamesHenry deleted the expand-ast-comparison branch August 18, 2017 12:45
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