JamesHenry / typescript-estree

:sparkles: A parser that converts TypeScript source code into an ESTree-compatible form
https://jameshenry.blog
Other
84 stars 13 forks source link

test: add test cases for broken converting #123

Closed armano2 closed 5 years ago

armano2 commented 5 years ago

by reviewing test results (without range checks) i found out few issues with how we are converting nodes.

this PR adds test scenarios and informations about bugs.

prettier.io/playground

codecov[bot] commented 5 years ago

Codecov Report

Merging #123 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #123   +/-   ##
=======================================
  Coverage   93.34%   93.34%           
=======================================
  Files           8        8           
  Lines        1398     1398           
  Branches      324      324           
=======================================
  Hits         1305     1305           
  Misses         51       51           
  Partials       42       42

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4315f94...5189d66. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #123 into master will increase coverage by 0.14%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
+ Coverage   93.34%   93.49%   +0.14%     
==========================================
  Files           8        8              
  Lines        1398     1398              
  Branches      324      324              
==========================================
+ Hits         1305     1307       +2     
+ Misses         51       50       -1     
+ Partials       42       41       -1
Impacted Files Coverage Δ
src/convert.ts 94.07% <0%> (+0.25%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4315f94...8166de8. Read the comment docs.

armano2 commented 5 years ago

i'm going to refactor generation of constructors, there is some dead code there, and we have some bugs with them... https://codecov.io/gh/JamesHenry/typescript-estree/src/master/src/convert.ts#L1063 we have tests for this case, but it looks like typescript had some changes with this. and this code is no longer executed, even after i added some additional tests for constructor with type parameters

anyway i don't like how it's generated

armano2 commented 5 years ago

@JamesHenry anyway, can you enable github check for travis and codecov instead of comment. example

https://blog.github.com/2018-05-07-introducing-checks-api/

JamesHenry commented 5 years ago

@armano2 is this ready for review?

Regarding the codecov setup - I cannot find any reference to it supporting the (relatively new) GitHub Checks API. Can you point me to an example?

armano2 commented 5 years ago

https://github.com/settings/installations/49856

image

you just have to give it right to do so

armano2 commented 5 years ago

at some point you added it, but it got broken: https://github.com/JamesHenry/typescript-estree/pull/114/checks

armano2 commented 5 years ago

and about those test scenarios, yea, i think that's all for constructors, i finished watching movie and i will start fixing it

JamesHenry commented 5 years ago

I cannot explain that - I didn't doing anything differently and it has those permissions already...

screenshot 2019-01-13 at 11 48 21
JamesHenry commented 5 years ago

Prettier uses codecov, and it doesn't show up as a status check there either: https://github.com/prettier/prettier/pull/5723/checks

Like I say, I cannot find an example of it mentioned in the docs

JamesHenry commented 5 years ago

Checks is not a separate permission, as you can see - it is included with commit statuses

armano2 commented 5 years ago

i checked codecov in my old project and its looks like checks are not working fully yet https://github.com/armano2/freemarker-parser/runs/49423953

JamesHenry commented 5 years ago

:tada: This PR is included in version 18.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: