[changelog skip] Move integration logging test to main test suite #2240
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I feel the PR is still needs a bit of work. Since this is my first contribution to Puma, I wanted to get feedback in early. Not sure how to assign reviewers, or labels. I'm assuming that is done by the project maintainers. |
harrylewis
commented
Apr 28, 2020
| def test_write_to_log | ||
| skip_unless_signal_exist? :TERM | ||
|
|
||
| suppress_output = '> /dev/null 2>&1' |
There was a problem hiding this comment.
I wanted to clean up the test output, so I suppressed the curl return.
| @@ -112,4 +112,23 @@ def test_siginfo_thread_print | |||
|
|
|||
| assert_match "Thread: TID", output.join | |||
| end | |||
|
|
|||
| def test_write_to_log | |||
There was a problem hiding this comment.
Most of this is copied from test/shell/t1.rb. One of the main modifications was to remove the #sleep calls. I felt they were unnecessary now given the helper methods #cli_server, and #stop-server wait until the Puma starts fully before handing control back to the test.
8 tasks
|
Cheers! Good work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR moves the first of 3 integration tests mentioned in #2172 that live in the
test/shelldirectory into the main test suite.There was a suggestion to remove the test altogether if there are current tests that exercise the functionality. A cursory look through the existing tests indicates to me that this test is fairly unique.
I decided to move the first test, the "does it log" test to the
TestIntegrationSingle, as it seemed to me to fit best into this kind of basic configuration scenario (hopefully I am understand the context of these tests correctly).Your checklist for this pull request
[changelog skip]the pull request title.[ci skip]to the title of the PR.#issue" to the PR description or my commit messages.