[PATCH 0/5] Perl module coverage

[Ralf Wildenhues]

I'm applying this patch series to the perl-coverage branch and merging
it into master.  It improves coverage for Version.pm, Wrap.pm,
Condition.pm, and DisjConditions.pm, where now, only statements aren't
counted as covered which consist of code used with ithreads only.

The last patch is a very small improvement for an unnecessary condition
check that became obvious by looking at coverage results.  Improves
condition coverage by having one less unreachable condition.  :-)

Coverage results for lib/Automake/tests suite before/after the merge:

---------------------------- ------ ------ ------ ------ ------ ------ ------
File                           stmt   bran   cond    sub    pod   time  total
---------------------------- ------ ------ ------ ------ ------ ------ ------
...b/Automake/ChannelDefs.pm   29.6    3.6    n/a   41.2   88.9    0.0   29.6
.../lib/Automake/Channels.pm   34.8    8.1    0.0   50.0  100.0    0.1   32.0
...lib/Automake/Condition.pm   97.8   94.8  100.0   96.2   94.7   97.3   96.9
...utomake/DisjConditions.pm   70.2   60.0    n/a   68.4  100.0    1.8   70.5
...e/lib/Automake/Version.pm   72.4   60.0   66.7   80.0  100.0    0.1   69.6
...make/lib/Automake/Wrap.pm   84.8   50.0  100.0   80.0   66.7    0.0   78.9
[...]

---------------------------- ------ ------ ------ ------ ------ ------ ------
File                           stmt   bran   cond    sub    pod   time  total
---------------------------- ------ ------ ------ ------ ------ ------ ------
...b/Automake/ChannelDefs.pm   32.4    3.6    n/a   47.1  100.0    0.0   32.8
.../lib/Automake/Channels.pm   52.5   27.4   19.0   60.5  100.0    0.2   48.4
...lib/Automake/Condition.pm  100.0  100.0  100.0  100.0  100.0   97.0  100.0
...utomake/DisjConditions.pm   94.7  100.0    n/a   94.7  100.0    1.8   96.2
...e/lib/Automake/Version.pm  100.0  100.0  100.0  100.0  100.0    0.4  100.0
...make/lib/Automake/Wrap.pm  100.0  100.0  100.0  100.0  100.0    0.1  100.0
[...]

[Ralf Wildenhues]

* lib/Automake/tests/Version.pl (test_version_compare): Also
try Automake::Version::check for the version pairs, taking into
account the special-case naming of code forks.
(@tests): Add more test cases.
(test_bad_versions, @bad_versions): New function, new test cases,
to ensure bad version strings are rejected.
* lib/Automake/tests/Version2.pl: New test.
* lib/Automake/tests/Version3.pl: Likewise.
* lib/Automake/tests/Makefile.am (TESTS): Add tests here ...
(XFAIL_TESTS): ... and here, new.

Signed-off-by: Ralf Wildenhues <Ralf.Wildenhues@...>
---

One thing that's worthwhile mentioning here is that the new XFAILing
tests are really supposed to be failing: they check that we error out
upon seeing an invalid Automake Version string.  Hope this won't be
too confusing.

 ChangeLog                      |   12 ++++++++++++
 lib/Automake/tests/Makefile.am |    6 ++++++
 lib/Automake/tests/Makefile.in |    6 ++++++
 lib/Automake/tests/Version.pl  |   36 ++++++++++++++++++++++++++++++++++--
 lib/Automake/tests/Version2.pl |    5 +++++
 lib/Automake/tests/Version3.pl |    5 +++++
 6 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 lib/Automake/tests/Version2.pl
 create mode 100644 lib/Automake/tests/Version3.pl

diff --git a/ChangeLog b/ChangeLog
index 3cba9d5..c4841df 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2009-10-18  Ralf Wildenhues  <Ralf.Wildenhues@...>
 
+ Coverage for Version.pm.
+ * lib/Automake/tests/Version.pl (test_version_compare): Also
+ try Automake::Version::check for the version pairs, taking into
+ account the special-case naming of code forks.
+ (@tests): Add more test cases.
+ (test_bad_versions, @bad_versions): New function, new test cases,
+ to ensure bad version strings are rejected.
+ * lib/Automake/tests/Version2.pl: New test.
+ * lib/Automake/tests/Version3.pl: Likewise.
+ * lib/Automake/tests/Makefile.am (TESTS): Add tests here ...
+ (XFAIL_TESTS): ... and here, new.
+
  Pod coverage for Perl modules.
  * lib/Automake/ChannelDefs.pm (parse_warnings): Fix
  typo in Pod documentation.
diff --git a/lib/Automake/tests/Makefile.am b/lib/Automake/tests/Makefile.am
index b8fb761..19d100f 100644
--- a/lib/Automake/tests/Makefile.am
+++ b/lib/Automake/tests/Makefile.am
@@ -25,6 +25,12 @@ Condition-t.pl \
 DisjConditions.pl \
 DisjConditions-t.pl \
 Version.pl \
+Version2.pl \
+Version3.pl \
 Wrap.pl
 
+XFAIL_TESTS = \
+Version2.pl \
+Version3.pl
+
 EXTRA_DIST = $(TESTS)
diff --git a/lib/Automake/tests/Makefile.in b/lib/Automake/tests/Makefile.in
index e44898b..2e38ba5 100644
--- a/lib/Automake/tests/Makefile.in
+++ b/lib/Automake/tests/Makefile.in
@@ -231,8 +231,14 @@ Condition-t.pl \
 DisjConditions.pl \
 DisjConditions-t.pl \
 Version.pl \
+Version2.pl \
+Version3.pl \
 Wrap.pl
 
+XFAIL_TESTS = \
+Version2.pl \
+Version3.pl
+
 EXTRA_DIST = $(TESTS)
 all: all-am
 
diff --git a/lib/Automake/tests/Version.pl b/lib/Automake/tests/Version.pl
index e496435..bea91f0 100644
--- a/lib/Automake/tests/Version.pl
+++ b/lib/Automake/tests/Version.pl
@@ -1,4 +1,4 @@
-# Copyright (C) 2002, 2003  Free Software Foundation, Inc.
+# Copyright (C) 2002, 2003, 2009  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -42,6 +42,29 @@ sub test_version_compare
     print "compare (\"$left\", \"$right\") = $res! (not $result?)\n";
     $failed = 1;
   }
+
+  my $check_expected = ($result == 0 || $result == 1) ? 0 : 1;
+  # Exception for 'foo' fork.
+  $check_expected = 1
+    if ($right =~ /foo/ && !($left =~ /foo/));
+
+  my $check = Automake::Version::check ($left, $right);
+  if ($check != $check_expected)
+  {
+    print "check (\"$left\", \"$right\") = $check! (not $check_expected?)\n";
+    $failed = 1;
+  }
+}
+
+sub test_bad_versions
+{
+  my ($ver) = @_;
+  my @version = Automake::Version::split ($ver);
+  if ($#version != -1)
+  {
+    print "shouldn't grok \"$ver\"\n";
+    $failed = 1;
+  }
 }
 
 my @tests = (
@@ -69,15 +92,24 @@ my @tests = (
   ['1.5a', '1.5.1f', 1],
   ['1.5', '1.5.1a', -1],
   ['1.5.1a', '1.5.1f', -1],
+  ['1.5.1f', '1.5.1a', 1],
+  ['1.5.1f', '1.5.1f', 0],
 # special exceptions
   ['1.6-p5a', '1.6.5a', 0],
   ['1.6', '1.6-p5a', -1],
   ['1.6-p4b', '1.6-p5a', -1],
   ['1.6-p4b', '1.6-foo', 1],
-  ['1.6-p4b', '1.6a-foo', -1]
+  ['1.6-p4b', '1.6a-foo', -1],
+  ['1.6-p5', '1.6.5', 0],
+  ['1.6a-foo', '1.6a-foo', 0],
+);
+
+my @bad_versions = (
+  '', 'a', '1', '1a', '1.2.3.4', '-1.2'
 );
 
 test_version_compare (@{$_}) foreach @tests;
+test_bad_versions ($_) foreach @bad_versions;
 
 exit $failed;
 
diff --git a/lib/Automake/tests/Version2.pl b/lib/Automake/tests/Version2.pl
new file mode 100644
index 0000000..038466d
--- /dev/null
+++ b/lib/Automake/tests/Version2.pl
@@ -0,0 +1,5 @@
+# prog_error due to invalid $VERSION.
+
+use Automake::Version;
+
+Automake::Version::check ('', '1.2.3');
diff --git a/lib/Automake/tests/Version3.pl b/lib/Automake/tests/Version3.pl
new file mode 100644
index 0000000..ebac23f
--- /dev/null
+++ b/lib/Automake/tests/Version3.pl
@@ -0,0 +1,5 @@
+# prog_error due to invalid $REQUIRED.
+
+use Automake::Version;
+
+Automake::Version::check ('1.2.3', '');
--
1.6.5.1.31.gad12b

[Ralf Wildenhues]

* lib/Automake/tests/Wrap.pl (@tests): Add test for word with
trailing space.
(test_makefile_wrap, @makefile_tests): New function, new list of
tests, to test makefile_wrap.

Signed-off-by: Ralf Wildenhues <Ralf.Wildenhues@...>
---
 ChangeLog                  |    6 ++++++
 lib/Automake/tests/Wrap.pl |   38 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c4841df..6ae4243 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-10-18  Ralf Wildenhues  <Ralf.Wildenhues@...>
 
+ Coverage for Wrap.pm.
+ * lib/Automake/tests/Wrap.pl (@tests): Add test for word with
+ trailing space.
+ (test_makefile_wrap, @makefile_tests): New function, new list of
+ tests, to test makefile_wrap.
+
  Coverage for Version.pm.
  * lib/Automake/tests/Version.pl (test_version_compare): Also
  try Automake::Version::check for the version pairs, taking into
diff --git a/lib/Automake/tests/Wrap.pl b/lib/Automake/tests/Wrap.pl
index 8d840fc..b415401 100644
--- a/lib/Automake/tests/Wrap.pl
+++ b/lib/Automake/tests/Wrap.pl
@@ -1,4 +1,4 @@
-# Copyright (C) 2003  Free Software Foundation, Inc.
+# Copyright (C) 2003, 2009  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-use Automake::Wrap 'wrap';
+use Automake::Wrap qw/wrap makefile_wrap/;
 
 my $failed = 0;
 
@@ -31,6 +31,18 @@ sub test_wrap
     }
 }
 
+sub test_makefile_wrap
+{
+  my ($in, $exp_out) = @_;
+
+  my $out = &makefile_wrap (@$in);
+  if ($out ne $exp_out)
+    {
+      print STDERR "For: @$in\nGot:\n$out\nInstead of:\n$exp_out\n---\n";
+      ++$failed;
+    }
+}
+
 my @tests = (
   [["HEAD:", "NEXT:", "CONT", 13, "v" ,"a", "l", "ue", "s", "values"],
 "HEAD:v aCONT
@@ -55,10 +67,32 @@ big continuation:diag3
 "big header: END
 cont: word1 END
 cont: word2
+"],
+  [["big header:", "", " END", 16, "w1", "w2 ", "w3"],
+"big header: END
+w1 w2 w3
 "]);
 
+my @makefile_tests = (
+  [["target:"],
+"target:
+"],
+  [["target:", "\t"],
+"target:
+"],
+  [["target:", "\t", "prereq1", "prereq2"],
+"target: prereq1 prereq2
+"],
+  [["target: ", "\t", "this is a long list of prerequisites ending in space",
+    "so that there is no need for another space before the backslash",
+    "unlike in the second line"],
+"target: this is a long list of prerequisites ending in space \\
+\tso that there is no need for another space before the backslash \\
+\tunlike in the second line
+"]);
 
 test_wrap (@{$_}) foreach @tests;
+test_makefile_wrap (@{$_}) foreach @makefile_tests;
 
 exit $failed;
 
--
1.6.5.1.31.gad12b

[Ralf Wildenhues]

* lib/Automake/Condition.pm (new): Catch common programming
errors better by checking type of passed argument before
munging them to all be strings through split.
* lib/Automake/tests/Condition.pl (test_basics): Also test
->human.
(test_merge): New function, test ->merge, ->merge_conds,
->strip.
* lib/Automake/tests/Condition-t.pl (test_basics, test_merge):
Likewise changes, but including state copies across thread
creation.
* lib/Automake/tests/Cond2.pl: New test for programming error.
* lib/Automake/tests/Cond3.pl: Likewise.
* lib/Automake/tests/Makefile.am (TESTS, XFAIL_TESTS): Update.

Signed-off-by: Ralf Wildenhues <Ralf.Wildenhues@...>
---

This was interesting.  I couldn't get the first 'confess' code branch to
ever go off.  Took a bit to notice that 'split' turns everything into a
string.

Cheers,
Ralf

 ChangeLog                         |   15 ++++++++++++
 lib/Automake/Condition.pm         |   13 ++++++----
 lib/Automake/tests/Cond2.pl       |    6 +++++
 lib/Automake/tests/Cond3.pl       |    6 +++++
 lib/Automake/tests/Condition-t.pl |   45 +++++++++++++++++++++++++++++-------
 lib/Automake/tests/Condition.pl   |   39 +++++++++++++++++++++++--------
 lib/Automake/tests/Makefile.am    |    4 +++
 lib/Automake/tests/Makefile.in    |    4 +++
 8 files changed, 108 insertions(+), 24 deletions(-)
 create mode 100644 lib/Automake/tests/Cond2.pl
 create mode 100644 lib/Automake/tests/Cond3.pl

diff --git a/ChangeLog b/ChangeLog
index 6ae4243..a0f6525 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
 2009-10-18  Ralf Wildenhues  <Ralf.Wildenhues@...>
 
+ Coverage and fixes for Condition.pm.
+ * lib/Automake/Condition.pm (new): Catch common programming
+ errors better by checking type of passed argument before
+ munging them to all be strings through split.
+ * lib/Automake/tests/Condition.pl (test_basics): Also test
+ ->human.
+ (test_merge): New function, test ->merge, ->merge_conds,
+ ->strip.
+ * lib/Automake/tests/Condition-t.pl (test_basics, test_merge):
+ Likewise changes, but including state copies across thread
+ creation.
+ * lib/Automake/tests/Cond2.pl: New test for programming error.
+ * lib/Automake/tests/Cond3.pl: Likewise.
+ * lib/Automake/tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
+
  Coverage for Wrap.pm.
  * lib/Automake/tests/Wrap.pl (@tests): Add test for word with
  trailing space.
diff --git a/lib/Automake/Condition.pm b/lib/Automake/Condition.pm
index 276c04a..5c54b8b 100644
--- a/lib/Automake/Condition.pm
+++ b/lib/Automake/Condition.pm
@@ -180,18 +180,21 @@ sub new ($;@)
   };
   bless $self, $class;
 
-  # Accept strings like "FOO BAR" as shorthand for ("FOO", "BAR").
-  @conds = map { split (' ', $_) } @conds;
-
   for my $cond (@conds)
     {
-      next if $cond eq 'TRUE';
-
       # Catch some common programming errors:
       # - A Condition passed to new
       confess "`$cond' is a reference, expected a string" if ref $cond;
       # - A Condition passed as a string to new
       confess "`$cond' does not look like a condition" if $cond =~ /::/;
+    }
+
+  # Accept strings like "FOO BAR" as shorthand for ("FOO", "BAR").
+  @conds = map { split (' ', $_) } @conds;
+
+  for my $cond (@conds)
+    {
+      next if $cond eq 'TRUE';
 
       # Detect cases when @conds can be simplified to FALSE.
       if (($cond eq 'FALSE' && $#conds > 0)
diff --git a/lib/Automake/tests/Cond2.pl b/lib/Automake/tests/Cond2.pl
new file mode 100644
index 0000000..4ad0e1c
--- /dev/null
+++ b/lib/Automake/tests/Cond2.pl
@@ -0,0 +1,6 @@
+# Catch common programming error:
+# A Condition passed as a string to 'new'.
+use Automake::Condition;
+
+my $cond = new Automake::Condition ('TRUE');
+new Automake::Condition ($cond);
diff --git a/lib/Automake/tests/Cond3.pl b/lib/Automake/tests/Cond3.pl
new file mode 100644
index 0000000..dc957af
--- /dev/null
+++ b/lib/Automake/tests/Cond3.pl
@@ -0,0 +1,6 @@
+# Catch common programming error:
+# A Condition passed as a string to 'new'.
+use Automake::Condition;
+
+my $cond = new Automake::Condition ("COND1_TRUE");
+new Automake::Condition ("$cond");
diff --git a/lib/Automake/tests/Condition-t.pl b/lib/Automake/tests/Condition-t.pl
index 0f1dde8..99004ac 100644
--- a/lib/Automake/tests/Condition-t.pl
+++ b/lib/Automake/tests/Condition-t.pl
@@ -34,15 +34,15 @@ use Automake::Condition qw/TRUE FALSE/;
 
 sub test_basics ()
 {
-  my @tests = (# [[Conditions], is_true?, is_false?, string, subst-string]
-       [[], 1, 0, 'TRUE', ''],
-       [['TRUE'], 1, 0, 'TRUE', ''],
-       [['FALSE'], 0, 1, 'FALSE', '#'],
-       [['A_TRUE'], 0, 0, 'A_TRUE', '@A_TRUE@'],
+  my @tests = (# [[Conditions], is_true?, is_false?, string, subst-string, human]
+       [[], 1, 0, 'TRUE', '', 'TRUE'],
+       [['TRUE'], 1, 0, 'TRUE', '', 'TRUE'],
+       [['FALSE'], 0, 1, 'FALSE', '#', 'FALSE'],
+       [['A_TRUE'], 0, 0, 'A_TRUE', '@A_TRUE@', 'A'],
        [['A_TRUE', 'B_FALSE'],
- 0, 0, 'A_TRUE B_FALSE', '@A_TRUE@@B_FALSE@'],
-       [['B_TRUE', 'FALSE'], 0, 1, 'FALSE', '#'],
-       [['B_TRUE', 'B_FALSE'], 0, 1, 'FALSE', '#']);
+ 0, 0, 'A_TRUE B_FALSE', '@A_TRUE@@B_FALSE@', 'A and !B'],
+       [['B_TRUE', 'FALSE'], 0, 1, 'FALSE', '#', 'FALSE'],
+       [['B_TRUE', 'B_FALSE'], 0, 1, 'FALSE', '#', 'FALSE']);
 
   for (@tests)
     {
@@ -55,6 +55,7 @@ sub test_basics ()
   return 1 if $_->[2] != ($a == FALSE);
   return 1 if $_->[3] ne $a->string;
   return 1 if $_->[4] ne $a->subst_string;
+  return 1 if $_->[5] ne $a->human;
  })->join;
     }
   return 0;
@@ -283,7 +284,33 @@ sub test_reduce_or ()
   return $failed;
 }
 
-exit (test_basics || test_true_when || test_reduce_and || test_reduce_or);
+sub test_merge ()
+{
+  my $cond = new Automake::Condition "COND1_TRUE", "COND2_FALSE";
+  return threads->new(sub {
+      my $other = new Automake::Condition "COND3_FALSE";
+      return threads->new(sub {
+ my $both = $cond->merge ($other);
+ return threads->new(sub {
+  my $both2 = $cond->merge_conds ("COND3_FALSE");
+  return threads->new(sub {
+    $cond = $both->strip ($other);
+    my @conds = $cond->conds;
+    return 1 if $both->string ne "COND1_TRUE COND2_FALSE COND3_FALSE";
+    return 1 if $cond->string ne "COND1_TRUE COND2_FALSE";
+    return 1 if $both != $both2;
+  })->join;
+ })->join;
+      })->join;
+    })->join;
+  return 0;
+}
+
+exit (test_basics
+      || test_true_when
+      || test_reduce_and
+      || test_reduce_or
+      || test_merge);
 
 ### Setup "GNU" style for perl-mode and cperl-mode.
 ## Local Variables:
diff --git a/lib/Automake/tests/Condition.pl b/lib/Automake/tests/Condition.pl
index 86f1745..e330e53 100644
--- a/lib/Automake/tests/Condition.pl
+++ b/lib/Automake/tests/Condition.pl
@@ -1,4 +1,4 @@
-# Copyright (C) 2001, 2002, 2003  Free Software Foundation, Inc.
+# Copyright (C) 2001, 2002, 2003, 2009  Free Software Foundation, Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -19,15 +19,15 @@ use Automake::Condition qw/TRUE FALSE/;
 
 sub test_basics ()
 {
-  my @tests = (# [[Conditions], is_true?, is_false?, string, subst-string]
-       [[], 1, 0, 'TRUE', ''],
-       [['TRUE'], 1, 0, 'TRUE', ''],
-       [['FALSE'], 0, 1, 'FALSE', '#'],
-       [['A_TRUE'], 0, 0, 'A_TRUE', '@A_TRUE@'],
+  my @tests = (# [[Conditions], is_true?, is_false?, string, subst-string, human]
+       [[], 1, 0, 'TRUE', '', 'TRUE'],
+       [['TRUE'], 1, 0, 'TRUE', '', 'TRUE'],
+       [['FALSE'], 0, 1, 'FALSE', '#', 'FALSE'],
+       [['A_TRUE'], 0, 0, 'A_TRUE', '@A_TRUE@', 'A'],
        [['A_TRUE', 'B_FALSE'],
- 0, 0, 'A_TRUE B_FALSE', '@A_TRUE@@B_FALSE@'],
-       [['B_TRUE', 'FALSE'], 0, 1, 'FALSE', '#'],
-       [['B_TRUE', 'B_FALSE'], 0, 1, 'FALSE', '#']);
+ 0, 0, 'A_TRUE B_FALSE', '@A_TRUE@@B_FALSE@', 'A and !B'],
+       [['B_TRUE', 'FALSE'], 0, 1, 'FALSE', '#', 'FALSE'],
+       [['B_TRUE', 'B_FALSE'], 0, 1, 'FALSE', '#', 'FALSE']);
 
   for (@tests)
     {
@@ -38,6 +38,7 @@ sub test_basics ()
       return 1 if $_->[2] != ($a == FALSE);
       return 1 if $_->[3] ne $a->string;
       return 1 if $_->[4] ne $a->subst_string;
+      return 1 if $_->[5] ne $a->human;
     }
   return 0;
 }
@@ -240,7 +241,25 @@ sub test_reduce_or ()
   return $failed;
 }
 
-exit (test_basics || test_true_when || test_reduce_and || test_reduce_or);
+sub test_merge ()
+{
+  my $cond = new Automake::Condition "COND1_TRUE", "COND2_FALSE";
+  my $other = new Automake::Condition "COND3_FALSE";
+  my $both = $cond->merge ($other);
+  my $both2 = $cond->merge_conds ("COND3_FALSE");
+  $cond = $both->strip ($other);
+  my @conds = $cond->conds;
+  return 1 if $both->string ne "COND1_TRUE COND2_FALSE COND3_FALSE";
+  return 1 if $cond->string ne "COND1_TRUE COND2_FALSE";
+  return 1 if $both != $both2;
+  return 0;
+}
+
+exit (test_basics
+      || test_true_when
+      || test_reduce_and
+      || test_reduce_or
+      || test_merge);
 
 ### Setup "GNU" style for perl-mode and cperl-mode.
 ## Local Variables:
diff --git a/lib/Automake/tests/Makefile.am b/lib/Automake/tests/Makefile.am
index 19d100f..cb5ed1a 100644
--- a/lib/Automake/tests/Makefile.am
+++ b/lib/Automake/tests/Makefile.am
@@ -22,6 +22,8 @@ TEST_EXTENSIONS = .pl
 TESTS = \
 Condition.pl \
 Condition-t.pl \
+Cond2.pl \
+Cond3.pl \
 DisjConditions.pl \
 DisjConditions-t.pl \
 Version.pl \
@@ -30,6 +32,8 @@ Version3.pl \
 Wrap.pl
 
 XFAIL_TESTS = \
+Cond2.pl \
+Cond3.pl \
 Version2.pl \
 Version3.pl
 
diff --git a/lib/Automake/tests/Makefile.in b/lib/Automake/tests/Makefile.in
index 2e38ba5..6402ade 100644
--- a/lib/Automake/tests/Makefile.in
+++ b/lib/Automake/tests/Makefile.in
@@ -228,6 +228,8 @@ TEST_EXTENSIONS = .pl
 TESTS = \
 Condition.pl \
 Condition-t.pl \
+Cond2.pl \
+Cond3.pl \
 DisjConditions.pl \
 DisjConditions-t.pl \
 Version.pl \
@@ -236,6 +238,8 @@ Version3.pl \
 Wrap.pl
 
 XFAIL_TESTS = \
+Cond2.pl \
+Cond3.pl \
 Version2.pl \
 Version3.pl
 
--
1.6.5.1.31.gad12b

[Ralf Wildenhues]

* lib/Automake/tests/DisjConditions.pl (test_basics): Increase
test coverage: test ->human, ->merge, ->simplify, ->multiply.
* lib/Automake/tests/DisjConditions-t.pl (test_basics): Likewise
changes, but including state copies across thread creation.
* lib/Automake/tests/DisjCon2.pl: New test.
* lib/Automake/tests/DisjCon3.pl: Likewise.
* lib/Automake/tests/Makefile.am (TESTS, XFAIL_TESTS): Adjust.

Signed-off-by: Ralf Wildenhues <Ralf.Wildenhues@...>
---
 ChangeLog                              |    9 +++++++++
 lib/Automake/tests/DisjCon2.pl         |    8 ++++++++
 lib/Automake/tests/DisjCon3.pl         |    7 +++++++
 lib/Automake/tests/DisjConditions-t.pl |   26 ++++++++++++++++++++++++++
 lib/Automake/tests/DisjConditions.pl   |   26 +++++++++++++++++++++++++-
 lib/Automake/tests/Makefile.am         |    4 ++++
 lib/Automake/tests/Makefile.in         |    4 ++++
 7 files changed, 83 insertions(+), 1 deletions(-)
 create mode 100644 lib/Automake/tests/DisjCon2.pl
 create mode 100644 lib/Automake/tests/DisjCon3.pl

diff --git a/ChangeLog b/ChangeLog
index a0f6525..96142b1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2009-10-18  Ralf Wildenhues  <Ralf.Wildenhues@...>
 
+ Coverage for DisjConditions.pm.
+ * lib/Automake/tests/DisjConditions.pl (test_basics): Increase
+ test coverage: test ->human, ->merge, ->simplify, ->multiply.
+ * lib/Automake/tests/DisjConditions-t.pl (test_basics): Likewise
+ changes, but including state copies across thread creation.
+ * lib/Automake/tests/DisjCon2.pl: New test.
+ * lib/Automake/tests/DisjCon3.pl: Likewise.
+ * lib/Automake/tests/Makefile.am (TESTS, XFAIL_TESTS): Adjust.
+
  Coverage and fixes for Condition.pm.
  * lib/Automake/Condition.pm (new): Catch common programming
  errors better by checking type of passed argument before
diff --git a/lib/Automake/tests/DisjCon2.pl b/lib/Automake/tests/DisjCon2.pl
new file mode 100644
index 0000000..9edd8d5
--- /dev/null
+++ b/lib/Automake/tests/DisjCon2.pl
@@ -0,0 +1,8 @@
+# Catch common programming error:
+# A non-Condition reference passed to new.
+use Automake::Condition;
+use Automake::DisjConditions;
+
+my $cond = new Automake::Condition ('TRUE');
+my $cond2 = new Automake::DisjConditions ($cond);
+new Automake::DisjConditions ($cond2);
diff --git a/lib/Automake/tests/DisjCon3.pl b/lib/Automake/tests/DisjCon3.pl
new file mode 100644
index 0000000..8e69e2b
--- /dev/null
+++ b/lib/Automake/tests/DisjCon3.pl
@@ -0,0 +1,7 @@
+# Catch common programming error:
+# A non-reference passed to new.
+use Automake::Condition qw/TRUE FALSE/;
+use Automake::DisjConditions;
+
+my $cond = new Automake::Condition ("COND1_TRUE");
+new Automake::DisjConditions ("$cond");
diff --git a/lib/Automake/tests/DisjConditions-t.pl b/lib/Automake/tests/DisjConditions-t.pl
index eccdcd6..4df5112 100644
--- a/lib/Automake/tests/DisjConditions-t.pl
+++ b/lib/Automake/tests/DisjConditions-t.pl
@@ -35,18 +35,44 @@ use Automake::DisjConditions;
 
 sub test_basics ()
 {
+  my $true = new Automake::DisjConditions TRUE;
+  my $false = new Automake::DisjConditions FALSE;
   my $cond = new Automake::Condition "COND1_TRUE", "COND2_FALSE";
   return threads->new (sub {
     my $other = new Automake::Condition "COND3_FALSE";
+    my $another = new Automake::Condition "COND3_TRUE", "COND4_FALSE";
     return threads->new (sub {
       my $set1 = new Automake::DisjConditions $cond, $other;
       return threads->new (sub {
  my $set2 = new Automake::DisjConditions $other, $cond;
+ my $set3 = new Automake::DisjConditions FALSE, $another;
  return 1 unless $set1 == $set2;
  return 1 if $set1->false;
  return 1 if $set1->true;
  return 1 unless (new Automake::DisjConditions)->false;
  return 1 if (new Automake::DisjConditions)->true;
+ return 1 unless $true->human eq 'TRUE';
+ return 1 unless $false->human eq 'FALSE';
+ return 1 unless $set1->human eq "(COND1 and !COND2) or (!COND3)";
+ return 1 unless $set2->human eq "(COND1 and !COND2) or (!COND3)";
+ my $one_cond_human = $set1->one_cond->human;
+ return 1 unless $one_cond_human eq "!COND3"
+                || $one_cond_human eq "COND1 and !COND2";
+ return 1 unless $set1->string eq "COND1_TRUE COND2_FALSE | COND3_FALSE";
+
+ my $merged1 = $set1->merge ($set2);
+ my $merged2 = $set1->merge ($cond);
+ my $mult1 = $set1->multiply ($set3);
+ return threads->new (sub {
+  my $mult2 = $set1->multiply ($another);
+  return threads->new (sub {
+    return 1 unless $merged1->simplify->string eq "COND1_TRUE COND2_FALSE | COND3_FALSE";
+    return 1 unless $merged2->simplify->string eq "COND1_TRUE COND2_FALSE | COND3_FALSE";
+    return 1 unless $mult1->string eq "COND1_TRUE COND2_FALSE COND3_TRUE COND4_FALSE";
+    return 1 unless $mult1 == $mult2;
+    return 0;
+  })->join;
+ })->join;
       })->join;
     })->join;
   })->join;
diff --git a/lib/Automake/tests/DisjConditions.pl b/lib/Automake/tests/DisjConditions.pl
index 47dea83..7ccac13 100644
--- a/lib/Automake/tests/DisjConditions.pl
+++ b/lib/Automake/tests/DisjConditions.pl
@@ -1,4 +1,5 @@
-# Copyright (C) 2001, 2002, 2003, 2008  Free Software Foundation, Inc.
+# Copyright (C) 2001, 2002, 2003, 2008, 2009  Free Software Foundation,
+# Inc.
 #
 # This file is part of GNU Automake.
 #
@@ -20,15 +21,38 @@ use Automake::DisjConditions;
 
 sub test_basics ()
 {
+  my $true = new Automake::DisjConditions TRUE;
+  my $false = new Automake::DisjConditions FALSE;
   my $cond = new Automake::Condition "COND1_TRUE", "COND2_FALSE";
   my $other = new Automake::Condition "COND3_FALSE";
+  my $another = new Automake::Condition "COND3_TRUE", "COND4_FALSE";
   my $set1 = new Automake::DisjConditions $cond, $other;
   my $set2 = new Automake::DisjConditions $other, $cond;
+  my $set3 = new Automake::DisjConditions FALSE, $another;
   return 1 unless $set1 == $set2;
   return 1 if $set1->false;
   return 1 if $set1->true;
   return 1 unless (new Automake::DisjConditions)->false;
   return 1 if (new Automake::DisjConditions)->true;
+  return 1 unless $true->human eq 'TRUE';
+  return 1 unless $false->human eq 'FALSE';
+  return 1 unless $set1->human eq "(COND1 and !COND2) or (!COND3)";
+  return 1 unless $set2->human eq "(COND1 and !COND2) or (!COND3)";
+  my $one_cond_human = $set1->one_cond->human;
+  return 1 unless $one_cond_human eq "!COND3"
+                  || $one_cond_human eq "COND1 and !COND2";
+  return 1 unless $set1->string eq "COND1_TRUE COND2_FALSE | COND3_FALSE";
+
+  my $merged1 = $set1->merge ($set2);
+  my $merged2 = $set1->merge ($cond);
+  my $mult1 = $set1->multiply ($set3);
+  my $mult2 = $set1->multiply ($another);
+  return 1 unless $merged1->simplify->string eq "COND1_TRUE COND2_FALSE | COND3_FALSE";
+  return 1 unless $merged2->simplify->string eq "COND1_TRUE COND2_FALSE | COND3_FALSE";
+  return 1 unless $mult1->string eq "COND1_TRUE COND2_FALSE COND3_TRUE COND4_FALSE";
+  return 1 unless $mult1 == $mult2;
+
+  return 0;
 }
 
 sub build_set (@)
diff --git a/lib/Automake/tests/Makefile.am b/lib/Automake/tests/Makefile.am
index cb5ed1a..722c75c 100644
--- a/lib/Automake/tests/Makefile.am
+++ b/lib/Automake/tests/Makefile.am
@@ -26,6 +26,8 @@ Cond2.pl \
 Cond3.pl \
 DisjConditions.pl \
 DisjConditions-t.pl \
+DisjCon2.pl \
+DisjCon3.pl \
 Version.pl \
 Version2.pl \
 Version3.pl \
@@ -34,6 +36,8 @@ Wrap.pl
 XFAIL_TESTS = \
 Cond2.pl \
 Cond3.pl \
+DisjCon2.pl \
+DisjCon3.pl \
 Version2.pl \
 Version3.pl
 
diff --git a/lib/Automake/tests/Makefile.in b/lib/Automake/tests/Makefile.in
index 6402ade..875b560 100644
--- a/lib/Automake/tests/Makefile.in
+++ b/lib/Automake/tests/Makefile.in
@@ -232,6 +232,8 @@ Cond2.pl \
 Cond3.pl \
 DisjConditions.pl \
 DisjConditions-t.pl \
+DisjCon2.pl \
+DisjCon3.pl \
 Version.pl \
 Version2.pl \
 Version3.pl \
@@ -240,6 +242,8 @@ Wrap.pl
 XFAIL_TESTS = \
 Cond2.pl \
 Cond3.pl \
+DisjCon2.pl \
+DisjCon3.pl \
 Version2.pl \
 Version3.pl
 
--
1.6.5.1.31.gad12b

[Ralf Wildenhues]

* lib/Automake/Variable.pm (_check_ambiguous_condition): No need
to check for $def since ambiguous_p returns an empty $message if
there is no other condition which is ambiguous to $cond.

Signed-off-by: Ralf Wildenhues <Ralf.Wildenhues@...>
---
 ChangeLog                |    5 +++++
 lib/Automake/Variable.pm |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 96142b1..de8d22e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2009-10-18  Ralf Wildenhues  <Ralf.Wildenhues@...>
 
+ Simplify Variable::_check_ambiguous_condition.
+ * lib/Automake/Variable.pm (_check_ambiguous_condition): No need
+ to check for $def since ambiguous_p returns an empty $message if
+ there is no other condition which is ambiguous to $cond.
+
  Coverage for DisjConditions.pm.
  * lib/Automake/tests/DisjConditions.pl (test_basics): Increase
  test coverage: test ->human, ->merge, ->simplify, ->multiply.
diff --git a/lib/Automake/Variable.pm b/lib/Automake/Variable.pm
index f0c867f..30dcc79 100644
--- a/lib/Automake/Variable.pm
+++ b/lib/Automake/Variable.pm
@@ -469,7 +469,7 @@ sub _check_ambiguous_condition ($$$)
   # We allow silent variables to be overridden silently,
   # by either silent or non-silent variables.
   my $def = $self->def ($ambig_cond);
-  if ($message && !($def && $def->pretty == VAR_SILENT))
+  if ($message && $def->pretty != VAR_SILENT)
     {
       msg 'syntax', $where, "$message ...", partial => 1;
       msg_var ('syntax', $var, "... `$var' previously defined here");
--
1.6.5.1.31.gad12b