| Subject: | t/12_acc.t: Order of unit tests affects outcome of tests |
| Date: | Sun, 13 Sep 2015 19:34:53 -0400 |
| To: | bug-Text-CSV_XS [...] rt.cpan.org |
| From: | James E Keenan <jkeen [...] verizon.net> |
The results of individual tests in t/12_acc.t depend on the results of
earlier tests in a way that it is not immediately apparent from the
layout of tests in the file. Inserting an additional test at a given
point may change the outcome of a test farther down in the file in an
unexpected way.
Today I cloned Text-CSV_XS from github at commit
a143fda784ab54baf765ade9872395b55736ba98. Being a coverage fanatic, one
of the first things I did was to run the test suite through
Devel::Cover. I was very pleased to see very high test coverage:
#####
$ cover -summary
Reading database from /home/jkeenan/gitwork/Text-CSV_XS/cover_db
------------------------- ------ ------ ------ ------ ------ ------
File stmt bran cond sub time total
------------------------- ------ ------ ------ ------ ------ ------
blib/lib/Text/CSV_XS.pm 99.6 92.5 80.8 100.0 100.0 93.7
Total 99.6 92.5 80.8 100.0 100.0 93.7
------------------------- ------ ------ ------ ------ ------ ------
#####
But, being a coverage fanatic, I could not resist the temptation to
start writing tests for uncovered branches. What I first saw was an
uncovered branch in sub quote():
#####
301 sub quote
302 {
303 14 14 10 my $self = shift;
304 14 100 20 if (@_) {
305 6 4 my $quote = shift;
306 *** 6 50 11 defined $quote or $quote
= "";
#####
Or, when looked at from the "Branches" section of the coverage report:
#####
306 *** 50 0 6 unless defined $quote
#####
I acked through the test suite to see where sub quote() was exercised,
then I studied the source code. I figured that with the following
addition to t/12_acc.t I could cover the uncovered branch at line 306 of
CSV_XS.pm.
#####
diff --git a/t/12_acc.t b/t/12_acc.t
index 90ea17c..347edf1 100644
--- a/t/12_acc.t
+++ b/t/12_acc.t
@@ -3,7 +3,7 @@
use strict;
use warnings;
-use Test::More tests => 183;
+use Test::More qw(no_plan); # tests => 183;
BEGIN {
use_ok "Text::CSV_XS";
@@ -50,6 +50,7 @@ is ($csv->sep (";"), ';',
"sep (;)");
is ($csv->sep_char (), ';', "sep_char ()");
is ($csv->quote_char ("="), '=', "quote_char (=)");
is ($csv->quote ("="), '=', "quote (=)");
+is ($csv->quote (undef), '', "quote (undef)");
is ($csv->eol (undef), "", "eol (undef)");
is ($csv->eol (""), "", "eol ('')");
is ($csv->eol ("\r"), "\r", "eol (\\r)");
#####
But when I ran this test file, I got a test failure -- but only several
tests after the test I added!
#####
$> prove -vb t/12_acc.t
...
ok 36 - quote (undef) # <- added test
ok 37 - eol (undef)
ok 38 - eol ('')
ok 39 - eol (\r)
ok 40 - keep_meta_info (1)
ok 41 - always_quote (undef)
ok 42 - always_quote (1)
ok 43 - allow_loose_quotes (1)
ok 44 - allow_loose_escapes (1)
ok 45 - allow_unquoted_escape (1)
ok 46 - allow_whitespace (1)
ok 47 - blank_is_undef (1)
ok 48 - empty_is_undef (1)
ok 49 - auto_diag (1)
ok 50 - auto_diag (2)
ok 51 - auto_diag (9)
ok 52 - auto_diag ("true")
ok 53 - auto_diag ("false")
ok 54 - auto_diag (undef)
ok 55 - auto_diag ("")
ok 56 - diag_verbose (1)
ok 57 - diag_verbose (2)
ok 58 - diag_verbose (9)
ok 59 - diag_verbose ("true")
ok 60 - diag_verbose ("false") is ($csv->quote ("="), '=', "quote (=)");
+is ($csv->quote (undef), '', "quote (undef)");
ok 61 - diag_verbose (undef)
ok 62 - diag_verbose ("")
ok 63 - verbatim (1)
ok 64 - quote_space (1)
ok 65 - quote_empty (1)
ok 66 - escape_null (1)
ok 67 - quote_null (1)
ok 68 - quote_binary (1)
ok 69 - escape_char (\)
ok 70 - combine
not ok 71 - string
...
# Failed test 'string'
# at t/12_acc.t line 88.
' got: 'txt =, "Hi!";Yes;;2;;1.09;
'; expected: '=txt \=, "Hi!"=;=Yes=;==;=2=;;=1.09=;=
# Looks like you failed 1 test of 184.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/184 subtests
Test Summary Report
-------------------
t/12_acc.t (Wstat: 256 Tests: 184 Failed: 1)
Failed test: 71
Non-zero exit status: 1
Files=1, Tests=184, 0 wallclock secs ( 0.03 usr 0.01 sys + 0.03 cusr
0.00 csys = 0.07 CPU)
Result: FAIL
#####
It took several minutes to figure out how adding a new test after the
35th test caused a failure in test 71. The new test 36 set the quote
character to an empty string -- as intended -- but I didn't realize that
would have an effect on the 'combine' in test 70 and hence on the
'string' in test 71.
Granted, there's no problem with the source code here -- only with the
layout of the test file. Since it is only with the 'is ($csv->string,
...' tests that we actually ask what comma-separated record has been
created, perhaps the test file should be broken up into several blocks,
in each of which a new Text::CSV_XS object is created and which
culminates in a '$csv->string' test.
Final thought: When I placed the 'quote (undef)' test *ahead of* the
'quote (=)' test, all tests PASSed and when I ran it through
Devel::Cover I found I got the expected increase in branch coverage.
Thank you very much.
Jim Keenan