From cca5b123ae93a811b335815b0f754c4e4f44c8d4 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Mon, 16 May 2011 20:12:51 -0700 Subject: Bug 657542: Make the AND/OR tests for xt/search.t function properly and catch known-broken tests as it's supposed to. r=mkanat, a=mkanat (module owner) --- xt/lib/Bugzilla/Test/Search.pm | 14 +++++++ xt/lib/Bugzilla/Test/Search/AndTest.pm | 12 +++--- xt/lib/Bugzilla/Test/Search/Constants.pm | 40 ++---------------- xt/lib/Bugzilla/Test/Search/CustomTest.pm | 2 + xt/lib/Bugzilla/Test/Search/FieldTest.pm | 22 +++++++++- xt/lib/Bugzilla/Test/Search/OrTest.pm | 68 +++++++++---------------------- 6 files changed, 64 insertions(+), 94 deletions(-) diff --git a/xt/lib/Bugzilla/Test/Search.pm b/xt/lib/Bugzilla/Test/Search.pm index 133996283..f18d3e4ca 100644 --- a/xt/lib/Bugzilla/Test/Search.pm +++ b/xt/lib/Bugzilla/Test/Search.pm @@ -851,6 +851,20 @@ sub value_translation_cache { return $self->{value_translation_cache}->{$test_name}; } +# When doing AND/OR tests, the value for transformed_value_was_equal +# (see Bugzilla::Test::Search::FieldTest) won't be recalculated +# if we pull our values from the value_translation_cache. So we need +# to also cache the values for transformed_value_was_equal. +sub was_equal_cache { + my ($self, $field_test, $number, $value) = @_; + return if !$self->option('long'); + my $test_name = $field_test->name; + if (@_ == 4) { + $self->{tvwe_cache}->{$test_name}->{$number} = $value; + } + return $self->{tvwe_cache}->{$test_name}->{$number}; +} + ############# # Main Test # ############# diff --git a/xt/lib/Bugzilla/Test/Search/AndTest.pm b/xt/lib/Bugzilla/Test/Search/AndTest.pm index b4554584b..b7f8b3cab 100644 --- a/xt/lib/Bugzilla/Test/Search/AndTest.pm +++ b/xt/lib/Bugzilla/Test/Search/AndTest.pm @@ -40,12 +40,10 @@ sub bug_is_contained { return all { $_->bug_is_contained($number) } $self->field_tests; } -######################## -# SKIP & TODO Messages # -######################## - -sub _join_skip { () } -sub _join_broken_constant { {} } +sub _bug_will_actually_be_contained { + my ($self, $number) = @_; + return all { $_->will_actually_contain_bug($number) } $self->field_tests; +} ############################## # Bugzilla::Search arguments # @@ -65,4 +63,4 @@ sub search_params { return \%params; } -1; \ No newline at end of file +1; diff --git a/xt/lib/Bugzilla/Test/Search/Constants.pm b/xt/lib/Bugzilla/Test/Search/Constants.pm index a06ceecc7..512d180d9 100644 --- a/xt/lib/Bugzilla/Test/Search/Constants.pm +++ b/xt/lib/Bugzilla/Test/Search/Constants.pm @@ -46,7 +46,6 @@ our @EXPORT = qw( KNOWN_BROKEN NUM_BUGS NUM_SEARCH_TESTS - OR_BROKEN SKIP_FIELDS SPECIAL_PARAM_TESTS SUBSTR_NO_FIELD_ADD @@ -299,10 +298,10 @@ use constant KNOWN_BROKEN => { CHANGED_VALUE_BROKEN, # All fields should have a way to search for "changing # from a blank value" probably. - blocked => { contains => [3,4,5] }, - dependson => { contains => [2,4,5] }, + blocked => { contains => [3,4,5], no_criteria => 1 }, + dependson => { contains => [2,4,5], no_criteria => 1 }, work_time => { contains => [1] }, - FIELD_TYPE_BUG_ID, { contains => [5] }, + FIELD_TYPE_BUG_ID, { contains => [5], no_criteria => 1 }, }, # changeto doesn't find remaining_time changes (possibly due to us not # tracking that data properly). @@ -941,39 +940,6 @@ use constant INJECTION_TESTS => ( { value => '/*STAR_COMMENT_TEST' } ); -# This overrides KNOWN_BROKEN for OR configurations. -# It indicates that these combinations are broken in some way that they -# aren't broken when alone, because they don't return what they logically -# should when put into an OR. -use constant OR_BROKEN => { - # Multi-value fields search on individual values, so "equals" OR "notequals" - # returns nothing, when it should instead logically return everything. - 'blocked-equals' => { - 'blocked-notequals' => { contains => [1,2,3,4,5] }, - }, - 'dependson-equals' => { - 'dependson-notequals' => { contains => [1,2,3,4,5] }, - }, - 'bug_group-equals' => { - 'bug_group-notequals' => { contains => [1,2,3,4,5] }, - }, - 'cc-equals' => { - 'cc-notequals' => { contains => [1,2,3,4,5] }, - }, - 'commenter-equals' => { - 'commenter-notequals' => { contains => [1,2,3,4,5] }, - 'longdesc-notequals' => { contains => [2,3,4,5] }, - 'longdescs.isprivate-notequals' => { contains => [2,3,4,5] }, - 'work_time-notequals' => { contains => [2,3,4,5] }, - }, - 'commenter-notequals' => { - 'commenter-equals' => { contains => [1,2,3,4,5] }, - 'longdesc-equals' => { contains => [1] }, - 'longdescs.isprivate-equals' => { contains => [1] }, - 'work_time-equals' => { contains => [1] }, - }, -}; - ################# # Special Tests # ################# diff --git a/xt/lib/Bugzilla/Test/Search/CustomTest.pm b/xt/lib/Bugzilla/Test/Search/CustomTest.pm index d19a9c350..62a622594 100644 --- a/xt/lib/Bugzilla/Test/Search/CustomTest.pm +++ b/xt/lib/Bugzilla/Test/Search/CustomTest.pm @@ -53,6 +53,8 @@ sub operator_test { die "unimplemented" } sub field_object { die "unimplemented" } sub main_value { die "unimplenmented" } sub test_value { die "unimplemented" } +# Custom tests don't use transforms. +sub transformed_value_was_equal { 0 } sub debug_value { my ($self) = @_; my $string = ''; diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm index 02e0df06c..283a90d16 100644 --- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm +++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm @@ -156,9 +156,12 @@ sub debug_value { # result was equal to its first value. sub transformed_value_was_equal { my ($self, $number, $value) = @_; - if (defined $value) { + if (@_ > 2) { $self->{transformed_value_was_equal}->{$number} = $value; + $self->search_test->was_equal_cache($self, $number, $value); } + my $cached = $self->search_test->was_equal_cache($self, $number); + return $cached if defined $cached; return $self->{transformed_value_was_equal}->{$number}; } @@ -215,6 +218,23 @@ sub contains_known_broken { return undef; } +# Used by subclasses. Checks both bug_is_contained and contains_known_broken +# to tell you whether or not the bug will *actually* be found by the test. +sub will_actually_contain_bug { + my ($self, $number) = @_; + my $is_contained = $self->bug_is_contained($number) ? 1 : 0; + my $is_broken = $self->contains_known_broken($number) ? 1 : 0; + + # If the test is supposed to contain the bug and *isn't* broken, + # then the test will contain the bug. + return 1 if ($is_contained and !$is_broken); + # If this test is *not* supposed to contain the bug, but that test is + # broken, then this test *will* contain the bug. + return 1 if (!$is_contained and $is_broken); + + return 0; +} + # Returns a string if creating a Bugzilla::Search object throws an error, # with this field/operator/value combination. sub search_known_broken { diff --git a/xt/lib/Bugzilla/Test/Search/OrTest.pm b/xt/lib/Bugzilla/Test/Search/OrTest.pm index 42dc30698..b1dbf646c 100644 --- a/xt/lib/Bugzilla/Test/Search/OrTest.pm +++ b/xt/lib/Bugzilla/Test/Search/OrTest.pm @@ -25,7 +25,7 @@ package Bugzilla::Test::Search::OrTest; use base qw(Bugzilla::Test::Search::FieldTest); use Bugzilla::Test::Search::Constants; -use List::MoreUtils qw(any uniq); +use List::MoreUtils qw(all any uniq); use constant type => 'OR'; @@ -70,16 +70,8 @@ sub debug_value { # SKIP & TODO Messages # ######################## -sub _join_skip { () } -sub _join_broken_constant { OR_BROKEN } - sub field_not_yet_implemented { my ($self) = @_; - foreach my $test ($self->field_tests) { - if (grep { $_ eq $test->field } $self->_join_skip) { - return $test->field . " is not yet supported in OR tests"; - } - } return $self->_join_messages('field_not_yet_implemented'); } sub invalid_field_operator_combination { @@ -100,17 +92,13 @@ sub _join_messages { sub _bug_will_actually_be_contained { my ($self, $number) = @_; - my @results; + foreach my $test ($self->field_tests) { - if ($test->bug_is_contained($number) - and !$test->contains_known_broken($number)) - { - return 1; - } - elsif (!$test->bug_is_contained($number) - and $test->contains_known_broken($number)) { - return 1; - } + # Some tests are broken in such a way that they actually + # generate no criteria in the SQL. In this case, the only way + # the test contains the bug is if *another* test contains it. + next if $test->_known_broken->{no_criteria}; + return 1 if $test->will_actually_contain_bug($number); } return 0; } @@ -118,46 +106,28 @@ sub _bug_will_actually_be_contained { sub contains_known_broken { my ($self, $number) = @_; - my $join_broken = $self->_join_known_broken; - if (my $contains = $join_broken->{contains}) { - my $contains_is_broken = grep { $_ == $number } @$contains; - if ($contains_is_broken) { - my $name = $self->name; - return "$name contains $number is broken"; - } - return undef; - } - - return $self->_join_contains_known_broken($number); -} - -sub _join_contains_known_broken { - my ($self, $number) = @_; - if ( ( $self->bug_is_contained($number) and !$self->_bug_will_actually_be_contained($number) ) or ( !$self->bug_is_contained($number) and $self->_bug_will_actually_be_contained($number) ) ) { - my @messages = map { $_->contains_known_broken($number) } $self->field_tests; + my @messages = map { $_->contains_known_broken($number) } + $self->field_tests; @messages = grep { $_ } @messages; + # Sometimes, with things that break because of no_criteria, there won't + # be anything in @messages even though we need to print out a message. + if (!@messages) { + my @no_criteria = grep { $_->_known_broken->{no_criteria} } + $self->field_tests; + @messages = map { "No criteria generated by " . $_->name } + @no_criteria; + } + die "broken test with no message" if !@messages; return join(' AND ', @messages); } return undef; } -sub _join_known_broken { - my ($self) = @_; - my $or_broken = $self->_join_broken_constant; - foreach my $test ($self->field_tests) { - @or_broken_for = map { $_->join_broken($or_broken) } $self->field_tests; - @or_broken_for = grep { defined $_ } @or_broken_for; - last if !@or_broken_for; - $or_broken = $or_broken_for[0]; - } - return $or_broken; -} - ############################## # Bugzilla::Search arguments # ############################## @@ -182,4 +152,4 @@ sub search_params { return \%params; } -1; \ No newline at end of file +1; -- cgit v1.2.1