From 433a5a81a2e5d593126608e14c26446a2cb23132 Mon Sep 17 00:00:00 2001 From: Ricardo Signes Date: Sat, 7 Jan 2023 11:08:50 -0500 Subject: [PATCH] Set: do not deduplicate Cmp objects You can't really tell whether they are equivalent! Sadly, this breaks some tests. --- Changes | 2 ++ lib/Test/Deep/Set.pm | 46 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/Changes b/Changes index da78d4c..13e36be 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,8 @@ Revision history for Test-Deep {{$NEXT}} + - when loading the various test modules, preserve global variables like + $@; thanks, Felipe Gasper 1.202 2023-01-04 20:40:46-05:00 America/New_York - no changes since trial releases diff --git a/lib/Test/Deep/Set.pm b/lib/Test/Deep/Set.pm index 28294f9..cd2cf00 100644 --- a/lib/Test/Deep/Set.pm +++ b/lib/Test/Deep/Set.pm @@ -119,6 +119,12 @@ EOM return $diag; } +sub _is_cmp { + my ($value) = @_; + return unless Scalar::Util::blessed($value); + return $value->isa('Test::Deep::Cmp'); +} + sub add { # this takes an array. @@ -128,7 +134,7 @@ sub add # added to the set. If a B is found and IgnoreDupes is true, then A will # be discarded, if IgnoreDupes is false, then B will be added to the set # again. - + my $self = shift; my @array = @_; @@ -142,15 +148,43 @@ sub add { my $want_push = 1; my $push_this = $new_elem; - foreach my $old_elem (@$already) + + # Say you've written this: + # + # set(1,2,2,3,4) + # + # You want the actual values stored to be (1,2,3,4). What, though, if you + # wrote this: + # + # set(re($x), re($y)); + # + # It's hard to say! The two re() objects (Test::Deep::Regexp objects) will + # be distinct objects. Their wrapped patterns *may* be the same (if $x and + # $y refer to the same qr object). At some level, the equivalence of two + # tests can't be decided. Meanwhile, eq_deeply (used below, although rjbs + # thinks it's possibly a bad choice) will end up testing one test object + # against the other's test. This is madness, as demonstrated by the case + # that brought this up: + # + # set(qr{1}, qr{2}); + # + # If the refaddr of the first item has a 2 in it, we will end up with a set + # containing only the first item. So, to avoid this, we will never + # deduplicate Test::Deep::Cmp objects, meaning that all this commentary + # just explains why the foreach below is wrapped in this unless: + unless (_is_cmp($new_elem)) { - if (Test::Deep::eq_deeply($new_elem, $old_elem)) + foreach my $old_elem (@$already) { - $push_this = $old_elem; - $want_push = ! $IgnoreDupes; - last; + if (Test::Deep::eq_deeply($new_elem, $old_elem)) + { + $push_this = $old_elem; + $want_push = ! $IgnoreDupes; + last; + } } } + push(@$already, $push_this) if $want_push; }