Skip to content
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

Add testing #1

Merged
merged 1 commit into from Dec 24, 2016
Merged

Add testing #1

merged 1 commit into from Dec 24, 2016

Conversation

vjeux
Copy link
Contributor

@vjeux vjeux commented Dec 23, 2016

  • This brings in the flow test suite that contains a ton of JavaScript parsing edge cases
  • This creates snapshot tests using the pretty printer for all of them
  • If uncomment RUN_AST_TESTS line in tests/run_specs.js, it checks ast(pretty_print(x)) == ast(x). Right now, "178 failed, 197 passed, 375 of 377 total". So half of the tests are not passing, most of them are crashes and many of the rest are subtle issues.

- This brings in the flow test suite that contains a ton of JavaScript parsing edge cases
- This creates snapshot tests using the pretty printer for all of them
- If uncomment `RUN_AST_TESTS` line in `tests/run_specs.js`, it checks ast(pretty_print(x)) == ast(x). Right now, "178 failed, 197 passed, 375 of 377 total". So half of the tests are not passing, most of them are crashes and many of the rest are subtle issues.
@vjeux
Copy link
Contributor Author

vjeux commented Dec 23, 2016

For the errors, adding

String.prototype.join = function(arr) {
  return arr.join(this);
}

seems to be making a lot go away. Is this a new thing inside of node 7? (I'm using 6.9 right now)

@jlongster
Copy link
Member

Wow, this is awesome, thanks so much!

Is this a new thing inside of node 7?

No, this is a difference with the old recast API and the new refactoring. Recast's old printer generated a Lines object which was its implementation of keeping track of string as they were generated. That object has a join method, and several places of the code haven't been converted to the new algorithm and still expect a Lines object to exist.

Now many of those places get a raw string (from the new algorithm) and are trying to join on it, which is why that seems to fix it (but it doesn't really :)). Many places are trivial to fix. If it does thing.join(","), you just need to change it to be join(",", thing). There are probably subtleties though; usually you want to make a few other tweaks when updating the old code.

You can search for where group and line are used to find several examples of using the new API.

This test suite is awesome and is a perfect way to track the refactoring. We just need to get all these tests to pass! I've been able to refactor a ton very quickly and I'm optimistic it shouldn't be hard to finish all the node types.

"glob": "^7.1.1"
"glob": "^7.1.1",
"jest": "^18.0.0",
"flow-parser": "^0.37.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this is here? I don't see it required anywhere

@jlongster
Copy link
Member

Going ahead and merging it, can't really look through all of it as it's so big but I think it's a good idea trying to get this test suite running. Thanks!

@jlongster jlongster merged commit cfa14f0 into prettier:master Dec 24, 2016
josephfrazier pushed a commit to josephfrazier/prettier that referenced this pull request May 19, 2017
@ryami333 ryami333 mentioned this pull request Oct 13, 2017
azz added a commit that referenced this pull request Nov 7, 2017
* Sync README -> docs/options.md

* Sync README -> docs/*.md

* Misc fixups

* Remove markdown-toc

* Remove insert-pragma from ToC

* Never again!

* Move all docs to ./docs

* Remove yarn toc

* Fix inter-doc links

* Fix links in footer

* Clean up README.md

* Add basic description to README.md

* Use flat badges

* Move editor guides to website

* Improve prettier-ignore docs

* Fixup bad find/replace

* Add JSON to README

* Fix custom parser API link

* Fixup GitHub centering, add downloads badge

* Add 1.8 docs

* docs(website): mention markdown on homepage (#1)

* Add intro

* Add watching-files.md

* Fix markdown syntax highlighting

* Switch back to .md links
czosel pushed a commit to czosel/prettier that referenced this pull request Dec 24, 2017
Class: Add support for abstract, final, extends, implements
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 21, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants