haskell-graphql / graphql-api

Write type-safe GraphQL services in Haskell
BSD 3-Clause "New" or "Revised" License
406 stars 35 forks source link

Variable definitions validation #186

Closed theobat closed 6 years ago

theobat commented 6 years ago

This PR is a required step towards the resolution of #145. It improves the validation of type declaration in variable definitions. For instance it validates the fact that when a document contains $myVariable: MyVar, then the MyVar type actually exists in the schema and it's an InputType.

Remaining tasks:

PS: this is my first "real" attempt to write haskell so please, any insight would be greatly appreciated.

Edit: I added DefinesTypes instances on arguments because failing tests were simply not finding their argument types in the schema (schema in the sense of what is returned by makeSchema)

theobat commented 6 years ago

@jml, Now I think I'm done here, for some reason the tests are marked as failed on CircleCI, but there isn't any error at any stage of the pipeline and the coverage increased everywhere:

#!/bin/bash -eo pipefail
STACK_YAML=stack-8.2.yaml ./scripts/hpc-ratchet

alternatives: BETTER (175 missing => 161 missing)
booleans: OK
expressions: BETTER (1494 missing => 1416 missing)
local_decls: BETTER (15 missing => 14 missing)
top_level_decls: BETTER (685 missing => 669 missing)
Exited with code 2

I'm still open to other changes though if things are not right yet.

jml commented 6 years ago

@theobat Thanks, I'll take a look now.

To fix the coverage error, you need to update the numbers in the script to match the new, better numbers: https://github.com/haskell-graphql/graphql-api/blob/master/scripts/hpc-ratchet#L37-L43

(Edit: correct link)

theobat commented 6 years ago

@jml There we are ! Thanks for your reviews. As a side note, it seems like every guard containing an otherwise statement is counted as "always true" by hpc which means increasing the number of otherwise might force us to decrease coverage..

57% guards (11/19), 6 always True, 2 unevaluated

Next step is a PR with the actual Aeson.Object to Value translation. I won't address it before some time though.

jml commented 6 years ago

As a side note, it seems like every guard containing an otherwise statement is counted as "always true" by hpc

Yeah, that's something I really would like to fix in hpc (or with some kind of tooling), rather than compromising on coverage. The Haskell testing ecosystem needs to get better.

jml commented 6 years ago

Thanks!