confuser
#92 validation by server side plugin
Code for Issues/FR #92.
Currently only PoC to show basic concept and agree on future direction. Do not expected to be merged in this state. @confuser please look at this PoC.
Few notes:
- currently only validation for arguments of fields directly in
queryis implemented (value provided both over variables or inlined),TODOs in the code for other things which need to be implemented - validate method and Apollo plugin added into
index.js. I expect to addvalidationRuleand maybe even Envelop plugin later. - query visitor is used for new new validation - implemented in
lib/QueryValidationVisitor.js - some code has been extracted from
index,jsto/lib/typeutils.jsso can be reused in new code -
validatemethods fromConstraintXXTypes exported and reused
For now you can run test of the new functionality using npm run test1 and test is completely separate in test/server-validator-test.js.
Main question for me is how to implement tests, as most of tests should be the same as current tests for current implementation (with very small changes as variable types in query and error messages can differ a bit sometimes). There will be probably some more tests for new functionality covering argument values inlined in the query.
I'd like to reuse current tests as much as possible but not sure how to do it correctly in mocha, any idea?
Currently I updated setup.js for tests to configure current or new validator based on env variable, but it would mean we have to run test suite twice in test script, with different env variables, and I'm not sure if it is possible to aggregate results (mainly coverage) into one common results.
Do you know other path how to run the same tests twice with different server validators? In java world I came from it's easy by use of parent classes, not sure how to do it in JavaScript.
Thanks in advance for any comment and proposal around tests
OK, looks like solution for tests is to wrap whole describe part into function, and then call it twice from two other describe parts for each type of server side code we want to test. See https://stackoverflow.com/questions/45358311/how-to-appropriately-reuse-the-describe-blocks-of-mocha-tests
So we should have two top level test files for each server side version run by mocha, which will include and run functions with all relevant tests implemented in other files. Input arguments to that methods can be setup function and argument indicating which kind of server side is tested, so individual tests can react to it and change what is necessary (some helper methods may be implemented in shared testutils.js to be reused in these tests)
I can try to reorganize tests this way as the next step, maybe in separate/independent PR, so they can be merged sooner to prevent conflicts later once this PR will be ready (as it will take some time and other development will probably run in parallel, which will change tests).
WDYT @confuser ?
Pull Request Test Coverage Report for Build 2390292579
- -67 of 97 (30.93%) changed or added relevant lines in 5 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage decreased (-25.5%) to 70.543%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| lib/typeutils.js | 16 | 23 | 69.57% |
| index.js | 6 | 22 | 27.27% |
| lib/QueryValidationVisitor.js | 6 | 50 | 12.0% |
| <!-- | Total: | 30 | 97 |
| Totals | |
|---|---|
| Change from base Build 2333386918: | -25.5% |
| Covered Lines: | 163 |
| Relevant Lines: | 236 |
💛 - Coveralls
Pull Request Test Coverage Report for Build 2933546790
- 143 of 150 (95.33%) changed or added relevant lines in 6 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.02%) to 96.162%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| lib/type-utils.js | 20 | 23 | 86.96% |
| lib/query-validation-visitor.js | 89 | 93 | 95.7% |
| <!-- | Total: | 143 | 150 |
| Totals | |
|---|---|
| Change from base Build 2393476511: | 0.02% |
| Covered Lines: | 289 |
| Relevant Lines: | 296 |
💛 - Coveralls
@confuser testsuite updated to reuse tests for both implementations, best example is /test/argument.test.js.
Only part of tests covering implemented features is running for the new server validator implementation for now.
package.json has scripts allowing to individually run testsuites for distinct implementations/server validation plugins. Main test runs them all together.
@confuser looks like server plugin implementation is finished, any comments welcome
This pull request is becoming rather large now @velias when will it be in a ready state to review? 😄
Sorry, I noticed improvement in Envelop plugin and README, now it is really done :-)