Convert cleanly prefork, response, subprocess_ev, and tls to use subtests #1520 #1595
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
spazm
force-pushed
the
fix/1520-use-subtests
branch
2 times, most recently
from
da575ba
to
6ebdd9a
Compare
spazm
force-pushed
the
fix/1520-use-subtests
branch
from
6ebdd9a
to
d6cff6b
Compare
spazm
force-pushed
the
fix/1520-use-subtests
branch
from
d6cff6b
to
9f70a6d
Compare
spazm
changed the title
kraih
reviewed
Nov 1, 2020
|
|
||
| # Silence | ||
| app->log->level('fatal'); | ||
| subtest "Silence" => sub { |
kraih
reviewed
Nov 1, 2020
| return $promise; | ||
| }; | ||
|
|
||
| get '/one' => {text => 'works!'}; |
|
These tests were super broken yesterday with syntax errors everywhere, like a bad script converted them. So please make sure to pay extra attention when reviewing. |
kraih
reviewed
Nov 1, 2020
| is $tx->res->code, 200, 'right status'; | ||
| is $tx->res->body, 'works too!', 'right content'; | ||
|
|
||
| subtest "Manage and clean up PID file" => sub { |
kraih
requested changes
Nov 1, 2020
|
I'm sorry, but i've decided to close these PRs, because they contain too many converted tests at once. And we don't have the review capacity to re-review the whole thing after every scoping issue fix. |
spazm
changed the title
spazm
changed the title
|
updated and rebased to remove the tests that didn't convert cleanly. These four required minimal changes, as detailed in the updated PR summary. |
spazm
changed the title
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.
Summary
Convert tests to use subtests. These tests converted cleanly with no data shared between tests.
prefork.t: converted cleanly. Redeclare re-used names for new scopes:$prefork,$log,$cb,$port,@spawn,@reap,$tx,$graceful.response.t: converted cleanly. Redeclare re-used names for new scopes:$res$compressed&$uncompressed$full,$count,$offset$fileand also new$dircreated from tempdirsubprocess_ev.t: one test converted with no additional changes.tls.t: both tests converted cleanly. Required redeclaring of new scopes in second test:$delay,$server,$client,$client_result,$server_result.Motivation
Requested in #1520
References
Requested in #1520
Viewing the diff ignoring whitespace makes the changes a lot clearer.
git diff -wor add&w=1to the diffs urlPlease feel free to edit or make any stylistic changes as you see fit. Just looking to toss in a helping hand.