Skip to content
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

Golden & roundtrips on *all* Api types #116

Closed
wants to merge 1 commit into from

Conversation

Anviking
Copy link
Collaborator

@Anviking Anviking commented Mar 24, 2019

Issue Number

#91

Overview

  • Added roundtripAndGoldenPerType + machinery to run tests on all Api Capture- and return-types.
  • TODO: Go through the servant combinators. ReqBody?
  • WIP machinery for taking apart ADTs and running roundtripAndGolden on each component.
    • It was able to run roundtripAndGolden for the type of each field of a record
    • I was unable to hook it up with the Api discovery
    • I didn't figure out how to "catch" an hspec failure, such that we could then run more detailed tests on the components on the failing type (see comment for GRecursiveCheckJSON)

Comments

Skärmavbild 2019-03-25 kl  18 49 58

@Anviking Anviking self-assigned this Mar 25, 2019
@Anviking Anviking force-pushed the anviking/91/typelevel branch 3 times, most recently from 6b0a3aa to f6086b7 Compare March 25, 2019 18:10
@Anviking Anviking marked this pull request as ready for review March 25, 2019 18:14
@Anviking Anviking requested a review from KtorZ March 25, 2019 18:20
@Anviking Anviking changed the title Golden & roundtrips on all Api types automatically Golden & roundtrips on all Api types Mar 25, 2019
@Anviking Anviking changed the title Golden & roundtrips on all Api types Golden & roundtrips on ALL Api types Mar 25, 2019
@Anviking Anviking changed the title Golden & roundtrips on ALL Api types Golden & roundtrips on *all* Api types Mar 25, 2019
@Anviking Anviking force-pushed the anviking/91/typelevel branch 3 times, most recently from 8a539ea to 1916637 Compare March 26, 2019 12:05
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good modulo the case for query / path parameters within Capture.
It'd be nice one day to also get the roundtrip automagically for nested types, yet, it's not trivial 🤔

roundtripAndGoldenPerType _ =
Set.union
(roundtripAndGoldenPerType $ Proxy @t)
(roundtripAndGoldenPerType $ Proxy @b)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That one is incorrect. There's no reason for a param t to have a a JSON instance. Instead, we should be checking for HttpApiData roundtrips here!

@Anviking
Copy link
Collaborator Author

Anviking commented Mar 26, 2019

It'd be nice one day to also get the roundtrip automagically for nested types, yet

@KtorZ I just discovered that

hspec-golden-aeson contains

goldenADTSpecs :: forall a. (ToADTArbitrary a, Eq a, Show a, ToJSON a, FromJSON a) => Settings -> Proxy a -> Spec

I'm checking how it works. If it is close to trivial we could add now, otherwise later. Probably in a separate pr regardless.

Did not get this working at all 🤷‍♂️ (as in simple to set-up, but it only ran tests on the original type for some reason)

@KtorZ
Copy link
Member

KtorZ commented Mar 26, 2019

@Anviking

-> TODO: Go through the servant combinators. ReqBody?

No need. We'll add them as we add API types.

Tests all Aeson instances in the Api automatically.
@Anviking
Copy link
Collaborator Author

Closing for now, the value wasn't clear enough. A few problems:

Lacking reduction of test cases

The pr had logic for generating test-cases for a instead of [a], but this was not generalised to other types than List.

Not testing ADT components

-- We are relying heavily on generically derived Aeson instances. This is used
-- to trace the failure of @roundtripAndGolden@.
--
-- Example:
-- @
--    data Record { a :: A, b :: B, c :: C, ds :: [D] }
--        deriving (Generic, ToJSON, FromJSON)
-- @
--
-- If we hand-write faulty Aeson instances for 'D', we'd like to discover that
-- easily when checking roundtrips and goldens on 'Record'.

Dangers of regeneration

If we believe we're making a purely additively change to a type, but screw something up, the tests won't help us. They are re-generated. (actually, no, but not re-generating them is also a problem)

For the future

  • In its current form this ensure that we haven't missed any roundtrip or golden tests in the Api-type. It serves like a backup, but shouldn't replace manually listed test cases.

  • With some more careful thought, we might be able to do something similar — but valuable —in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants