Use openapi3 for schema tests #2289

hasufell · 2020-11-03T12:43:46Z

Issue Number

Overview

added openapi3 as dep, fixed stack.cabal
fixed types and import, worked around lack of servant-openapi package
fix test failures
fix jormungandr

Comments

hasufell · 2020-11-03T12:47:17Z

stack.yaml

+- optics-extra-0.3
+- optics-th-0.3.0.2
+- optics-vl-0.2.1
+- insert-ordered-containers-0.2.3.1


There's a difficult to track down build failure with 0.2.3 (which is the lowest bound for openapi3). It seems it only fails on a certain combination of packages, so I can't provide an upstream PR without further investigation.

lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs

hasufell · 2020-11-03T13:38:58Z

specifications/api/swagger.yaml

-      format: uri
-      example: https://smash.cardano-mainnet.iohk.io/
+  type: string
+  pattern: '(null|direct|https?:\/\/[a-zA-Z0-9-_~.]+/?)'


The oneOf had an overlap, because it doesn't check the format. So I think it's better to provide a lax javascript regex.

hasufell · 2020-11-04T08:21:13Z

This broke all of nix

KtorZ · 2020-11-04T15:39:07Z

@hasufell I've referenced the dependency (openapi3) from its github repository instead of its hackage package because the hackage index depends on which version of haskell.nix is used by the CI behind the scene. And the version we're currently using is too old so it doesn't know about openapi3.

Bumping Haskell.nix is a possibility, but it sometimes can be quite some work which would be unnecessary now.

rvl · 2020-11-05T08:44:16Z

Thanks for fixing the nix @KtorZ .

@hasufell, for the record, here is the documentation about updating the pins, and I just added a link to the Haskell.nix ChangeLog file.

hasufell · 2020-11-05T19:50:52Z

@hasufell, for the record, here is the documentation about updating the pins, and I just added a link to the Haskell.nix ChangeLog file.

I read that already and it didn't work:

$ niv update haskell.nix
Update haskell.nix
  INFO: new sources.nix available: 18 -> 23
    Please run 'niv init' or add the following line in the nix/sources.nix file:
    # niv: no_update
Done: Update haskell.nix
$ nix-shell
unpacking 'https://github.com/input-output-hk/haskell.nix/archive/23539aef9ea3591dc0d218c5f60b2a4f7d229a20.tar.gz'...
unpacking 'https://github.com/input-output-hk/hackage.nix/archive/7f67b08b9c6f5e9bd5128bf06de54f5de012ee60.tar.gz'...
warning: dumping very large path (> 256 MiB); this may run out of memory
trace: Using IOHK default nixpkgs
trace: gitSource.nix: /nix/store/dvp437wx7bmj4kfpvybifnj0aak27hmc-cardano-node-src does not seem to be a git repository,
assuming it is a clean checkout.
trace: Using index-state: 2020-07-15T00:00:00Z for cardano-node-src
trace: Using index-state: 2020-07-15T00:00:00Z for cardano-node-src
trace: Cleaning component source not supported for hpack package: cardano-addresses-2.1.0
error: The Haskell package set does not contain the package: indexed-profunctors (build dependency).
If you are using Stackage, make sure that you are using a snapshot that contains the package. Otherwise you may need to update the Hackage snapshot you are using, usually by updating haskell.nix.

rvl · 2020-11-09T13:25:43Z

error: The Haskell package set does not contain the package: indexed-profunctors (build dependency).
If you are using Stackage, make sure that you are using a snapshot that contains the package.

What this means is that the Stackage snapshot doesn't contain indexed-profunctors. So it needed the change that @KtorZ added.

The Haskell.nix rev is now updated on master anyway. I have pushed a fix.

hasufell · 2020-11-09T13:47:15Z

What this means is that the Stackage snapshot doesn't contain indexed-profunctors. So it needed the change that @KtorZ added.

Can we add documentation about this? What are the options when this case happens and how to execute the different solutions.

rvl · 2020-11-10T06:43:37Z

Sure. But note that the nix build is based on stack.yaml. So if the stack build fails due to dependencies not in stackage, then the nix build is also going to fail.

hasufell · 2020-11-10T10:08:41Z

@rvl please don't do reverse merges back from master. It messes up the history and makes rebasing etc harder.

rvl · 2020-11-10T10:17:54Z

What is the actual problem @hasufell? It should be easy for you to rebase, and then the merge commit disappears.
I wasn't going to rebase and force push your branch out of consideration for you, and also because it sometimes happens that the other person just force pushes the branch back to what it was before, and the changes are lost.

hasufell · 2020-11-10T11:16:13Z

It should be easy for you to rebase, and then the merge commit disappears.

That's only true if you rebase against upstream master and that now causes lots of other conflicts. Squashing isn't easily possible anymore either (which reduces merge conflicts during rebases against upstream branches).

rvl · 2020-11-10T11:51:46Z

It's easy. There are two merge conflicts which you would have needed to resolve anyway before merging into master.

hasufell · 2020-11-10T12:07:26Z

It's easy. There are two merge conflicts which you would have needed to resolve anyway before merging into master.

I'd prefer not to have reverse merges in my branches. Thanks :)

Co-authored-by: KtorZ <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

Co-authored-by: KtorZ <[email protected]>

lib/test-utils/src/Test/Utils/OpenApi.hs

lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs

lib/core/cardano-wallet-core.cabal

cabal.project

KtorZ · 2020-11-12T09:34:14Z

LGTM, modulo the revision on cardano-addresses which we need to address (release for this one is coming today hopefully).

hasufell · 2020-11-12T10:11:31Z

bors try

KtorZ · 2020-11-12T09:30:06Z

lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs

 -- | Utility function to provide an ad-hoc 'ToSchema' instance for a definition:
 -- we simply look it up within the Swagger specification.
 declareSchemaForDefinition :: Text -> Declare (Definitions Schema) NamedSchema
 declareSchemaForDefinition ref = do
    let json = foldl' unsafeLookupKey specification ["components","schemas",ref]
-    case Aeson.eitherDecode' $ replaceRefs  $ Aeson.encode json of
+    case Aeson.eitherDecode' $ Aeson.encode json of


KtorZ · 2020-11-12T09:30:49Z

specifications/api/swagger.yaml

@@ -308,18 +308,12 @@ x-poolMetadataSource: &poolMetadataSource
    Pool metadata source. This sets the metadata fetching strategy.

    Possible values are
-      * null -> no fetching
+      * none -> no fetching


iohk-bors · 2020-11-12T11:18:59Z

try

Build succeeded:

hasufell requested a review from KtorZ November 3, 2020 12:43

hasufell marked this pull request as draft November 3, 2020 12:43

hasufell added the IMPROVEMENT Mark a PR as an improvement, for auto-generated CHANGELOG label Nov 3, 2020

hasufell commented Nov 3, 2020

View reviewed changes

lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs Outdated Show resolved Hide resolved

hasufell force-pushed the hasufell/ADP-417/migrate-to-openapi3 branch 3 times, most recently from a1bd98e to 7d9f0f5 Compare November 3, 2020 13:37

hasufell commented Nov 3, 2020

View reviewed changes

hasufell self-assigned this Nov 3, 2020

hasufell changed the title ~~[WIP] Use openapi3 for schema tests~~ Use openapi3 for schema tests Nov 3, 2020

hasufell marked this pull request as ready for review November 3, 2020 13:40

hasufell force-pushed the hasufell/ADP-417/migrate-to-openapi3 branch from 370cf7e to 0c5cc12 Compare November 3, 2020 15:40

KtorZ force-pushed the hasufell/ADP-417/migrate-to-openapi3 branch 2 times, most recently from 845f2da to e172b25 Compare November 4, 2020 15:24

Use openapi3 for schema tests

cd79834

Co-authored-by: KtorZ <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

hasufell force-pushed the hasufell/ADP-417/migrate-to-openapi3 branch 2 times, most recently from 1a1b3bb to 328ad66 Compare November 10, 2020 17:45

Get rid of remaining oneOf workarounds and validate patterns

9c40258

Co-authored-by: KtorZ <[email protected]>

hasufell force-pushed the hasufell/ADP-417/migrate-to-openapi3 branch from 328ad66 to 9c40258 Compare November 10, 2020 17:49

hasufell commented Nov 10, 2020

View reviewed changes

lib/test-utils/src/Test/Utils/OpenApi.hs Outdated Show resolved Hide resolved

Fix at_least

bd3f8c0

hasufell commented Nov 10, 2020

View reviewed changes

lib/core/test/unit/Cardano/Wallet/Api/TypesSpec.hs Show resolved Hide resolved

hasufell commented Nov 10, 2020

View reviewed changes

lib/core/cardano-wallet-core.cabal Show resolved Hide resolved

Julian Ospald added 3 commits November 11, 2020 11:02

Remove unnecessary dependencies

5ccdce6

Fix cabal configure

ed648fc

Update cabal files

6b50bc8

hasufell force-pushed the hasufell/ADP-417/migrate-to-openapi3 branch from b8f2409 to 6b50bc8 Compare November 11, 2020 15:41

Use servant-openapi3

6a1912d

KtorZ reviewed Nov 12, 2020

View reviewed changes

cabal.project Outdated Show resolved Hide resolved

Julian Ospald added 2 commits November 12, 2020 11:09

Revert cardano-addresses bump in cabal.project

18d6c35

Remove unneeded dependencies

f196d93

iohk-bors bot added a commit that referenced this pull request Nov 12, 2020

Try #2289:

943fede

KtorZ reviewed Nov 12, 2020

View reviewed changes

KtorZ approved these changes Nov 12, 2020

View reviewed changes

KtorZ merged commit bb384f5 into master Nov 12, 2020

KtorZ deleted the hasufell/ADP-417/migrate-to-openapi3 branch November 12, 2020 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use openapi3 for schema tests #2289

Use openapi3 for schema tests #2289

hasufell commented Nov 3, 2020 •

edited

Loading

hasufell Nov 3, 2020

hasufell Nov 3, 2020 •

edited

Loading

hasufell commented Nov 4, 2020

KtorZ commented Nov 4, 2020

rvl commented Nov 5, 2020

hasufell commented Nov 5, 2020 •

edited

Loading

rvl commented Nov 9, 2020

hasufell commented Nov 9, 2020

rvl commented Nov 10, 2020

hasufell commented Nov 10, 2020

rvl commented Nov 10, 2020

hasufell commented Nov 10, 2020 •

edited

Loading

rvl commented Nov 10, 2020

hasufell commented Nov 10, 2020

KtorZ commented Nov 12, 2020

hasufell commented Nov 12, 2020

KtorZ Nov 12, 2020

KtorZ Nov 12, 2020

iohk-bors bot commented Nov 12, 2020

Use openapi3 for schema tests #2289

Use openapi3 for schema tests #2289

Conversation

hasufell commented Nov 3, 2020 • edited Loading

Issue Number

Overview

Comments

hasufell Nov 3, 2020

Choose a reason for hiding this comment

hasufell Nov 3, 2020 • edited Loading

Choose a reason for hiding this comment

hasufell commented Nov 4, 2020

KtorZ commented Nov 4, 2020

rvl commented Nov 5, 2020

hasufell commented Nov 5, 2020 • edited Loading

rvl commented Nov 9, 2020

hasufell commented Nov 9, 2020

rvl commented Nov 10, 2020

hasufell commented Nov 10, 2020

rvl commented Nov 10, 2020

hasufell commented Nov 10, 2020 • edited Loading

rvl commented Nov 10, 2020

hasufell commented Nov 10, 2020

KtorZ commented Nov 12, 2020

hasufell commented Nov 12, 2020

KtorZ Nov 12, 2020

Choose a reason for hiding this comment

KtorZ Nov 12, 2020

Choose a reason for hiding this comment

iohk-bors bot commented Nov 12, 2020

try

hasufell commented Nov 3, 2020 •

edited

Loading

hasufell Nov 3, 2020 •

edited

Loading

hasufell commented Nov 5, 2020 •

edited

Loading

hasufell commented Nov 10, 2020 •

edited

Loading