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.

Sign up for GitHub

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

Jump to bottom

Add spec for HTTP::Server#close #5958

Merged
merged 1 commit into from Sep 10, 2018
Merged

Add spec for HTTP::Server#close #5958

merged 1 commit into from Sep 10, 2018

Conversation

straight-shoota
Copy link
Member

This PR adds a spec for HTTP::Server#close that is shuts down gracefully and active requests can be finished.

See also #5957

@straight-shoota
Copy link
Member Author

straight-shoota commented Apr 17, 2018 •
edited

For some reason on linux the spec fails when run together with the #listen specs in the same file. If those are skipped, it passes.

I have really no idea what side effect might influence this.

@straight-shoota straight-shoota force-pushed the jm/feature/http-server-gracefully-close branch from 3a92ee0 to e7776ab Compare April 21, 2018 19:39
@straight-shoota straight-shoota mentioned this pull request Apr 30, 2018
Merged
@straight-shoota
Copy link
Member Author

This has also popped up in #6027. It seems to be related to HTTP::Server#close closing the server sockets. I'll investigate further.

straight-shoota added a commit to straight-shoota/crystal that referenced this pull request Apr 30, 2018
@straight-shoota
Fix specs to work around crystal-lang#5958
3660707
chris-huxtable pushed a commit to chris-huxtable/crystal that referenced this pull request Jun 6, 2018
@straight-shoota @chris-huxtable
Fix specs to work around crystal-lang#5958
4d7c608
@straight-shoota straight-shoota force-pushed the jm/feature/http-server-gracefully-close branch from e7776ab to 0eed03c Compare September 4, 2018 22:17
@straight-shoota
Copy link
Member Author

The spec seems to pass now as intended. Maybe the issue that caused it to fail on linux was resolved in the mean time in another PR...

At least this spec can probably be merged now to detect regressions in the future.

@straight-shoota straight-shoota mentioned this pull request Sep 6, 2018
Closed
@RX14 RX14 added this to the 0.27.0 milestone Sep 10, 2018
@RX14 RX14 merged commit 2314da4 into crystal-lang:master Sep 10, 2018
@straight-shoota straight-shoota deleted the jm/feature/http-server-gracefully-close branch September 10, 2018 19:23
ezrast pushed a commit to ezrast/crystal that referenced this pull request Oct 2, 2018
@straight-shoota
Add spec for HTTP::Server#close (crystal-lang#5958)
0dd5ea6
vladfaust
vladfaust reviewed Nov 16, 2018
View reviewed changes
spec/std/http/server/server_spec.cr
context.response.puts "foo"
context.response.flush

Fiber.yield
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no doc for Fiber.yield, could you please explain how it works here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fiber.yield gives the scheduler the possibility to yield execution to a different fiber.
This was a sloppy implementation though and has been replaced since then in #6953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vladfaust vladfaust vladfaust left review comments

@jhass jhass jhass approved these changes

@RX14 RX14 RX14 approved these changes

Assignees
No one assigned
Labels
kind:feature topic:stdlib:specs topic:stdlib
Projects
None yet
Milestone
0.27.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@straight-shoota @jhass @RX14 @vladfaust