search

cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.27k stars 3.63k forks source link

Update swagger file #3120

Closed jackzampolin closed 5 years ago

jackzampolin commented 5 years ago

3038 was merged with 2 unaddressed comments.

fedekunze commented 5 years ago

Swagger file is not up to date with the latest changes

fedekunze commented 5 years ago

also need to update the full node to stargate.cosmos.networkor rpc.cosmos.network

sabau commented 5 years ago

I found some of the missing fields, probably not all of them, there is a list of unaligned items that I can see? Checking each endpoint is a bit time consuming

alexanderbez commented 5 years ago

Oof, I really wish we had a way to generate/update Swagger based on the code definition -- it's too easy to forget or miss things.

sabau commented 5 years ago

I would love to spend some time on it. The fastest way I usually achieve this if via a CCI test that at least forbid merging if swagger is not aligned. Next step would be having the auto generation, but it still relies on comments written on top of handlers, so if the comment is not updated we may fall into the same problem

alexanderbez commented 5 years ago

You'd be my hero @sabau if you could figure out a way to do this.

jackzampolin commented 5 years ago

@sabau mine too, been thinking about this for a while: https://github.com/cosmos/cosmos-sdk/issues/3011

sabau commented 5 years ago

Cool! @jackzampolin you already had a plan! I found we have already some examples in tendermint regarding this, we can ask PRO and CONS of their experience in maintaining those comments and learn from them.

I'm in search for a solution to a swagger~interface test in Go, (A couple of years ago in JS I was using this package: https://www.npmjs.com/package/swagger-test) I would like something similar if it's ok for everyone

https://github.com/apiaryio/dredd seems language agnostic + offers Go hooks, seems worth to investigate further in that direction

fedekunze commented 5 years ago

We should also add support for swagger v3: https://github.com/APIDevTools/swagger-express-middleware/issues/81#issuecomment-473872199

sabau commented 5 years ago

Yestarday night I made some little experiment, dredd seems to do the job, but we need some work on the swagger file to make him happy for the first round, I tested it against a voyager/lunie node I was running:

dredd client/lcd/swagger-ui/swagger.yaml localhost:9070

and I got back a tons of errors for missing examples/default, on the other hands once I added them and edited the interface he was angry with me because the interface didn't match the real node output, once that is fixed I think we will have our first (really basic) contract tests 🎉 We can use tendermint/node docker image to run this on CCI, for more accurate tests we will need to introduce go hooks, I think the discussion should follow in #3011

jackzampolin commented 5 years ago

We have updated this, but need to come up with a better story for long term maintainability. Closing this issue.