Item1871: Cannot disable registered tag handlers in rest handler or persistent environment

Foswiki stores registered %TAG% handlers in %Foswiki::functionTags, which is initialised from a BEGIN block. Creating a new Foswiki session does not reset %Foswiki::functionTags. As a result, the only way to disable tag handlers registered with Foswiki::Func::registerTagHandler is to disable the plugin via configure, or with the DISABLEDPLUGINS preference.

The preferences are determined before the plugins' initPlugin handlers are called, so if I set DISABLEDPLUGINS in a topic, then the listed plugins' initPlugin handlers are not called and so those plugins' tag handlers are not registered.

However, plugins register their rest handlers from their initPlugin handler, all of which are called before the rest handler is called. This means that all tag handlers are registered before the rest handler is called, and there is no way to inhibit all of the tag handlers registered by a specified plugin. In contrast, a rest handler could modify %Foswiki::cfg and create a new session (like the PublishPlugin does) to disable other handlers.

This also causes problems with persistent environments like modperl. Once a tag handler is enabled, it cannot be disabled with DISABLEDPLUGINS.

Crawford suggested that the wrapper created by Foswiki::Func::registerTagHandler could check to see if the plugin is enabled before calling the handler.

-- MichaelTempest - 31 Jul 2009

This change breaks plugin unit tests, because it removes the plugin from the installed/enabled plugins list. Plugins that are part of the initial set of installed plugins run fine, but other plugins fail. For instance, try DateTimePlugin tests.

-- ArthurClemens - 20 Aug 2009

Thank you. I'm sorry - I missed this aspect of the bug. I think the unit test infrastructure has the same kind of bug that the core had, in that plugin handlers are invoked even when the plugin is disabled. I suggest that the unit test instrastructure should therefore have a way to enable non-core plugins. (By the same token, it should also have a way to disable core plugins.)

There are a number of things that should happen when a plugin is enabled (and similarly, when a plugin is disabled). I think this should be encapsulated in FoswikiFnTestCase.

I have an initial implementation, and it is enough to make the DateTimePlugin tests pass, but it might not be complete. It is easy enough to change the implementation, but I would like to check if the approach is sound and if the API is sensible before I check anything in.

I propose that the unit tests for non-core plugins that rely on functionality such as Foswiki::Func::expandCommonVariables should enable themselves in the unit test setup method, like this: $this->enablePlugin('DateTimePlugin');

I also propose that FoswikiFnTestCase should automatically detect when the test class name is something like MyPluginTests, and enable MyPlugin.

Crawford - you wrote the UnitTestContrib - what is your opinion on this?

-- MichaelTempest - 21 Aug 2009

To be able to run the non-default plugin unit test, I MUST enable it in configure. I had the assumption that using $this->{session}->enterContext('MyPluginEnabled') would enable the plugin already.

-- ArthurClemens - 21 Aug 2009

I temporarily reverted the change I made that caused the problem with non-default plugin unit tests, and I found that the unit test only passes if the plugin is enabled in configure. So that is not new.

I have figured out how to make FoswikiFnTestCase enable a plugin automatically (e.g. enable MyPlugin for MyPluginTests), even when the plugin is not enabled in configure. I think that is worth doing.

However, if the plugin is not enabled in configure, I cannot see a way for a test based on FoswikiFnTestCase to enable a plugin explicitly (i.e. not automatically), without creating a new session.

-- MichaelTempest - 21 Aug 2009

We discussed this on IRC. Some important points from that discussion:

• If a plugin is enabled, then registered %TAG% handlers and handlers like commonTagsHandler should both be called. If a plugin is not enabled, then neither should be called.
• Creating a new session is not a problem
• There is not agreement about whether plugins should be enabled automatically for their unit tests. At present, they are not enabled automatically, and the safest path is to leave them like that.
• An interface like $this->disablePlugin('MyPlugin'); is not practical because there is no way to undo what initPlugin has already done.

After investigating the problem a little further I found this:

• The unit tests create the first Foswiki object before changing the list of enabled plugins in %Foswiki::cfg. initPlugin is therefore called for all plugins enabled in configure. This affects the global state in the unit tests.
• Unit tests must create a new session object to enable plugins, even if they provide a loadExtraConfig method.

I can address both these issues by changing the setup order in FoswikiTestCase.pm so that:

1. the first session is created after the setting the list of enabled plugins based on the Foswiki MANIFEST, and
2. the loadExtraConfig method is called after the setting the list of enabled plugins.

The diff below shows the change.

In practice, all unit tests have to create a session object, whether they that wish to enable additional plugins or not. FoswikiFnTestCase.pm does that work for most of the unit tests. With this change, a unit test based on FoswikiFnTestCase.pm will not have to create an additional session object because loadExtraConfig is called after the list of enabled plugins is set to the default.

With this change, plugins for non-default unit tests would have to do something like this:

sub loadExtraConfig {
    my $this = shift;
    $this->SUPER::loadExtraConfig();

    $Foswiki::cfg{Plugins}{DateTimePlugin}{Enabled} = 1;
}

I also found that I can run the unit tests in separate processes. This is obviously slower, but it provides a way to deal with the global-state problem - unit tests for plugins that modify global state in their initPlugin should be executed in a separate process. I intend that most tests should run in the current manner, and the test runner should spawn a separate process for tests that "request" it (perhaps by doing something like our $MODIFIES_GLOBAL_STATE = 1;)

--- FoswikiTestCase.pm   2009-09-02 07:30:13.000000000 +0200
+++ FoswikiTestCase.mt.pm   2009-09-02 07:29:52.000000000 +0200
@@ -98,46 +98,23 @@
     my $this = shift;
 
     $this->SUPER::set_up();
 
     $this->{__EnvSafe} = {};
     foreach my $sym (%ENV) {
         next unless defined($sym);
         $this->{__EnvSafe}->{$sym} = $ENV{$sym};
     }
 
-    # force a read of $Foswiki::cfg
-    my $query = new Unit::Request();
-    my $tmp = new Foswiki( undef, $query );
-
     # This needs to be a deep copy
     $this->{__FoswikiSafe} =
       Data::Dumper->Dump( [ \%Foswiki::cfg ], ['*Foswiki::cfg'] );
-    $tmp->finish();
-
-    $Foswiki::cfg{WorkingDir} = File::Temp::tempdir( CLEANUP => $cleanup );
-    mkdir("$Foswiki::cfg{WorkingDir}/tmp");
-    mkdir("$Foswiki::cfg{WorkingDir}/registration_approvals");
-    mkdir("$Foswiki::cfg{WorkingDir}/work_areas");
-
-    # Move logging into a temporary directory
-    $Foswiki::cfg{LogFileName} =
-      "$Foswiki::cfg{TempfileDir}/FoswikiTestCase.log";
-    $Foswiki::cfg{WarningFileName} =
-      "$Foswiki::cfg{TempfileDir}/FoswikiTestCase.warn";
-    $Foswiki::cfg{AdminUserWikiName} = 'AdminUser';
-    $Foswiki::cfg{AdminUserLogin}    = 'root';
-    $Foswiki::cfg{SuperAdminGroup}   = 'AdminGroup';
-
-    $this->loadExtraConfig();
-
-    onceOnlyChecks();
 
     # Disable/enable plugins so that only core extensions (those defined
     # in lib/MANIFEST) are enabled, but they are *all* enabled.
 
     # First disable all plugins
     foreach my $k ( keys %{ $Foswiki::cfg{Plugins} } ) {
         next unless ref( $Foswiki::cfg{Plugins}{$k} ) eq 'HASH';
         $Foswiki::cfg{Plugins}{$k}{Enabled} = 0;
     }
 
@@ -148,20 +125,44 @@
     else {
         open( F, "../../lib/MANIFEST" ) || die $!;
     }
     local $/ = "\n";
     while (<F>) {
         if (/^!include .*?([^\/]+Plugin)$/) {
             $Foswiki::cfg{Plugins}{$1}{Enabled} = 1;
         }
     }
     close(F);
+
+    # force completion of %Foswiki::cfg
+    my $query = new Unit::Request();
+    my $tmp = new Foswiki( undef, $query );
+    $tmp->finish();
+
+    $Foswiki::cfg{WorkingDir} = File::Temp::tempdir( CLEANUP => $cleanup );
+    mkdir("$Foswiki::cfg{WorkingDir}/tmp");
+    mkdir("$Foswiki::cfg{WorkingDir}/registration_approvals");
+    mkdir("$Foswiki::cfg{WorkingDir}/work_areas");
+
+    # Move logging into a temporary directory
+    $Foswiki::cfg{LogFileName} =
+      "$Foswiki::cfg{TempfileDir}/FoswikiTestCase.log";
+    $Foswiki::cfg{WarningFileName} =
+      "$Foswiki::cfg{TempfileDir}/FoswikiTestCase.warn";
+    $Foswiki::cfg{AdminUserWikiName} = 'AdminUser';
+    $Foswiki::cfg{AdminUserLogin}    = 'root';
+    $Foswiki::cfg{SuperAdminGroup}   = 'AdminGroup';
+
+    $this->loadExtraConfig();
+
+    onceOnlyChecks();
+
 }
 
 # Restores Foswiki::cfg and %ENV from backup
 sub tear_down {
     my $this = shift;
     $this->{session}->finish() if $this->{session};
     eval { File::Path::rmtree( $Foswiki::cfg{WorkingDir} ); };
     %Foswiki::cfg = eval $this->{__FoswikiSafe};
     foreach my $sym ( keys %ENV ) {
         unless ( defined( $this->{__EnvSafe}->{$sym} ) ) { 

-- MichaelTempest - 02 Sep 2009

Conditional spawning of unit tests is an excellent idea. I can't see anything obvious to comment on; I agree with your analysis.

-- CrawfordCurrie - 02 Sep 2009

I changed FoswikiTestCase.pm as described above.

Unit tests can now be run in separate processes by overriding run_in_new_process()

-- MichaelTempest - 25 Sep 2009