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

[Backport 2.5.x] Add tests of ServerResultUtils#validateResult to make sure an empty body is sent for 1xx, 204 and 304 responses (#6436) #6453

Merged

Conversation

Projects
None yet
3 participants
@monkey-mas
Copy link
Contributor

commented Aug 17, 2016

Pull Request Checklist

  • Have you read How to write the perfect pull request?
  • Have you read through the contributor guidelines?
  • Have you signed the Lightbend CLA?
  • Have you [squashed your commits]? (Optional, but makes merge messages nicer.)
  • Have you added copyright headers to new files?
  • Have you checked that both Scala and Java APIs are updated?
  • Have you updated the documentation for both Scala and Java sections?
  • Have you added tests for any changed functionality?

Helpful things

Purpose

Backport #6436

Add tests of ServerResultUtils#validateResult to make sure an empty b…
…ody is sent for 1xx, 204 and 304 responses

Purpose
Currently there's no tests to check if the returned body of 1xx, 204 and 304 responses
is empty, which is required by RFC7230. To make sure that no entity is sent to a client,
we add unit tests of ServerResultUtils#validateResult[1] that takes responsibility for
cancelling a body if necessary.

Background Context
PR(#6261)[2] was merged to meet the requirement of RFC7230. BasicHttpClient is used for
the integration tests to check if an empty body is returned for 1xx, 204 and 304 responses.
These integration tests, however, don't guarantee that those responses return an empty body
as our BasicHttpClient just returns an empty body instead of sending an actual body.[3]

References
PR(#6261)[2]
PR Comment[4]

[1] https://github.com/playframework/playframework/blob/2.5.4/framework/src/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala#L67-L68
[2] #6261
[3] https://github.com/playframework/playframework/blob/2.5.4/framework/src/play-integration-test/src/test/scala/play/it/http/BasicHttpClient.scala#L214-L218
[4] #6261 (comment)

@mkurz mkurz merged commit ea3bb33 into playframework:2.5.x Aug 17, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details
@mkurz

This comment has been minimized.

Copy link
Member

commented Aug 17, 2016

Thanks!

@monkey-mas

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2016

Thanks for your review!

@monkey-mas monkey-mas deleted the monkey-mas:backport-add-test-to-server-result-utils branch Aug 17, 2016

@marcospereira marcospereira added this to the 2.5.5 milestone Aug 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.