Skip to content

Return NoContent when mocking HTTP 204 operations#2000

Merged
Ruwann merged 13 commits intospec-first:mainfrom
julienschuermans:patch-1
Dec 9, 2024
Merged

Return NoContent when mocking HTTP 204 operations#2000
Ruwann merged 13 commits intospec-first:mainfrom
julienschuermans:patch-1

Conversation

@julienschuermans
Copy link
Contributor

@julienschuermans julienschuermans commented Nov 21, 2024

There's never a need to return a response for HTTP 204 - and these code are unlikely to have any "example" response documented in the spec.

Avoids a uvicorn RuntimeError "Response content longer than Content-Length" when mocking a (3.0) spec containing:

...
      responses:
        '204':
          description: No Content
        '401':
          description: Unauthorized
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ProblemDetails'
...
  • We could also move this if/else check into the operation's example_response method. Thoughts?

Never a need to return a response for HTTP 204
@coveralls
Copy link

coveralls commented Nov 21, 2024

Coverage Status

coverage: 94.419% (+0.01%) from 94.408%
when pulling 74fb926 on julienschuermans:patch-1
into 087f05f on spec-first:main.

@chrisinmtown
Copy link
Contributor

I guess you'll have to add/extend a test case to cover the newly added lines?

leverage HTTP Status Enum and NoContent as resp
change import order
@julienschuermans julienschuermans changed the title Update mock.py Return NoContent when mocking HTTP 204 operations Nov 22, 2024
@julienschuermans
Copy link
Contributor Author

julienschuermans commented Nov 22, 2024

I guess you'll have to add/extend a test case to cover the newly added lines?

I can do that - not sure where to place this though. In test_mock.py, test_mock2.py or test_mock3.py ? I guess we want to test behaviour for both Swagger 2.0 and OpenAPI 3? @RobbeSneyders do you have any input?

@RobbeSneyders
Copy link
Member

Thanks @julienschuermans!

I would indeed prefer to move it to the example_response method though.

For the test, I think you can add:

  • A swagger test in test_mock.py
  • An OpenAPI test in test_mock3.py

It should be quite straight forward if you start from a copy of an existing test.

Don't ask me why the numbering of the test_mock files is as it is 😓

julienschuermans and others added 9 commits November 28, 2024 12:48
Return no content if code == 204
Status code check moved to example_response definition
return no content if status==204
Create NoContent mock test for Swagger
Create NoContent mock test for OpenAPI3
remove whitespace
Remove whitespace
@julienschuermans
Copy link
Contributor Author

I've moved the code into the example_response implementations and added some tests.
@RobbeSneyders should be good to go?

@chrisinmtown
Copy link
Contributor

I recommend you squash the 13 commits down to a single one.

Copy link
Member

@Ruwann Ruwann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @julienschuermans. This one can be merged for me.

We can squash the commits during merging, as we're typically doing a squash and commit.

@Ruwann Ruwann merged commit 6e7dd39 into spec-first:main Dec 9, 2024
6 checks passed
@julienschuermans julienschuermans deleted the patch-1 branch December 9, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants