Item12381: Item12888: Fix unit tests and failure to Expand

Query::getSetParams() was expanding embedded config variables during eval, and then expanded a second time at return. Add a function call to join('', \$value) in the eval, which prevents any expansion of $value. Removed the call to expand the config. Also, getSetParams was converting '' to undef. Removed that test. Fixed remaining ConfigureQuery failures.
foswiki · Jan 17, 2015 · bd7a482 · bd7a482
1 parent 5123cf4
commit bd7a482
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 20 deletions.
diff --git a/UnitTestContrib/lib/Foswiki/Contrib/UnitTestContrib/Config.spec b/UnitTestContrib/lib/Foswiki/Contrib/UnitTestContrib/Config.spec
@@ -77,7 +77,7 @@ $Foswiki::cfg{UnitTestContrib}{Configure}{URLPATH} = '/';
 # **URL CHECK="undefok"**
 # Default: http://localhost
 $Foswiki::cfg{UnitTestContrib}{Configure}{URL} = 'http://localhost';
-# **STRING H CHECK="undefok"**
+# **STRING H CHECK="noemptyok noundefok"**
 # Default: H
 $Foswiki::cfg{UnitTestContrib}{Configure}{H} = 'H';
 # **STRING EXPERT CHECK="undefok"**

diff --git a/UnitTestContrib/test/unit/ConfigureQueryTests.pm b/UnitTestContrib/test/unit/ConfigureQueryTests.pm
@@ -506,12 +506,12 @@ sub test_generic_check_PATH {
     #SMELL: expanded values not reported for type URL
 
     # reports
-    $this->assert_num_equals( 1, scalar( @{ $report->{reports} } ) );
+    $this->assert_num_equals( 2, scalar( @{ $report->{reports} } ) );
 
-    #$r = $report->{reports}->[0];
-    #$this->assert_str_equals( 'notes',                    $r->{level} );
-    #$this->assert_str_equals( 'Expands to: =blah punk\\junk=', $r->{text} );
     $r = $report->{reports}->[0];
+    $this->assert_str_equals( 'notes',                         $r->{level} );
+    $this->assert_str_equals( 'Expands to: =blah punk\\junk=', $r->{text} );
+    $r = $report->{reports}->[1];
     $this->assert_str_equals( 'warnings', $r->{level} );
     $this->assert_matches( qr/You should use/, $r->{text} );
 }
@@ -638,7 +638,7 @@ sub test_generic_check_STRING {
     $this->assert_deep_equals( \@ui_path, $report->{path} );
 
     # reports
-    $this->assert_num_equals( 0, scalar( @{ $report->{reports} } ) );
+    $this->assert_num_equals( 1, scalar( @{ $report->{reports} } ) );
 
     $params->{set}->{'{UnitTestContrib}{Configure}{STRING}'} = 'x';
     $report =
@@ -660,7 +660,7 @@ sub test_generic_check_STRING {
     $this->assert_str_equals( 'errors', $r->{level} );
     $this->assert_matches( qr/Length must be at least/, $r->{text} );
 
-    # H does not have nullEXPERT has nullok
+    # H does not have undefok EXPERT has undefok
     $params = {
         keys => [
             '{UnitTestContrib}{Configure}{H}',
@@ -672,13 +672,12 @@ sub test_generic_check_STRING {
     $report =
       Foswiki::Configure::Query::check_current_value( $params, $reporter );
 
-    #print STDERR Data::Dumper->Dump([$report]);
     $this->assert_num_equals( 2, scalar @$report );
     $this->assert_str_equals( $params->{keys}->[0], $report->[0]->{keys} );
     $this->assert_num_equals( 1, scalar( @{ $report->[0]->{reports} } ) );
     $r = $report->[0]->{reports}->[0];
-    $this->assert_str_equals( 'errors',            $r->{level} );
-    $this->assert_str_equals( 'Must be non-empty', $r->{text} );
+    $this->assert_str_equals( 'errors',           $r->{level} );
+    $this->assert_str_equals( 'May not be empty', $r->{text} );
 
     $report = $report->[1];
     $this->assert_str_equals( $params->{keys}->[1], $report->{keys} );
@@ -775,12 +774,11 @@ sub test_generic_check_URLPATH {
     $this->assert_deep_equals( \@ui_path, $report->{path} );
 
     # reports
-    $this->assert_num_equals( 1, scalar( @{ $report->{reports} } ) );
+    $this->assert_num_equals( 2, scalar( @{ $report->{reports} } ) );
     $r = $report->{reports}->[0];
-
-    #$this->assert_str_equals( 'notes',                   $r->{level} );
-    #$this->assert_str_equals( 'Expands to: =punk junk=', $r->{text} );
-    #$r = $report->{reports}->[1];
+    $this->assert_str_equals( 'notes',                        $r->{level} );
+    $this->assert_str_equals( 'Expands to: =hunk punk junk=', $r->{text} );
+    $r = $report->{reports}->[1];
     $this->assert_str_equals( 'errors', $r->{level} );
     $this->assert_matches( qr/is not valid/, $r->{text} );
 }

diff --git a/core/lib/Foswiki/Configure/Query.pm b/core/lib/Foswiki/Configure/Query.pm
@@ -42,7 +42,7 @@ sub _getSetParams {
     if ( $params->{set} ) {
         while ( my ( $k, $v ) = each %{ $params->{set} } ) {
             ($v) = $v =~ m/^(.*)$/s;    # UNTAINT
-            if ( defined $v && $v ne '' ) {
+            if ( defined $v ) {
                 my $spec  = $root->getValueObject($k);
                 my $value = $v;
                 if ($spec) {
@@ -57,7 +57,10 @@ sub _getSetParams {
                     }
                 }
                 if ( defined $value ) {
-                    eval "\$Foswiki::cfg$k=\$value";
+
+                 # This is needed to prevent expansion of embedded $Foswiki::cfg
+                 # variables during the eval.
+                    eval "\$Foswiki::cfg$k=join('',\$value)";
                 }
                 else {
                     eval "undef \$Foswiki::cfg$k";
@@ -75,9 +78,6 @@ sub _getSetParams {
                       . '</verbatim>' );
             }
         }
-
-        # Expand imported values
-        Foswiki::Configure::Load::expandValue( \%Foswiki::cfg );
     }
 }