-
Notifications
You must be signed in to change notification settings - Fork 213
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
Translate the Swagger API specification into a Servant API specification. #53
Comments
We also need to think about how we'll test that the Servant API is equivalent to the Swagger API specification. |
@jonathanknowles On that last remark, I'd recommend looking into: with some inspiration from (https://github.com/input-output-hk/cardano-wallet-legacy/blob/develop/test/unit/API/SwaggerSpec.hs) And maybe later, give a shot to https://hackage.haskell.org/package/servant-quickcheck-0.0.2.2/docs/Servant-QuickCheck.html later on might be a good idea. |
@jonathanknowles let's tackle generation of golden tests in a separate ticket (I'll explain tomorrow in the planning meeting). |
@jonathanknowles, @KtorZ - really nice sets of tests for this. I like 👍 Looking at coveralls there are few places where some cases could be produced:
What do you think? |
I admit I don't full understand the third one 🤔 ... But I'll look into them all 👍 |
Also this one seems weird -> https://coveralls.io/builds/22429878/source?filename=src/Cardano/Wallet/Api/Types.hs#L388 since Anyway, I'll put back to in progress for the time being. |
For |
lgtm 👍 |
Context
We intend to maintain both a Swagger API specification and a Servant API specification, and to keep the two in synchronization with one another.
Task
Translate the Swagger API specification (
specifications/api/swagger.yaml
) into an equivalent Servant API specification, including:Acceptance Criteria
Development Plan
PR
master
master
master
master
master
master
master
master
QA
The scope of this ticket has been reduced down to the endpoints discussed during the last iteration planning meeting:
(- Delete wallet) done as bonus here
Transaction creation and listing will be covered by Model API Types for Transactions in Servant #93
Translated types are available in the haddock documentation here: https://input-output-hk.github.io/cardano-wallet/haddock/cardano-wallet-2.0.0/Cardano-Wallet-Api-Types.html.
The strategy here was to re-use most of our low-level types and wrap them in a polymorphic
ApiT
to define the various serialization instances we needed. This way, we keep the API layers fairly separated from the rest and we avoid cluttering the core code with API-related concerns.For now, there's no server actually implementing these types, but we have been already able to perform quite some extensive testing (cf: Automatically generate golden tests for ToJSON instances of API types #91). Testing can be found in Cardano.Wallet.Api.TypesSpec and are of four kinds:
Roundtrip tests for JSON for types appearing in the API
Golden tests for JSON representation of types appearing the API (prevent breaking change), cf Automatically generate golden tests for ToJSON instances of API types #91
Comparison of every response & body param type of the API against their corresponding swagger representation (so here,
Wallet
,[Wallet]
andWalletPostData
). It works by generating arbitrary samples for each types, and then, checks whether they satisfy their corresponding schema from the spec. So, it was possible to represent a value in Haskell that would violate one of the constraints we have in swagger, the test would fail.And finally, tests which check that all paths present in our API definition also exist and match one from the specification. We do not do the other direction (verifying that every path in the spec exist in our API) because we obviously haven't implemented everything yet, though in practice, it would be a nice one to have later.
Added a few negative test cases to test failing paths in various decoders that aren't fully generic (see: Add a few negative test cases to ApiTypes specs for JSON instances that aren't fully generic #128)
The text was updated successfully, but these errors were encountered: