Rewrite fixes for RT#48555

diff --git a/Changes b/Changes
index 115a0d3..f6bd37a 100644 (file)
--- a/Changes
+++ b/Changes
@@ -9,11 +9,9 @@
        - Catalyst::Engine::FastCGI - relax the check for versions of Microsoft
          IIS. Provides compatibility with Windows 2008 R2 as well as
          (hopefully) future versions.
-       - In tests which depend on the values of environment variables, create
-         a mock application which is then thrown away before blanking the
-         environment. This ensures that anything dynamically loaded
-         (e.g. HTML/Parser/Parser.dll) is loaded before the environment is
-         trashed. RT#48555
+       - In tests which depend on the values of environment variables,
+         localise the environment, then delete only relevant environment
+         variables (RT#48555)
 
   Refactoring / cleanups:
        - Deleted the Restarter engine and its Watcher code. Use the

diff --git a/t/unit_core_setup.t b/t/unit_core_setup.t
index 8f41a2b..cbc5aac 100644 (file)
--- a/t/unit_core_setup.t
+++ b/t/unit_core_setup.t
@@ -26,16 +26,17 @@ sub build_test_app_with_setup {
     return $name;
 }
 
-build_test_app_with_setup('UnusedApp'); # Mock an app before localizing %ENV
-                                  # to ensure that anything which is dynamically
-                                  # loaded from the enviornment is loaded
+local %ENV = %ENV;
 
-local %ENV; # Don't allow env variables to mess us up.
+# Remove all relevant env variables to avoid accidental fail
+foreach my $name (grep { /^(CATALYST|TESTAPP)/ } keys %ENV) {
+    delete $ENV{$name};
+}
 
 {
-    my $app = build_test_app_with_setup('MyTestDebug', '-Debug');
+    my $app = build_test_app_with_setup('TestAppMyTestDebug', '-Debug');
 
-    ok my $c = MyTestDebug->new, 'Get debug app object';
+    ok my $c = $app->new, 'Get debug app object';
     ok my $log = $c->log, 'Get log object';
     isa_ok $log, 'Catalyst::Log', 'It should be a Catalyst::Log object';
     ok $log->is_warn, 'Warnings should be enabled';
@@ -47,7 +48,7 @@ local %ENV; # Don't allow env variables to mess us up.
 }
 
 {
-    my $app = build_test_app_with_setup('MyTestLogParam', '-Log=warn,error,fatal');
+    my $app = build_test_app_with_setup('TestAppMyTestLogParam', '-Log=warn,error,fatal');
 
     ok my $c = $app->new, 'Get log app object';
     ok my $log = $c->log, 'Get log object';
@@ -60,7 +61,7 @@ local %ENV; # Don't allow env variables to mess us up.
     ok !$c->debug, 'Catalyst debugging is off';
 }
 {
-    my $app = build_test_app_with_setup('MyTestNoParams');
+    my $app = build_test_app_with_setup('TestAppMyTestNoParams');
 
     ok my $c = $app->new, 'Get log app object';
     ok my $log = $c->log, 'Get log object';
@@ -76,12 +77,12 @@ my $log_meta = Class::MOP::Class->create_anon_class(
     methods => { map { $_ => sub { 0 } } qw/debug error fatal info warn/ },
 );
 {
-    package MyTestAppWithOwnLogger;
+    package TestAppWithOwnLogger;
     use base qw/Catalyst/;
     __PACKAGE__->log($log_meta->new_object);
     __PACKAGE__->setup('-Debug');
 }
 
-ok my $c = MyTestAppWithOwnLogger->new, 'Get with own logger app object';
+ok my $c = TestAppWithOwnLogger->new, 'Get with own logger app object';
 ok $c->debug, '$c->debug is true';
 

diff --git a/t/unit_core_setup_log.t b/t/unit_core_setup_log.t
index 401ea99..1406944 100644 (file)
--- a/t/unit_core_setup_log.t
+++ b/t/unit_core_setup_log.t
@@ -27,10 +27,15 @@ sub test_log_object {
     }
 }
 
-local %ENV; # Ensure blank or someone, somewhere will fail..
+local %ENV = %ENV;
+
+# Remove all relevant env variables to avoid accidental fail
+foreach my $name (grep { /^(CATALYST|TESTAPP)/ } keys %ENV) {
+    delete $ENV{$name};
+}
 
 {
-    my $app = mock_app('TestLogAppParseLevels');
+    my $app = mock_app('TestAppParseLogLevels');
     $app->setup_log('error,warn');
     ok !$app->debug, 'Not in debug mode';
     test_log_object($app->log,
@@ -42,8 +47,9 @@ local %ENV; # Ensure blank or someone, somewhere will fail..
     );
 }
 {
-    local %ENV = ( CATALYST_DEBUG => 1 );
-    my $app = mock_app('TestLogAppDebugEnvSet');
+    local %ENV = %ENV;
+    $ENV{CATALYST_DEBUG} = 1;
+    my $app = mock_app('TestAppLogDebugEnvSet');
     $app->setup_log('');
     ok $app->debug, 'In debug mode';
     test_log_object($app->log,
@@ -55,8 +61,9 @@ local %ENV; # Ensure blank or someone, somewhere will fail..
     );
 }
 {
-    local %ENV = ( CATALYST_DEBUG => 0 );
-    my $app = mock_app('TestLogAppDebugEnvUnset');
+    local %ENV = %ENV;
+    $ENV{CATALYST_DEBUG} = 0;
+    my $app = mock_app('TestAppLogDebugEnvUnset');
     $app->setup_log('warn');
     ok !$app->debug, 'Not In debug mode';
     test_log_object($app->log,
@@ -68,7 +75,7 @@ local %ENV; # Ensure blank or someone, somewhere will fail..
     );
 }
 {
-    my $app = mock_app('TestLogAppEmptyString');
+    my $app = mock_app('TestAppLogEmptyString');
     $app->setup_log('');
     ok !$app->debug, 'Not In debug mode';
     # Note that by default, you get _all_ the log levels turned on
@@ -81,7 +88,7 @@ local %ENV; # Ensure blank or someone, somewhere will fail..
     );
 }
 {
-    my $app = mock_app('TestLogAppDebugOnly');
+    my $app = mock_app('TestAppLogDebugOnly');
     $app->setup_log('debug');
     ok $app->debug, 'In debug mode';
     test_log_object($app->log,

diff --git a/t/unit_core_setup_stats.t b/t/unit_core_setup_stats.t
index ed79147..9f7a8d3 100644 (file)
--- a/t/unit_core_setup_stats.t
+++ b/t/unit_core_setup_stats.t
@@ -22,44 +22,47 @@ my $mock_log = Class::MOP::Class->create_anon_class(
 sub mock_app {
     my $name = shift;
     %log_messages = (); # Flatten log messages.
-    print "Setting up mock application: $name\n";
+    diag "Setting up mock application: $name";
     my $meta = Moose->init_meta( for_class => $name );
     $meta->superclasses('Catalyst');
     $meta->add_method('log', sub { $mock_log }); 
     return $meta->name;
 }
 
-mock_app('UnusedApp'); # Mock an app before localizing %ENV
-                       # to ensure that anything which is dynamically
-                       # loaded from the enviornment is loaded
+local %ENV = %ENV;
 
-local %ENV; # Ensure blank or someone, somewhere will fail..
+# Remove all relevant env variables to avoid accidental fail
+foreach my $name (grep { /^(CATALYST|TESTAPP)/ } keys %ENV) {
+    delete $ENV{$name};
+}
 
 {
-    my $app = mock_app('TestNoStats');
+    my $app = mock_app('TestAppNoStats');
     $app->setup_stats();
     ok !$app->use_stats, 'stats off by default';
 }
 {
-    my $app = mock_app('TestStats');
+    my $app = mock_app('TestAppStats');
     $app->setup_stats(1);
     ok $app->use_stats, 'stats on if you say >setup_stats(1)';
 }
 {
-    my $app = mock_app('TestStatsDebugTurnsStatsOn');
+    my $app = mock_app('TestAppStatsDebugTurnsStatsOn');
     $app->meta->add_method('debug' => sub { 1 });
     $app->setup_stats();
     ok $app->use_stats, 'debug on turns stats on';
 }
 {
-    local %ENV = ( CATALYST_STATS => 1 );
-    my $app = mock_app('TestStatsAppStatsEnvSet');
+    local %ENV = %ENV;
+    $ENV{CATALYST_STATS} = 1;
+    my $app = mock_app('TestAppStatsEnvSet');
     $app->setup_stats();
     ok $app->use_stats, 'ENV turns stats on';
 }
 {
-    local %ENV = ( CATALYST_STATS => 0 );
-    my $app = mock_app('TestStatsAppStatsEnvUnset');
+    local %ENV = %ENV;
+    $ENV{CATALYST_STATS} = 0;
+    my $app = mock_app('TestAppStatsEnvUnset');
     $app->meta->add_method('debug' => sub { 1 });
     $app->setup_stats(1);
     ok !$app->use_stats, 'ENV turns stats off, even when debug on and ->setup_stats(1)';