search

RobWin / assertj-swagger

A Swagger assertj test library which compares a contract-first Swagger YAML/JSON file with a code-first Swagger JSON
Apache License 2.0
171 stars 44 forks source link

Assert failed for identical swagger files #24

Closed zacyang closed 6 years ago

zacyang commented 6 years ago

Hi RobWin, We have been using this library for a while now. It worked nicely until yesterday when we needed to implement a (stupid biz rule), where a specific parameter could be placed on as in and/orquery in the parameters section. e.g if you have something like the following in any parameters section, assertj-swagger will fail (just by compare two identical swagger files) on v0.6.0 { "name": "projectId", "in": "path", "description": "projectId", "required": true, "type": "string" }, { "name": "projectId", "in": "query", "required": false, "type": "integer", "format": "int64" }

I've checked the swagger specification it seems this is a legitimate case.

I had the issue fixed locally yesterday and will create a PR. I am creating this issue just for the sake of tracking purpose.

Thanks.

RobWin commented 6 years ago

Hi, I'm not sure if I understand you correctly. Do you mean that your APIs has optional path parameters? The client of your API can either use a path parameter or a query parameter?

zacyang commented 6 years ago

Hi, Not exactly, in our case, the query parameter projectId is optional while the path parameter projectId is mandatory.

The reason for the wired way the contract is present is because we are using springfox to generate the contract based on a kotlin project. In this project we have a filter to inject the userContext object, in which also contains projectId. Somehow, spirngfox recognized it as a query parameter.

Anyway, my point is, despite the wired way the contract is written. It is still a valid swagger file, correct? I would expect assert-swagger to return true if two identical contract files were compared with each other.

RobWin commented 6 years ago

Could you please show me the assertions error which is thrown?

zacyang commented 6 years ago

As I mentioned early, I've created a PR for this issue, where I added a test case to explain this case. https://github.com/RobWin/assertj-swagger/pull/25 could you take a look? Thanks.

atsu85 commented 6 years ago

@zacyang, if i understood correctly, You should manually close this issue (as github didn't understand based on your commit message "Fixed issue #24 ..." that this issue should be closed when merging the PR). Had You used "Fixed #24..." then this issue would have been closed automatically when PR was merged :)

zacyang commented 6 years ago

@atsu85 , thanks for the reminding.

25 was merged to master. I am closing this issue.