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

Refactor response writes, test refactors, fix UNPACK_TCP_STATE_FROM_TCP_INFO location #2554

Closed
wants to merge 2 commits into from

Conversation

MSP-Greg
Copy link
Member

@MSP-Greg MSP-Greg commented Feb 15, 2021

Description

  1. request.rb
    1.1. Refactor handle_request
    1.2. Refactor and simplify fast_write for fewer string assignments

  2. minissl.rb
    2.1. Refactor and simplify write for fewer string assignments

  3. server.rb
    3.1 Move UNPACK_TCP_STATE_FROM_TCP_INFO to correct location
    3.2. Add PURGE_INTERRUPT_QUEUE constant
    3.3. Refactor cork_socket, uncork_socket, & closed_socket? to work with ssl sockets
    3.4. Return from above before entering begin block
    3.5 Anchor Ruby 'root' constants, e.g. Socket and TCPSocket

  4. test_puma_server.rb
    4.1 Add multi-byte characters to test_very_large_return
    4.2 Cleanup @server.stop code, misc

  5. test_puma_server_ssl.rb
    5.1. Add multi-byte characters to test_very_large_return
    5.2. Rename a few variables, quotes, misc

NOTES:

Code refactors done while working on updated test framework.

CI using updated tests - MRI, non-MRI. At the bottom of the test step log, see 'Debugging Info`, and click open to see various integration test info.

Closes #2556

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] or [ci skip] to the pull request title.
  • I have added (or updated) 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.

@MSP-Greg MSP-Greg changed the title Refactor some code used for generating responses, test refactors [changelog skip] Refactor some code used for generating responses, test refactors Feb 15, 2021
@MSP-Greg
Copy link
Member Author

MSP-Greg commented Feb 15, 2021

Also added a fix for check_changelog.yml, missing a paren...

REMOVED: see b0bd212

@dentarg
Copy link
Member

dentarg commented Feb 15, 2021

Also added a fix for check_changelog.yml, missing a paren...

Oops, my bad (26776c8), however, I think you could add that paren directly to master :)

@MSP-Greg MSP-Greg added the waiting-for-review Waiting on review from anyone label Feb 18, 2021
@MSP-Greg MSP-Greg changed the title Refactor some code used for generating responses, test refactors Refactor response writes, test refactors, fix UNPACK_TCP_STATE_FROM_TCP_INFO location Mar 7, 2021
@MSP-Greg MSP-Greg mentioned this pull request Mar 10, 2021
8 tasks
@MSP-Greg
Copy link
Member Author

Rebased

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Apr 9, 2021

Closing in favor of #2595. I will extract test updates and maybe a few lib updates (3.3, 3.4, ??) from this into another pr...

@MSP-Greg MSP-Greg closed this Apr 9, 2021
@MSP-Greg MSP-Greg deleted the refacto-02-14 branch November 3, 2021 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undefined constant on platforms without tcp_cork support and with closed_socket support
3 participants