-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
move GLS tests to jest, add codecov #871
Conversation
acao
commented
Jun 20, 2019
•
edited
Loading
edited
- - jest for GLS repositories
- - introduce codecov
- - per subpackage tests/test coverage
1ca021d
to
5bb316e
Compare
Codecov Report
@@ Coverage Diff @@
## master #871 +/- ##
=========================================
Coverage ? 33.78%
=========================================
Files ? 79
Lines ? 3481
Branches ? 753
=========================================
Hits ? 1176
Misses ? 1814
Partials ? 491 Continue to review full report at Codecov.
|
c779066
to
e244de9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please pull the codecov part out into a separate PR. To make this PR easier, you could consider running mocha and jest alongside each other (i.e. jest runs .test.js tests, while mocha runs -spec.js tests). That will avoid a big PR like this & you can merge some of your progress without having to figure out every detail :)
packages/codemirror-graphql/src/variables/__tests__/hint-test.js
Outdated
Show resolved
Hide resolved
we actually already have jest and mocha running side by side! |
as per codecov, we are in progress working with the codecov folks to be able to have labels for subpackages, so that the LSP libs and the codemirror mode don't look bad just because graphiql is lacking in coverage. good idea to remove codecov from the scope. and yeah, i felt like i was so close with codemirror-graphql, but it seems that codemirror + jsdom + jest is not an easy mix, according to my research. |
@ganemone see here, we will not be able to replace mocha entirely yet |
I can clean this PR up tonight after work |
161f4fc
to
cafb5ae
Compare
@Neitsch should be good to go. Left code-mirror tests as is, but changed the configuration to make codemirror-graphql the only mocha harnessed subpackage. Left codecov in place for now. Converted graphiql to jest.expect. Did not worry about converting enzyme yet, that can come in a different PR. |
@acao, looks like Travis is not passing. Would you mind checking on that first? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you forgot to convert a couple assertions :)
23504aa
to
e82a942
Compare
@Neitsch this happens every time i rebase! I think its because new tests keep getting added upstream using mocha |
- added fix and comment for new failing tests - added assertions for new tests for Jest - switched back to global jest run, 12.8s for all but codemirror! woo! - make sure mocks arent being built by babel as well (fixes warning)
Nice! |
- added fix and comment for new failing tests - added assertions for new tests for Jest - switched back to global jest run, 12.8s for all but codemirror! woo! - make sure mocks arent being built by babel as well (fixes warning)
- added fix and comment for new failing tests - added assertions for new tests for Jest - switched back to global jest run, 12.8s for all but codemirror! woo! - make sure mocks arent being built by babel as well (fixes warning)
- added fix and comment for new failing tests - added assertions for new tests for Jest - switched back to global jest run, 12.8s for all but codemirror! woo! - make sure mocks arent being built by babel as well (fixes warning)
- added fix and comment for new failing tests - added assertions for new tests for Jest - switched back to global jest run, 12.8s for all but codemirror! woo! - make sure mocks arent being built by babel as well (fixes warning)