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

Close connection in web server tests [skip changelog] #2072

Merged
merged 1 commit into from Nov 11, 2019

Conversation

composerinteralia
Copy link
Contributor

@composerinteralia composerinteralia commented Nov 11, 2019

Closes #2049

Before this commit, test_trickle_attack and
test_file_streamed_request were each taking an extra 20 seconds to run.

Without the Connection: close header, the connection was kept
alive
until it timed out after the
PERSISTENT_TIMEOUT duration of 20 seconds.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] to all commit messages.
  • I have added appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@composerinteralia composerinteralia changed the title Close connection in web server tests Close connection in web server tests [skip changelog] Nov 11, 2019
[skip changelog]

Closes puma#2049

Before this commit, `test_trickle_attack` and
`test_file_streamed_request` were each taking an extra 20 seconds to run.

Without the `Connection: close` header, the [connection was kept
alive][keep alive] until it timed out after the
[`PERSISTENT_TIMEOUT`][PERSISTENT_TIMEOUT] duration of 20 seconds.

[keep alive]: https://github.com/puma/puma/blob/3835ac74dec5511d637cbc4ebc140c3b8632b447/lib/puma/server.rb#L706-L708
[PERSISTENT_TIMEOUT]: https://github.com/puma/puma/blob/3835ac74dec5511d637cbc4ebc140c3b8632b447/lib/puma/const.rb#L109-L111
Copy link
Contributor

@hahmed hahmed left a comment

Choose a reason for hiding this comment

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

Nice 👌👍 -- confirm this worked for me, tested on JRuby.

@nateberkopec nateberkopec merged commit fcb99b9 into puma:master Nov 11, 2019
@nateberkopec
Copy link
Member

💯 💯

@composerinteralia composerinteralia deleted the close-connection branch November 12, 2019 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate 20-second-long test
3 participants