Item13378: discovered that the StoreTests were not being run on all i…

…mplementations

PlainFileStoreContrib/data/System/PlainFileStoreContrib.txt

@@ -29,6 +29,13 @@ The !PlainFileStoreContrib implements topic and attachment histories by keeping
 The latest rev also has a history file (note: this means that
 everything is stored at least twice; same as in the RCS stores)
 
+%X%
+   * Automatic attachment of files placed in pub directories is not supported
+     for performance reasons.
+   * Mapping the 'TWiki' web to the Foswiki 'System' web (as required for
+     the !TWikiCompatibilityPlugin) is *not* supported. If you require
+     TWiki compatibility, use one of the !RCSStoreContrib stores.
+
 Developers note: this contrib is tested using the standard store unit tests.
 
 ---++ Moving between store implementations

PlainFileStoreContrib/lib/Foswiki/Store/PlainFile.pm

@@ -656,12 +656,11 @@ sub getApproxRevTime {
 }
 
 # Implement Foswiki::Store
-# An attachment is only an attachment if it has a presence in the meta-data
 sub eachAttachment {
     my ( $this, $meta ) = @_;
 
     my $dh;
-    opendir( $dh, _attachmentsDir($meta) )
+    opendir( $dh, _getPub($meta) )
       or return new Foswiki::ListIterator( [] );
     my @list = grep { !/^[.*_]/ && !/,pfv$/ }
 
@@ -951,7 +950,7 @@ sub _latestFile {
 }
 
 # Get the absolute file path to the attachments metadir for a topic
-sub _attachmentsDir {
+sub _attachmentsHistoryDir {
     return _getData( $_[0] ) . ',pfv/ATTACHMENTS';
 }
 
@@ -977,7 +976,7 @@ sub _historyDir {
         # a subdir with the same name as the topic and "extension" ,pfm
         # This keeps the pub directory "clean"; a requirement when these
         # files are visible via a web interface.
-        return _attachmentsDir($p1) . "/${p2}";
+        return _attachmentsHistoryDir($p1) . "/${p2}";
     }
     else {
 
@@ -1357,8 +1356,7 @@ sub _moveFile {
     if ( -d $from ) {
         $ok = File::Copy::Recursive::dirmove( $from, $to );
     }
-    else {
-        ASSERT( -e $from, $from ) if DEBUG;
+    elsif ( -e $from ) {
         $ok = File::Copy::move( $from, $to );
     }
     $ok or die "PlainFile: move $from to $to failed: $!";

RCSStoreContrib/test/unit/RCSStoreContrib/AutoAttachTests.pm

@@ -26,6 +26,27 @@ my %cfg;
 
 use Data::Dumper;
 
+sub fixture_groups {
+    my $this = shift;
+    my @groups;
+
+    return [ 'RcsWrap', 'RcsLite' ];
+}
+
+sub RcsWrap {
+    my $this = shift;
+    $Foswiki::cfg{Store}{Implementation}   = 'Foswiki::Store::RcsWrap';
+    $Foswiki::cfg{RCS}{AutoAttachPubFiles} = 1;
+    $this->createNewFoswikiSession( $Foswiki::cfg{AdminUserLogin} );
+}
+
+sub RcsLite {
+    my $this = shift;
+    $Foswiki::cfg{Store}{Implementation}   = 'Foswiki::Store::RcsLite';
+    $Foswiki::cfg{RCS}{AutoAttachPubFiles} = 1;
+    $this->createNewFoswikiSession( $Foswiki::cfg{AdminUserLogin} );
+}
+
 sub set_up_topic {
     my $this = shift;
 
@@ -95,18 +116,13 @@ sub touchFile {
     close(FILE);
 }
 
-sub test_no_autoattach {
-}
-
-sub test_autoattach {
-
-    $Foswiki::cfg{RCS}{AutoAttachPubFiles} = 1;
+sub verify_autoattach {
+    my $this = shift;
 
-    my $this  = shift;
     my $topic = "UnitTest1";
     $this->set_up_topic($topic);
-    $this->verify_normal_attachment( $topic, "afile.txt" );
-    $this->verify_normal_attachment( $topic, "bfile.txt" );
+    $this->check_normal_attachment( $topic, "afile.txt" );
+    $this->check_normal_attachment( $topic, "bfile.txt" );
 
     sleep 2;    # make sure attachment timestamps can change
 
@@ -226,7 +242,7 @@ sub printAttachments {
     }
 }
 
-sub verify_normal_attachment {
+sub check_normal_attachment {
     my $this = shift;
 
     my $topic      = shift;

TWikiCompatibilityPlugin/test/unit/TWikiCompatibilityPlugin/TWikiFuncTests.pm

@@ -67,8 +67,10 @@ sub test_TWiki_web {
 
     $Foswiki::cfg{Plugins}{TWikiCompatibilityPlugin}{Enabled} = 0;
 
-    $this->assert( !Foswiki::Func::webExists('TWiki') );
-    $this->assert( !TWiki::Func::webExists('TWiki') );
+    if ( $Foswiki::cfg{Store}{Implementation} =~ /Rcs/ ) {
+        $this->assert( !Foswiki::Func::webExists('TWiki') );
+        $this->assert( !TWiki::Func::webExists('TWiki') );
+    }
 
     $Foswiki::cfg{Plugins}{TWikiCompatibilityPlugin}{Enabled} = 1;
 

UnitTestContrib/test/unit/FoswikiFnTestCase.pm

@@ -66,8 +66,6 @@ sub loadExtraConfig {
     $Foswiki::cfg{Store}{Implementation}   = "Foswiki::Store::PlainFile";
     $Foswiki::cfg{RCS}{AutoAttachPubFiles} = 0;
 
-    #$Foswiki::cfg{Store}{Implementation}    = "Foswiki::Store::PlainFile";
-
     $Foswiki::cfg{Register}{AllowLoginName} = 1;
     $Foswiki::cfg{Htpasswd}{FileName} = "$Foswiki::cfg{WorkingDir}/htpasswd";
     unless ( -e $Foswiki::cfg{Htpasswd}{FileName} ) {

UnitTestContrib/test/unit/FoswikiStoreTestCase.pm

@@ -88,7 +88,7 @@ sub fixture_groups {
     # Uncomment below to test one store in isolation
     # return [ ref($this) . '_PlainFile' ];
     #    return [ ref($this) . '_RcsWrap' ];
-    return [ ref($this) . '_RcsLite' ];
+    #return [ ref($this) . '_RcsLite' ];
     return \@groups;
 }
 

UnitTestContrib/test/unit/HierarchicalWebsTests.pm

@@ -12,16 +12,19 @@ sub set_up {
     my $this = shift;
 
     $Foswiki::cfg{EnableHierarchicalWebs} = 1;
-    $this->SUPER::set_up();
-    $this->{sub_web}      = "Subweb";
+    $this->{sub_web} = "Subweb";
     $this->{sub_web_path} = "$this->{test_web}/$this->{sub_web}";
-    my $webObject = $this->populateNewWeb( $this->{sub_web_path} );
-    $webObject->finish();
+    $this->SUPER::set_up();
 }
 
 sub set_up_for_verify {
     my $this = shift;
+
     $this->createNewFoswikiSession();
+
+    # subweb of test web, so default tear_down will nosh it
+    my $webObject = $this->populateNewWeb( $this->{sub_web_path} );
+    $webObject->finish();
 }
 
 sub verify_createSubSubWeb {

UnitTestContrib/test/unit/RenameTests.pm

@@ -2188,6 +2188,8 @@ sub test_rename_attachment_Rename_Allowed_Change_Denied {
 sub test_rename_attachment_not_in_meta {
     my $this = shift;
 
+    return unless $Foswiki::cfg{Store}{Implementation} =~ /Rcs/;
+
     my ($to) = Foswiki::Func::readTopic( $this->{test_web}, 'NewTopic' );
     $to->text('Wibble');
     $to->save();
@@ -2209,7 +2211,9 @@ sub test_rename_attachment_not_in_meta {
         }
     );
 
-    my ($text) = $this->captureWithKey( rename => $UI_FN, $this->{session} );
+    my ($text);
+
+    ($text) = $this->captureWithKey( rename => $UI_FN, $this->{session} );
     $this->assert_matches( qr/Status: 302/,                 $text );
     $this->assert_matches( qr#/$this->{test_web}/NewTopic#, $text );
     $this->assert(

UnitTestContrib/test/unit/StoreTests.pm

@@ -40,25 +40,6 @@ use Error qw( :try );
 my $web   = "TemporaryTestStoreWeb";
 my $topic = "TestStoreTopic";
 
-sub set_up {
-    my $this = shift;
-
-    $this->SUPER::set_up();
-
-    my $testWebObj = $this->populateNewWeb($web);
-    $testWebObj->finish();
-
-    #  Store doesn't do access checks anyway, so run as admin
-    #  so that Func:: works
-    $this->createNewFoswikiSession( $Foswiki::cfg{AdminUserLogin} );
-
-    ASSERT( open( my $FILE, '>', "$Foswiki::cfg{TempfileDir}/testfile.gif" ) );
-    print $FILE "one two three";
-    ASSERT( close($FILE) );
-
-    return;
-}
-
 sub tear_down {
     my $this = shift;
 
@@ -72,8 +53,19 @@ sub tear_down {
 }
 
 sub set_up_for_verify {
+    my $this = shift;
 
-    # Required to satisfy superclass
+    my $testWebObj = $this->populateNewWeb($web);
+    $testWebObj->finish();
+
+    #  Store doesn't do access checks anyway, so run as admin
+    #  so that Func:: works
+    $this->createNewFoswikiSession( $Foswiki::cfg{AdminUserLogin} );
+
+    # Source data for attachments
+    ASSERT( open( my $FILE, '>', "$Foswiki::cfg{TempfileDir}/testfile.gif" ) );
+    print $FILE "one two three";
+    ASSERT( close($FILE) );
 
     return;
 }
@@ -539,7 +531,7 @@ sub verify_beforeSaveHandlerChangeMeta {
 
     # set expected meta
     $meta->putKeyed( 'FIELD', { name => 'fieldname', value => 'meta' } );
-    foreach my $fld (qw(rev version date)) {
+    foreach my $fld (qw(rev version date comment)) {
         delete $meta->get('TOPICINFO')->{$fld};
         delete $readMeta->get('TOPICINFO')->{$fld};
     }
@@ -591,7 +583,7 @@ sub verify_beforeSaveHandlerChangeBoth {
     # set expected meta. Changes in the *meta object* take priority
     # over conflicting changes in the *text*.
     $meta->putKeyed( 'FIELD', { name => 'fieldname', value => 'meta' } );
-    foreach my $fld (qw(rev version date)) {
+    foreach my $fld (qw(rev version date comment)) {
         delete $meta->get('TOPICINFO')->{$fld};
         delete $readMeta->get('TOPICINFO')->{$fld};
     }
@@ -944,32 +936,35 @@ sub verify_eachAttachment {
     ($meta) =
       Foswiki::Func::readTopic( $this->{test_web}, $this->{test_topic} );
 
-    my ($f) =
-      "$Foswiki::cfg{PubDir}/$this->{test_web}/$this->{test_topic}/noise.dat";
-    $this->assert( open( my $F, ">", $f ) );
-    print $F "Naff\n";
-    close($F);
-    $this->assert( -e $f );
+    my $expected = "testfile.gif";
 
+    if ( $Foswiki::cfg{Store}{Implementation} =~ /Rcs|PlainFile/ ) {
+        my ($f) =
+"$Foswiki::cfg{PubDir}/$this->{test_web}/$this->{test_topic}/xtra.dat";
+        $this->assert( open( my $F, ">", $f ) );
+        print $F "Naff\n";
+        close($F);
+        $this->assert( -e $f );
+
+        $expected .= " xtra.dat";
+    }
     $meta->save();
     $meta->finish();
     ($meta) =
       Foswiki::Func::readTopic( $this->{test_web}, $this->{test_topic} );
 
     my $it = $this->{session}->{store}->eachAttachment($meta);
     my $list = join( ' ', sort $it->all() );
-    $this->assert_str_equals( "noise.dat testfile.gif", $list );
-
-    $this->assert(
-        Foswiki::Func::attachmentExists(
-            $this->{test_web}, $this->{test_topic}, 'testfile.gif'
-        )
-    );
-    $this->assert(
-        Foswiki::Func::attachmentExists(
-            $this->{test_web}, $this->{test_topic}, 'noise.dat'
-        )
-    );
+    $this->assert_str_equals( $expected, $list );
+
+    foreach my $a ( split( / /, $expected ) ) {
+        $this->assert(
+            Foswiki::Func::attachmentExists(
+                $this->{test_web}, $this->{test_topic}, $a
+            ),
+            "'$a' missing"
+        );
+    }
 
     my ($preDeleteMeta) =
       Foswiki::Func::readTopic( $this->{test_web}, $this->{test_topic} );
@@ -985,11 +980,6 @@ sub verify_eachAttachment {
             $this->{test_web}, $this->{test_topic}, 'testfile.gif'
         )
     );
-    $this->assert(
-        Foswiki::Func::attachmentExists(
-            $this->{test_web}, $this->{test_topic}, 'noise.dat'
-        )
-    );
 
     my ($postDeleteMeta) =
       Foswiki::Func::readTopic( $this->{test_web}, $this->{test_topic} );
@@ -1000,9 +990,11 @@ sub verify_eachAttachment {
     $this->assert_deep_equals( $preDeleteMeta->{FILEATTACHMENT},
         $postDeleteMeta->{FILEATTACHMENT} );
 
-    $it = $this->{session}->{store}->eachAttachment($postDeleteMeta);
-    $list = join( ' ', sort $it->all() );
-    $this->assert_str_equals( "noise.dat", $list );
+    if ( $expected =~ /xtra/ ) {
+        $it = $this->{session}->{store}->eachAttachment($postDeleteMeta);
+        $list = join( ' ', sort $it->all() );
+        $this->assert_str_equals( "xtra.dat", $list );
+    }
     $preDeleteMeta->finish();
     $postDeleteMeta->finish();
 

UnitTestContrib/test/unit/ViewScriptTests.pm

@@ -31,7 +31,7 @@ NotTOAutoLink
 HERE
 
 my $topic2meta =
-'%META:TOPICINFO{author="BaseUserMapping_666" date="[0-9]{10,10}" format="1.1" version="1"}%'
+'%META:TOPICINFO{author="BaseUserMapping_666"( comment="")? date="[0-9]{10,10}" format="1.1" version="1"}%'
   . "\n";
 my $topic2metaQ = $topic2meta;
 $topic2metaQ =~ s/"/"/g;
@@ -257,8 +257,7 @@ sub test_render_raw {
     ( $text, $hdr ) =
       $this->setup_view( $this->{test_web}, 'TestTopic2', 'viewfour', 'all' );
 
-    $this->assert_matches( qr#$topic2meta$topic2#, $text,
-        "Unexpected output from raw=all" );
+    $this->assert_matches( qr#$topic2meta$topic2#, $text );
     $this->assert_matches( qr#^Content-Type: text/plain#ms,
         $hdr, "raw=all should return text/plain - got $hdr" );