This particular piece of code is only executed if a test *fails, *in _verify_download(). So as long as the tests succeded, this particular code path was never executed. :-)
Best regards, /Pär 2015-05-20 9:59 GMT+02:00 Tim Ruehsen <tim.rueh...@gmx.de>: > Thank you, Pär and Hubert. > > A very good catch ! > > Since none of our Perl tests failed though we have broken Perl code, the > question arises, if this is dead code (not executed when running our > tests) !? > > BTW, I would like to mention that we plan to translate the Perl tests into > Python. In the end, we want to get rid of the Perl test suite. > So, before you start spending your precious developer time into improving > the > Perl test suite, please consider improving the Python test suite (and/or > move > tests from Perl to Python). > > Anyways, i'll push your patch soon if nobody complains. > > Regards, Tim > > On Tuesday 19 May 2015 22:27:24 Pär Karlsson wrote: > > Yes, you are right. The declaration of $i inside the for expression > shadows > > the scope of the outer $i inside the for loop (which indeed makes the > outer > > $i uninitialized after the loop). > > > > The for loop itself is not stylistically Perlish with the C-style loop, > but > > that's a minor gripe. I made an error when using the Perl-style range > > operator ( N1 .. N2 ) loop and forgot to remove the commented out line > too. > > And I mistakenly left the extraneous 'my' there, which declares $i in a > new > > scope. After some unintentional off-list conversation with Hubert - which > > was lucky since he discovered another embarrasing mistake - my suggestion > > would be to change it to a while loop instead (see attached patch). > > > > Best regards, > > > > /Pär > > > > 2015-05-19 20:04 GMT+02:00 Hubert Tarasiuk <hubert.taras...@gmail.com>: > > > > From: Pär Karlsson <address@hidden> > > > > > > > > A number of Perl-specific cleanups in the test suite perl modules. > > > > > > > > Most of these changes have no immediate functional difference but > puts > > > > > > code > > > > > > > in line with the same formatting (perltidy GNU style) for > readability. > > > > > > > > A warning regarding invalid operators on incompatible values > (numeric vs > > > > strings) have been fixed. > > > > > > > > Some common but discouraged Perl idioms have been updated to a > somewhat > > > > clearer style, especially regarding evals and map() vs. for loops. > > > > > > > > I did not touch the FTP and HTTP server modules functionally, but > there > > > > > > seem > > > > > > > to be a lot of strange code there in, and would warrant further > cleanups > > > > and investigations if these tests would remain in Perl, instead of > > > > moving > > > > over to the Python based tests. > > > > > > > > All tests work nominally on my machine (with perl-5.20.1) but should > be > > > > compatible with versions down to 5.6. > > > > > > I believe I have found an issue with WgetTests module: > > > > my $i; > > > > > > > > # for ($i=0; $i != $min; ++$i) { > > > > for my $i (0 .. $min - 1) > > > > { > > > > > > > > last if substr($expected, $i, 1) ne substr $actual, $i, 1; > > > > if (substr($expected, $i, 1) eq q{\n}) > > > > { > > > > > > > > $line++; > > > > $col = 0; > > > > > > > > } > > > > else > > > > { > > > > > > > > $col++; > > > > > > > > } > > > > > > > > } > > > > my $snip_start = $i - ($SNIPPET_SIZE / 2); > > > > if ($snip_start < 0) > > > > { > > > > > > > > $SNIPPET_SIZE += $snip_start; # Take it from the end. > > > > $snip_start = 0; > > > > > > > > } > > > > > > I am no Perl expert but I tested it and current for loop will not set > > > the $i variable to any value (outside the loop). (Actually it gives me > > > warning about using uninitialized variable when calculating $snip_start > > > and the diff is not printed correctly.) > > > Reverting the commented version seems to fix this problem. Removing the > > > "my" from current for loop does not seem to fix the problem. > > > -- > > > Hubert Tarasiuk >