From 2664046c32406fd2c9319b7671e7c1d86c0a66b5 Mon Sep 17 00:00:00 2001 From: Vadim Belman Date: Thu, 15 Sep 2016 19:26:37 -0400 Subject: [PATCH] Item14152: Mostly finished tests for current extensions implementation. ExtensionsTests now have examples of extensions code and functionality. - Moved extension version check into prepareDisabledExtenstions() method to allow processing of manually registered extensions. Tests support mainly. - %ENV is not any more passed to the application object constructor by its reference but copied over into a new hash instead. This is to avoid the side effect of enforced value stringification upon assignment into a %ENV key. - Unit::TestApp constructor now support cfgParams similar to engineParams and requestParams. - Bug fixes. --- UnitTestContrib/lib/Unit/FoswikiTestRole.pm | 10 +- UnitTestContrib/lib/Unit/TestApp.pm | 19 ++- UnitTestContrib/test/bin/TestRunner.pl | 11 +- UnitTestContrib/test/unit/ExtensionsTests.pm | 140 ++++++++++++++---- .../TestExtensions/Foswiki/Extension/Empty.pm | 51 ------- .../Foswiki/Extension/Sample.pm | 35 ----- .../Foswiki/Extension/Sample/Config.pm | 5 + core/lib/Foswiki/App.pm | 2 +- core/lib/Foswiki/Engine/CLI.pm | 4 +- core/lib/Foswiki/Extensions.pm | 92 +++++++----- 10 files changed, 192 insertions(+), 177 deletions(-) delete mode 100644 UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Empty.pm delete mode 100644 UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample.pm diff --git a/UnitTestContrib/lib/Unit/FoswikiTestRole.pm b/UnitTestContrib/lib/Unit/FoswikiTestRole.pm index 05bcefc20c..64e5365fde 100644 --- a/UnitTestContrib/lib/Unit/FoswikiTestRole.pm +++ b/UnitTestContrib/lib/Unit/FoswikiTestRole.pm @@ -47,10 +47,8 @@ has app => ( clearer => 1, isa => Foswiki::Object::isaCLASS( 'app', 'Unit::TestApp', noUndef => 1, ), default => sub { - if ( defined $Foswiki::app ) { - return $Foswiki::app; - } - return Unit::TestApp->new( env => \%ENV ); + return Unit::TestApp->new( env => { map { $_ => $ENV{$_} } keys %ENV }, + ); }, handles => [qw(create)], ); @@ -593,7 +591,9 @@ sub createNewFoswikiApp { $app->cfg->data->{Store}{Implementation} ||= 'Foswiki::Store::PlainFile'; $params{env} //= $app->cloneEnv; - my $newApp = Unit::TestApp->new( cfg => $app->cfg->clone, %params ); + my $cfgData = $app->cfg->clone->data; + my $newApp = + Unit::TestApp->new( cfgParams => { data => $cfgData, }, %params ); $this->app($newApp); $this->_fixupAppObjects; diff --git a/UnitTestContrib/lib/Unit/TestApp.pm b/UnitTestContrib/lib/Unit/TestApp.pm index 2d1c2779b7..70271c145d 100644 --- a/UnitTestContrib/lib/Unit/TestApp.pm +++ b/UnitTestContrib/lib/Unit/TestApp.pm @@ -41,6 +41,13 @@ has engineParams => ( default => sub { {} }, ); +# cfgParams hash is used to initialize a new cfg object. +has cfgParams => ( + is => 'rw', + lazy => 1, + default => sub { {} }, +); + # Hash of the test callbacks to be registered on the app object. has callbacks => ( is => 'rw', @@ -54,10 +61,6 @@ has _cbRegistered => ( default => 0, ); -before BUILDARGS => sub { - -}; - around BUILDARGS => sub { my $orig = shift; my $this = shift; @@ -141,6 +144,14 @@ around _prepareEngine => sub { return $orig->( $this, %{ $this->engineParams } ); }; +around _prepareConfig => sub { + my $orig = shift; + my $this = shift; + + my $cfg = $this->create( 'Foswiki::Config', %{ $this->cfgParams } ); + return $cfg; +}; + around handleRequest => sub { my $orig = shift; my $this = shift; diff --git a/UnitTestContrib/test/bin/TestRunner.pl b/UnitTestContrib/test/bin/TestRunner.pl index ed25ed63f7..ea2a0010bd 100755 --- a/UnitTestContrib/test/bin/TestRunner.pl +++ b/UnitTestContrib/test/bin/TestRunner.pl @@ -72,14 +72,11 @@ BEGIN my ( $app, $cfg ); local %ENV = %ENV; try { - my $env = \%ENV; - $env->{FOSWIKI_ACTION} = + $ENV{FOSWIKI_ACTION} = 'view'; # SMELL Shan't we add a 'test' action to the SwitchBoard? - $env->{FOSWIKI_ENGINE} = 'Foswiki::Engine::Test'; - $app = Unit::TestApp->new( - env => $env, - engineParams => { initialAttributes => { action => 'view', }, }, - ); + $ENV{FOSWIKI_ENGINE} = 'Foswiki::Engine::Test'; + my $env = { map { $_ => $ENV{$_} } keys %ENV }; + $app = Unit::TestApp->new( env => $env, ); $cfg = $app->cfg; } catch { diff --git a/UnitTestContrib/test/unit/ExtensionsTests.pm b/UnitTestContrib/test/unit/ExtensionsTests.pm index b0f5eb58c7..8333e2ad9b 100644 --- a/UnitTestContrib/test/unit/ExtensionsTests.pm +++ b/UnitTestContrib/test/unit/ExtensionsTests.pm @@ -1,37 +1,41 @@ # See bottom of file for license and copyright information -package Foswiki::ExtensionsTests::SampleClass { - use Foswiki::Class qw(extensible); - extends qw(Foswiki::Object); +package Foswiki::ExtensionsTests::SampleClass; - has allowFlowControl => ( - is => 'rw', - default => 0, - ); +use Foswiki::Class qw(extensible); +extends qw(Foswiki::Object); - pluggable testPluggableMethod => sub { - my $this = shift; +has allowFlowControl => ( + is => 'rw', + default => 0, +); - return wantarray - ? ( qw(This is a sample return array), @_ ) - : "This is a sample string {" . join( "}{", @_ ) . "}"; - }; -} +pluggable testPluggableMethod => sub { + my $this = shift; + + return wantarray + ? ( qw(This is a sample return array), @_ ) + : "This is a sample string {" . join( "}{", @_ ) . "}"; +}; package ExtensionsTests; use Assert; use Foswiki::Exception (); +use version 0.77; + use Foswiki::Class; extends qw(FoswikiFnTestCase); $| = 1; # List of all auto generated extension modules. -has autoGenerated => ( - is => 'rw', - default => sub { [] }, +has autoGenExt => ( + is => 'rw', + lazy => 1, + predicate => 1, + default => sub { [] }, ); around set_up => sub { @@ -39,16 +43,28 @@ around set_up => sub { my $this = shift; my $mKey = ( grep { /ExtensionsTests/ } keys %INC )[0]; - $this->app->env->{FOSWIKI_EXTLIBS} = + $ENV{FOSWIKI_EXTLIBS} = File::Spec->catdir( ( File::Spec->splitpath( $INC{$mKey} ) )[1], 'TestExtensions' ); # Disable all extensions generated by previous tests. - $this->app->env->{FOSWIKI_DISABLED_EXTENSIONS} = $this->autoGenerated; + $ENV{FOSWIKI_DISABLED_EXTENSIONS} = join ",", @{ $this->autoGenExt }; + + $orig->( $this, @_ ); $this->app->cfg->data->{DisableAllPlugins} = 1; +}; - $orig->( $this, @_ ); +around tear_down => sub { + my $orig = shift; + my $this = shift; + + # Disable any extensions a test has generated. + $this->_disableAllCurrentExtensions; + + $this->reCreateFoswikiApp; + + return $orig->( $this, @_ ); }; sub _getExtName { @@ -63,6 +79,8 @@ sub _genExtModules { my $this = shift; my ( $count, @extCode ) = @_; + ASSERT($count); + my @extNames; for ( 1 .. $count ) { my $extName = $this->_getExtName; @@ -72,6 +90,9 @@ package $extName; use Foswiki::Class qw(extension); extends qw(Foswiki::Extension); +use version 0.77; our \$VERSION = version->declare(0.0.1); +our \$API_VERSION = version->declare("2.99.0"); + Foswiki::Extensions::registerExtModule('$extName'); $code @@ -87,7 +108,7 @@ EXT } # Remember what extensions have been autogenerated. - push @{ $this->autoGenerated }, @extNames; + push @{ $this->autoGenExt }, @extNames; return @extNames; } @@ -135,8 +156,12 @@ sub test_orderedList { sub test_manual_disable { my $this = shift; + $this->_disableAllCurrentExtensions; + my @ext = $this->_genExtModules(2); - $this->app->env->{FOSWIKI_DISABLED_EXTENSIONS} = $ext[1]; + + push @{ $this->app->env->{FOSWIKI_DISABLED_EXTENSIONS} }, $ext[1]; + $this->reCreateFoswikiApp; $this->assert_not_null( @@ -154,8 +179,11 @@ sub test_manual_disable { sub test_depend_on_manual_disable { my $this = shift; + $this->_disableAllCurrentExtensions; + my @ext = $this->_genExtModules(4); - $this->app->env->{FOSWIKI_DISABLED_EXTENSIONS} = $ext[1]; + + push @{ $this->app->env->{FOSWIKI_DISABLED_EXTENSIONS} }, $ext[1]; $this->_setExtDependencies( $ext[3] => $ext[2], $ext[2] => $ext[1], @@ -226,8 +254,6 @@ plugBefore 'Foswiki::ExtensionsTests::SampleClass::testPluggableMethod' => sub { my $this = shift; my ($params) = @_; - $this->_traceMsg(""); - # We expect at least to parameters to be passed in. $params->{args}[1] = "ext1ArgFromBefore"; }; @@ -237,8 +263,6 @@ plugAround 'Foswiki::ExtensionsTests::SampleClass::testPluggableMethod' => sub { my $this = shift; my ($params) = @_; - $this->_traceMsg(""); - # Only if flow control is allowed by the calling test code. if ($params->{object}->allowFlowControl) { my $rc = 'ext2ReturnFromAround'; @@ -264,8 +288,6 @@ plugBefore 'Foswiki::ExtensionsTests::SampleClass::testPluggableMethod' => sub { my $this = shift; my ($params) = @_; - $this->_traceMsg(""); - $params->{ext3Flag1} = 1; $params->{ext3Flag2} = 1; }; @@ -274,8 +296,6 @@ plugAfter 'Foswiki::ExtensionsTests::SampleClass::testPluggableMethod' => sub { my $this = shift; my ($params) = @_; - $this->_traceMsg(""); - foreach my $flag (qw(ext3Flag1 ext3Flag2)) { if ($params->{$flag}) { my $rc = "with_$flag"; @@ -347,6 +367,64 @@ EXT3 [@rc] ); } +sub test_API_VERSION { + my $this = shift; + + $this->_disableAllCurrentExtensions; + + my @ext = $this->_genExtModules(2); + my $ns0 = Foswiki::getNS( $ext[0] ); + + *{ $ns0->{API_VERSION} } = \version->declare("v2.98.0"); + + $this->reCreateFoswikiApp; + + ASSERT( + !$this->app->extensions->extEnabled( $ext[0] ), + "Extension with API version " + . Foswiki::fetchGlobal("\$$ext[0]::API_VERSION") + . " must have been disabled but it's not" + ); + + ASSERT( + $this->app->extensions->extEnabled( $ext[1] ), + "Extension with API version " + . Foswiki::fetchGlobal("\$$ext[1]::API_VERSION") + . " must have been enabled but it's not" + ); +} + +sub test_subClassing { + my $this = shift; + + $this->_disableAllCurrentExtensions; + + my $configExt = $this->_genExtModules( 1, <<'CFG_EXT'); + +extClass 'Foswiki::Config' => 'Foswiki::Extension::Sample::Config'; + +CFG_EXT + + $this->reCreateFoswikiApp; + + $this->assert( + ref( $Foswiki::cfg{'.version'} ) eq 'version', + "Config key '.version' is expected to be a version object" + ); + $this->assert_equals( + $Foswiki::Extension::Sample::Config::VERSION, + $Foswiki::cfg{'.version'}, + "Config key '.version' returned unexpected version number" + ); + + my $testKey = "_ThisKeyMustNotBeUsed"; + $Foswiki::cfg{$testKey} = my $testVal = + "Число пі дорівнює 3.1415926"; + $this->assert_equals( $testVal, $this->app->cfg->data->{$testKey}, +"Value stored to %Foswiki::cfg doesn't match with fetched from application object cfg attribute" + ); +} + 1; __END__ Foswiki - The Free and Open Source Wiki, http://foswiki.org/ diff --git a/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Empty.pm b/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Empty.pm deleted file mode 100644 index 44f808d853..0000000000 --- a/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Empty.pm +++ /dev/null @@ -1,51 +0,0 @@ -# See bottom of file for license and copyright information - -package Foswiki::Extension::Empty; - -use Foswiki::Class qw(extension); -extends qw(Foswiki::Extension); - -use version 0.77; our $VERSION = version->declare(0.0.1); -our $API_VERSION = version->declare("2.99.0"); - -#extAfter qw(Sample); - -#extBefore qw(Test1 Foswiki::Extension::Test2); - -plugAfter 'Foswiki::Config::plugMethod' => sub { - my $this = shift; - my ($params) = @_; - - $this->_traceMsg( - "This is plugMethod after processing. ", - "The return value is ", - ( - exists $params->{rc} - ? ( defined $params->{rc} ? $params->{rc} : '*undef*' ) - : '*missing*' - ) - ); - if ( defined $params->{rc} && ref( $params->{rc} ) eq 'ARRAY' ) { - push @{ $params->{rc} }, 'Additional from ', __PACKAGE__; - } -}; - -1; -__END__ -Foswiki - The Free and Open Source Wiki, http://foswiki.org/ - -Copyright (C) 2016 Foswiki Contributors. Foswiki Contributors -are listed in the AUTHORS file in the root of this distribution. -NOTE: Please extend that file, not this notice. - -This program is free software; you can redistribute it and/or -modify it under the terms of the GNU General Public License -as published by the Free Software Foundation; either version 2 -of the License, or (at your option) any later version. For -more details read LICENSE in the root of this distribution. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - -As per the GPL, removal of this notice is prohibited. diff --git a/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample.pm b/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample.pm deleted file mode 100644 index 1479f74edf..0000000000 --- a/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample.pm +++ /dev/null @@ -1,35 +0,0 @@ -# See bottom of file for license and copyright information - -package Foswiki::Extension::Sample; - -use Data::Dumper; - -use Foswiki::Class qw(extension); -extends qw(Foswiki::Extension); - -use version 0.77; our $VERSION = version->declare(0.0.1); -our $API_VERSION = version->declare("2.99.0"); - -extClass 'Foswiki::Config', 'Foswiki::Extension::Sample::Config'; - -extBefore qw(Empty); - -1; -__END__ -Foswiki - The Free and Open Source Wiki, http://foswiki.org/ - -Copyright (C) 2016 Foswiki Contributors. Foswiki Contributors -are listed in the AUTHORS file in the root of this distribution. -NOTE: Please extend that file, not this notice. - -This program is free software; you can redistribute it and/or -modify it under the terms of the GNU General Public License -as published by the Free Software Foundation; either version 2 -of the License, or (at your option) any later version. For -more details read LICENSE in the root of this distribution. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - -As per the GPL, removal of this notice is prohibited. diff --git a/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample/Config.pm b/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample/Config.pm index 984a5dd30e..78ed2f61d2 100644 --- a/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample/Config.pm +++ b/UnitTestContrib/test/unit/TestExtensions/Foswiki/Extension/Sample/Config.pm @@ -15,6 +15,9 @@ package Foswiki::Extension::Sample::TiedConfig { sub FETCH { my ( $this, $key ) = @_; ASSERT( defined $this->{cfg} ) if DEBUG; + if ( $key eq '.version' ) { + return $Foswiki::Extension::Sample::Config::VERSION; + } $this->trace("FETCH{$key}"); return $this->{cfg}->data->{$key}; } @@ -89,6 +92,8 @@ package Foswiki::Extension::Sample::Config; use Carp; use Moo::Role; +use version; our $VERSION = version->declare("v0.0.1"); + around assignGLOB => sub { my $orig = shift; my $this = shift; diff --git a/core/lib/Foswiki/App.pm b/core/lib/Foswiki/App.pm index 3df844e5aa..9d525652d5 100644 --- a/core/lib/Foswiki/App.pm +++ b/core/lib/Foswiki/App.pm @@ -1593,7 +1593,7 @@ sub _checkReqCache { # implicit untaint required, because $cache may be used in a # filename. Note that the cache serialises the method and path_info, # which will be restored. - Foswiki::Request::Cache->new->load( $1, $req ); + $this->create('Foswiki::Request::Cache')->load( $1, $req ); } } diff --git a/core/lib/Foswiki/Engine/CLI.pm b/core/lib/Foswiki/Engine/CLI.pm index 76af0c044f..415027a4a0 100644 --- a/core/lib/Foswiki/Engine/CLI.pm +++ b/core/lib/Foswiki/Engine/CLI.pm @@ -119,8 +119,8 @@ around _preparePath => sub { $action = ( File::Spec->splitpath($0) )[2]; } if ( $this->has_path_info ) { - $path_info = $this->pathInfo; - $this->clear_pathInfo; + $path_info = $this->path_info; + $this->clear_path_info; } $action = 'view' unless defined $this->app->cfg->data->{SwitchBoard}{$action}; diff --git a/core/lib/Foswiki/Extensions.pm b/core/lib/Foswiki/Extensions.pm index e7e0c4ac1b..1fc6e0e3d2 100644 --- a/core/lib/Foswiki/Extensions.pm +++ b/core/lib/Foswiki/Extensions.pm @@ -166,39 +166,34 @@ sub extEnabled { return defined $this->disabledExtensions->{$extName} ? undef : $extName; } -sub checkVersion { +sub isBadVersion { my $this = shift; my ($extName) = @_; my @apiScalar = grep { /::API_VERSION$/ } Devel::Symdump->scalars($extName); - Foswiki::Exception::Ext::Load->throw( - extension => $extName, - reason => "No \$API_VERSION scalar defined in $extName", - ) unless @apiScalar; + return "No \$API_VERSION scalar defined in $extName" + unless @apiScalar; my $api_ver = Foswiki::fetchGlobal( '$' . $apiScalar[0] ); - Foswiki::Exception::Ext::Load->throw( - extension => $extName, - reason => "Failed to fetch \$API_VERSION", - ) unless defined $api_ver; - - Foswiki::Exception::Ext::Load->throw( - extension => $extName, - reason => "Declared API version " - . $api_ver - . " is lower than supported " - . $MIN_VERSION, - ) if $api_ver < $MIN_VERSION; - - Foswiki::Exception::Ext::Load->throw( - extension => $extName, - reason => "Declared API version " - . $api_ver - . " is higher than supported " - . $VERSION, - ) if $api_ver > $VERSION; + return "Failed to fetch \$API_VERSION" + unless defined $api_ver; + + return + "Declared API version " + . $api_ver + . " is lower than supported " + . $MIN_VERSION + if $api_ver < $MIN_VERSION; + + return + "Declared API version " + . $api_ver + . " is higher than supported " + . $VERSION + if $api_ver > $VERSION; + return ''; } sub _loadExtModule { @@ -209,7 +204,6 @@ sub _loadExtModule { try { Foswiki::load_class($extModule); - $this->checkVersion($extModule); registerExtModule($extModule); } catch { @@ -254,7 +248,7 @@ sub _loadFromSubDir { } } catch { - # We don't really fail upon extension load because this ain't fatal + # We don't really fail upon extension load because this isn't fatal # in neither way. What bad could unloaded extension cause? push @{ $this->_errors }, Foswiki::Exception::Ext::Load->transmute( $_, 1, @@ -480,12 +474,12 @@ are extension names, values are text reasons for disabling the extension. =cut sub prepareDisabledExtensions { - my $this = shift; - my $env = $this->app->env; - my $envVar = 'FOSWIKI_DISABLED_EXTENSIONS'; - my $disabled = $env->{$envVar} // ''; - my %list; - if ( my $reftype = ref($disabled) ) { + my $this = shift; + my $env = $this->app->env; + my $envVar = 'FOSWIKI_DISABLED_EXTENSIONS'; + my $envDisabled = $env->{$envVar} // ''; + my %disabled; + if ( my $reftype = ref($envDisabled) ) { Foswiki::Exception::Fatal->throw( text => "Environment variable $envVar is a ref to " . $reftype @@ -493,13 +487,31 @@ sub prepareDisabledExtensions { unless $reftype eq 'ARRAY'; } else { - $disabled = [ split /,/, $disabled ]; + $envDisabled = [ split /,/, $envDisabled ]; } - %list = - map { $_ => "Disabled by $envVar environment variable." } @$disabled; + %disabled = + map { + $this->normalizeExtName($_) => + "Disabled by $envVar environment variable." + } @$envDisabled; - return \%list; + foreach my $ext (@extModules) { + my $extMod = $this->normalizeExtName($ext); + my $reason; + + unless ( $disabled{$extMod} ) { + + # Disable API-incompatible modules + $reason = $this->isBadVersion($extMod); + + if ($reason) { + $disabled{$extMod} = $reason; + } + } + } + + return \%disabled; } sub prepareDependencies { @@ -525,11 +537,9 @@ sub prepareRegisteredClasses { my %ext2class; foreach my $coreClass ( keys %extSubClasses ) { foreach my $registration ( @{ $extSubClasses{$coreClass} } ) { - my $extName = - $this->extEnabled( - $this->normalizeExtName( $registration->{extension} ) ); + my $extName = $this->extEnabled( $registration->{extension} ); - next unless defined $extName; + next unless $extName; if ( defined $ext2class{$extName}{$coreClass} ) {