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

Support "default" and "5XX" types of response status code #226

Open
leonoh-oua opened this issue Mar 12, 2021 · 4 comments
Open

Support "default" and "5XX" types of response status code #226

leonoh-oua opened this issue Mar 12, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@leonoh-oua
Copy link

Which package are you using?
chai-openapi-response-validator

OpenAPI version
3

Describe the bug
Support "default" and "5XX" types of response status code. Those responses are valid based on the open api documents.
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#responsesObject
https://swagger.io/docs/specification/describing-responses/

OpenAPIValidators doesn't seem to support these responses other than status codes in number.

To Reproduce
Sorry for not having time to provide recreation test case.

  1. In open api spec, add 2 different types of response type, "200" and "default", under responses section for whatever path and method

  2. In a test, call satisfyApiSpec with actual response with 400 status code

  3. Test fails with

         expected res to satisfy a '400' response defined for endpoint 'GET /aaa/bbb' in your API spec
         res had status '400', but your API spec has no '400' response defined for endpoint 'GET /aaa/bbb'
         Response statuses found for endpoint 'GET /subject/{subjectCode}/{apiVersion}/textbooks' in API spec: 200, default`
    

Expected behavior
The response is supposed to be validated based on the "default" response definition under responses

Additional context
Reproduce steps above are only about default. But I can see that OpenAPIValidators doesn't support status code range such as "5XX" either

Are you going to resolve the issue?
I'd like you to confirm this is a bug or there's something I'm missing.

@leonoh-oua leonoh-oua added the bug Something isn't working label Mar 12, 2021
@rwalle61
Copy link
Collaborator

rwalle61 commented Mar 13, 2021

Thanks @leonoh-oua! I think we overlooked this functionality. The bug is occuring because we currently throw an error if the actual status code doesn't exactly match that of any responses in the API spec.

As you say, if there are no exact matches, instead we should try to match (e.g.) 400 to a 4XX response in the spec. If there's still no match, we should try to match a default response.

To capture this in tests, we probably want new tests here asserting:

  • 200 matches 2XX when no 200 response in API spec
  • 200 matches default when no 200 or 2XX response in API spec
  • (optional) 300 matches 3XX when no 300 response in API spec

And we may want to update the current error messages thrown when there are no matches, e.g.

res has status '200', but your API spec has no '200', '2XX', or 'default' response defined ...

What do you think of that error message?

Also, if you'd like to contribute a PR, I'm happy to support you with advice or by writing the tests. Let me know :)

@setaman
Copy link

setaman commented Mar 15, 2021

@rwalle61
Found the same issue in my tests with 4XX today:

expected res to satisfy a '400' response defined for endpoint 'POST /x/y' in your API spec
res had status '400', but your API spec has no '400' response defined for endpoint 'POST /x/y'

Response statuses found for endpoint 'POST /x/y' in API spec: 200, 401, 404, 406, 500, 4XX

I think your suggested error message is obvious enough!

@leonoh-oua
Copy link
Author

Thanks @leonoh-oua! I think we overlooked this functionality. The bug is occuring because we currently throw an error if the actual status code doesn't exactly match that of any responses in the API spec.

As you say, if there are no exact matches, instead we should try to match (e.g.) 400 to a 4XX response in the spec. If there's still no match, we should try to match a default response.

To capture this in tests, we probably want new tests here asserting:

* 200 matches 2XX when no 200 response in API spec

* 200 matches default when no 200 or 2XX response in API spec

* (optional) 300 matches 3XX when no 300 response in API spec

And we may want to update the current error messages thrown when there are no matches, e.g.

res has status '200', but your API spec has no '200', '2XX', or 'default' response defined ...

What do you think of that error message?

Also, if you'd like to contribute a PR, I'm happy to support you with advice or by writing the tests. Let me know :)

I wish I would have time to fix it myself :/ I agree with @setaman . Your suggested error message looks good to me too.

@syncush
Copy link

syncush commented Jun 12, 2022

@rwalle61
Hey, I am encountering the same issue, any ETA for merging #271 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants