feat(graphiql): Prettify also formats query variables #991
Conversation
Codecov Report
@@ Coverage Diff @@
## master #991 +/- ##
==========================================
- Coverage 43.14% 43.08% -0.07%
==========================================
Files 64 64
Lines 2934 2943 +9
Branches 635 637 +2
==========================================
+ Hits 1266 1268 +2
- Misses 1400 1406 +6
- Partials 268 269 +1
Continue to review full report at Codecov.
|
|
@benjie should I add some tests to increase coverage?) or I could just remove the code part which rid of unnecessary editors updates |
There was a problem hiding this comment.
Thanks @zouxuoz! Adding tests would be preferably; we have the beginnings of Cypress tests but I'm not sure if they're hooked up to code coverage or not (@acao?). Adding a test like those here: https://github.com/graphql/graphiql/blob/master/packages/graphiql/cypress/integration/init.spec.js would be great, testing the following circumstances:
- Regular prettification: enter ugly query and variables, click button, and check they are prettier
- Noop prettification: enter prettified query and variables, click button, and check that everything's unchanged (and no errors)
- No crash on bad query: enter broken query and ugly variables, click button, query should remain unchanged but variables should be prettier
- No crash on bad variables: enter ugly query and broken variables, click button, query should prettify but variables should remain unchanged.
Thanks so much 🙌
|
|
||
| describe('GraphiQL Prettify', function() { | ||
| it('Regular prettification', function() { | ||
| cy.visit(`/?query=${uglyQuery}&variables=${uglyVaribles}`); |
There was a problem hiding this comment.
Please escape the URLs; it doesn't seem to matter for these tests but it'll help future tests get it right 👍
| cy.visit(`/?query=${uglyQuery}&variables=${uglyVaribles}`); | |
| cy.visit(`/?query=${encodeURIComponent(uglyQuery)}&variables=${encodeURIComponent(uglyVaribles)}`); |
| cy.expect(w.g.getVariableEditor().getValue()).to.equal(brokenVaribles); | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Excellent job of translating my list into tests! 🙌
|
@benjie Updated 🥳 |
benjie
changed the title
|
Merged! Thanks 🎉 |
Closes #990
and rid of unnecessary editors updates when content not changed during prettification