Convert io_loop_v6.t, subprocess.t, and websocket_proxy.t to use subtests. #1520 #1598
Conversation
|
|
||
| }; | ||
|
|
||
| my $reactor = Mojo::IOLoop->singleton->reactor; |
There was a problem hiding this comment.
This seems like completely new code that changes what the test does.
There was a problem hiding this comment.
These tests had far too much shared code, which is why so much of it had to be moved out of the subtests. I was hoping to discuss how to better compartmentalize the tests.
But then I got into a fugue state at the end of hacktoberfest, converting more and more -- only to find I was breaking the tests when run on integration, as I was missing local ENV vars to run all the tests. so it goes.
|
|
||
| }; | ||
|
|
||
| my $server; |
There was a problem hiding this comment.
variable shared between tests and moved out of subtest. How best to isolate?
|
|
||
| }; | ||
|
|
||
| my $reactor2; |
| local $ENV{MOJO_CERT_FILE} = 't/mojo/certs/client.crt'; | ||
| local $ENV{MOJO_KEY_FILE} = 't/mojo/certs/client.key'; | ||
| local $ENV{MOJO_INSECURE} = 0; | ||
| my $ua; |
There was a problem hiding this comment.
variable shared between tests and moved out of subtest. How best to isolate?
|
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
|
Updated PR to remove the tests that did not convert cleanly. Hoping some of the effort can be salvaged to get some small part of this conversion off of your plate. Please consider re-opening and reviewing. Sorry for producing a headache rather than the happy joy of found work. |
|
Thanks for your effort. But from now on please just one test per PR. This is just too much to review. |
|
I've also updated #1520 with a few additions (one test per PR and skip tests with lots of shared state). |
|
If you want to salvage these PRs, limit them to one test file and move everything else in new PRs. |
Summary
Convert 3 tests to use subtests that converted cleanly.
ioloop_ipv6.t: single commented section converted to a single subtest with no other changes.subprocess.t: commented sections converted cleanly to subtests. Three variable names were reused between tests, but they values were not reused or shared. Fix required only redeclaring and reinitializing as needed for$fail,$result, and$subprocess.websocket_proxy_lts.t: commented sections converted cleanly to subtests. Three variable names were reused between tests, but they values were not reused or shared. Fix required only redeclaring and reinitializing as needed for$tx,$keepaliveand$result.Motivation
Requested in #1520
References
See #1520