search

jamesmacaulay / elm-graphql

A GraphQL library for Elm
http://package.elm-lang.org/packages/jamesmacaulay/elm-graphql/latest
BSD 3-Clause "New" or "Revised" License
313 stars 19 forks source link

Added unit tests to document validator functions #28

Closed Vynlar closed 6 years ago

Vynlar commented 6 years ago

I've added tests to the new validation functions written by @jamesmacaulay. I intend to write tests for (some of) the remaining validation functions and hopefully work on actually implementing them.

Aside: Writing the tests has helped greatly with understanding the structure of the AST which will help me contribute more meaningfully later.

jamesmacaulay commented 6 years ago

Thanks! This is great. There is a failing test in your first commit that doesn't fail (or doesn't get reported by the test runner) when I run tests after pulling the second commit:

↓ GraphQL.Request.Document.AST.ValidateTests
↓ validateLoneAnonymousOperation
✗ should disallow mixed named and anonymous operations

    mixed anonymous/named operations were incorrectly passed during validation

I have a fix for this test in the pull28-dev branch I just pushed, in this commit:

https://github.com/jamesmacaulay/elm-graphql/commit/930e55a9f09a172ba73b03547530dffdffa8af70

Vynlar commented 6 years ago

Whoops! I had left a test skip in there for that test while I was working on others. I've re-enabled that test and, after merging pull28-dev, confirmed that your changes cause it to pass.

jamesmacaulay commented 6 years ago

Sorry that took so long – I wasn't sure if I wanted the failing tests in the feature branch or not until they were resolved, but I think it's best to merge it in and then make them pass. Thanks for this!