Add [tests] config option to enable logging in tests. (#13335)

* Add [tests] config option to enable logging in tests.

* Allow tests:run/tests:run-ui commands to enable logging for individual runs + during tests only log to file.

* Remove Fixture field

* fix failing test

* fixing build

* Fix another failure.

* Fix an other test.

15 files changed, 85 insertions, 15 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 035a634074..22253064a6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,12 @@ This is the Developer Changelog for Matomo platform developers. All changes in o
 
 The Product Changelog at **[matomo.org/changelog](https://matomo.org/changelog)** lets you see more details about any Matomo release, such as the list of new guides and FAQs, security fixes, and links to all closed issues. 
 
+## Matomo 3.6.1
+
+### New Developer Features
+
+* Logging to a file can now be easily enabled during tests. A new `[tests] enable_logging` INI option has been added, which you can set to `1` to enable logging for all tests. The `tests:run` and `tests:run-ui` commands now both have an `--enable-logging` option to enable logging for a specific run.
+
 ## Matomo 3.6.0
 
 ### New Features
diff --git a/config/environment/test.php b/config/environment/test.php
index 2ac39275b8..785eb15ce4 100644
--- a/config/environment/test.php
+++ b/config/environment/test.php
@@ -8,7 +8,26 @@ use Piwik\Tests\Framework\Mock\TestConfig;
 return array(
 
     // Disable logging
-    'Psr\Log\LoggerInterface' => DI\object('Psr\Log\NullLogger'),
+    'Psr\Log\LoggerInterface' => \DI\decorate(function ($previous, ContainerInterface $c) {
+        $enableLogging = $c->get('ini.tests.enable_logging') == 1 || !empty(getenv('MATOMO_TESTS_ENABLE_LOGGING'));
+        if ($enableLogging) {
+            return $previous;
+        } else {
+            return $c->get(\Psr\Log\NullLogger::class);
+        }
+    }),
+
+    'Tests.log.allowAllHandlers' => false,
+
+    'log.handlers' => \DI\decorate(function ($previous, ContainerInterface $c) {
+        if ($c->get('Tests.log.allowAllHandlers')) {
+            return $previous;
+        }
+
+        return [
+            $c->get('Piwik\Plugins\Monolog\Handler\FileHandler'),
+        ];
+    }),
 
     'Piwik\Cache\Backend' => function () {
         return \Piwik\Cache::buildBackend('file');
diff --git a/config/global.ini.php b/config/global.ini.php
index fd7b26c826..17df341682 100644
--- a/config/global.ini.php
+++ b/config/global.ini.php
@@ -72,6 +72,7 @@ http_host   = localhost
 remote_addr = "127.0.0.1"
 request_uri = "@REQUEST_URI@"
 port =
+enable_logging = 0
 
 ; access key and secret as listed in AWS -> IAM -> Users
 aws_accesskey = ""
diff --git a/plugins/Monolog/config/tracker.php b/plugins/Monolog/config/tracker.php
index c9d0044b35..620101ff7a 100644
--- a/plugins/Monolog/config/tracker.php
+++ b/plugins/Monolog/config/tracker.php
@@ -10,13 +10,13 @@ function isTrackerDebugEnabled(ContainerInterface $c)
 
 return array(
 
-    'Psr\Log\LoggerInterface' => function (ContainerInterface $c) {
+    'Psr\Log\LoggerInterface' => \DI\decorate(function ($previous, ContainerInterface $c) {
         if (isTrackerDebugEnabled($c)) {
-            return $c->get('Monolog\Logger');
+            return $previous;
         } else {
             return new \Psr\Log\NullLogger();
         }
-    },
+    }),
 
     'log.handler.classes' => DI\decorate(function ($previous) {
         if (isset($previous['screen'])) {
diff --git a/plugins/Monolog/tests/Integration/LogTest.php b/plugins/Monolog/tests/Integration/LogTest.php
index 0389b504f0..744b59b41c 100644
--- a/plugins/Monolog/tests/Integration/LogTest.php
+++ b/plugins/Monolog/tests/Integration/LogTest.php
@@ -258,7 +258,8 @@ class LogTest extends IntegrationTestCase
             'ini.log.log_level' => $level,
             'ini.log.string_message_format' => self::STRING_MESSAGE_FORMAT,
             'ini.log.logger_file_path' => self::getLogFileLocation(),
-            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger')
+            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger'),
+            'Tests.log.allowAllHandlers' => true,
         ));
         $newEnv->init();
 
diff --git a/plugins/Monolog/tests/System/TrackerLoggingTest.php b/plugins/Monolog/tests/System/TrackerLoggingTest.php
index 0c85338850..661c69e710 100644
--- a/plugins/Monolog/tests/System/TrackerLoggingTest.php
+++ b/plugins/Monolog/tests/System/TrackerLoggingTest.php
@@ -10,6 +10,7 @@ namespace Piwik\Plugins\Monolog\tests\System;
 
 use Piwik\Config;
 use Piwik\Date;
+use Piwik\Plugins\Monolog\Handler\EchoHandler;
 use Piwik\Tests\Framework\Fixture;
 use Piwik\Tests\Framework\TestCase\SystemTestCase;
 use Piwik\Tests\Framework\TestingEnvironmentVariables;
@@ -92,7 +93,14 @@ DEBUG:   'apiv' => '1',", $response);
     public static function provideContainerConfigBeforeClass()
     {
         return array(
-            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger')
+            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger'),
+            Config::class => \DI\decorate(function (Config $config) {
+                $config->tests['enable_logging'] = 1;
+                return $config;
+            }),
+            'log.handlers' => [
+                \DI\get(EchoHandler::class),
+            ],
         );
     }
 
diff --git a/plugins/TestRunner/Commands/TestsRun.php b/plugins/TestRunner/Commands/TestsRun.php
index 76dd65583a..d60b5afc25 100644
--- a/plugins/TestRunner/Commands/TestsRun.php
+++ b/plugins/TestRunner/Commands/TestsRun.php
@@ -34,6 +34,7 @@ class TestsRun extends ConsoleCommand
         $this->addOption('group', null, InputOption::VALUE_REQUIRED, 'Run only a specific test group. Separate multiple groups by comma, for instance core,plugins', '');
         $this->addOption('file', null, InputOption::VALUE_REQUIRED, 'Execute tests within this file. Should be a path relative to the tests/PHPUnit directory.');
         $this->addOption('testsuite', null, InputOption::VALUE_REQUIRED, 'Execute tests of a specific test suite, for instance unit, integration or system.');
+        $this->addOption('enable-logging', null, InputOption::VALUE_NONE, 'Enable logging to the configured log file during tests.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
@@ -43,6 +44,7 @@ class TestsRun extends ConsoleCommand
         $magics  = $input->getArgument('variables');
         // @todo remove piwik-domain fallback in Matomo 4
         $matomoDomain = $input->getOption('matomo-domain') ?: $input->getOption('piwik-domain');
+        $enableLogging = $input->getOption('enable-logging');
 
         $groups = $this->getGroupsFromString($groups);
 
@@ -115,7 +117,7 @@ class TestsRun extends ConsoleCommand
             }
         }
 
-        $this->executeTests($matomoDomain, $suite, $testFile, $groups, $options, $command, $output);
+        $this->executeTests($matomoDomain, $suite, $testFile, $groups, $options, $command, $output, $enableLogging);
 
         return $this->returnVar;
     }
@@ -160,12 +162,12 @@ class TestsRun extends ConsoleCommand
         return $this->fixPathToTestFileOrDirectory($testFile);
     }
 
-    private function executeTests($piwikDomain, $suite, $testFile, $groups, $options, $command, OutputInterface $output)
+    private function executeTests($piwikDomain, $suite, $testFile, $groups, $options, $command, OutputInterface $output, $enableLogging)
     {
         if (empty($suite) && empty($groups) && empty($testFile)) {
             foreach ($this->getTestsSuites() as $suite) {
                 $suite = $this->buildTestSuiteName($suite);
-                $this->executeTests($piwikDomain, $suite, $testFile, $groups, $options, $command, $output);
+                $this->executeTests($piwikDomain, $suite, $testFile, $groups, $options, $command, $output, $enableLogging);
             }
 
             return;
@@ -177,15 +179,18 @@ class TestsRun extends ConsoleCommand
             $params = $params . " " . $testFile;
         }
 
-        $this->executeTestRun($piwikDomain, $command, $params, $output);
+        $this->executeTestRun($piwikDomain, $command, $params, $output, $enableLogging);
     }
 
-    private function executeTestRun($piwikDomain, $command, $params, OutputInterface $output)
+    private function executeTestRun($piwikDomain, $command, $params, OutputInterface $output, $enableLogging)
     {
         $envVars = '';
         if (!empty($piwikDomain)) {
             $envVars .= "PIWIK_DOMAIN=$piwikDomain";
         }
+        if (!empty($enableLogging)) {
+            $envVars .= " MATOMO_TESTS_ENABLE_LOGGING=1";
+        }
 
         $cmd = $this->getCommand($envVars, $command, $params);
         $output->writeln('Executing command: <info>' . $cmd . '</info>');
diff --git a/plugins/TestRunner/Commands/TestsRunUI.php b/plugins/TestRunner/Commands/TestsRunUI.php
index 3c6fc15aa5..3a59994dff 100644
--- a/plugins/TestRunner/Commands/TestsRunUI.php
+++ b/plugins/TestRunner/Commands/TestsRunUI.php
@@ -11,6 +11,7 @@ use Piwik\AssetManager;
 use Piwik\Config;
 use Piwik\Plugin\ConsoleCommand;
 use Piwik\Tests\Framework\Fixture;
+use Piwik\Tests\Framework\TestingEnvironmentVariables;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -39,6 +40,7 @@ class TestsRunUI extends ConsoleCommand
         $this->addOption('store-in-ui-tests-repo', null, InputOption::VALUE_NONE, "For tests");
         $this->addOption('debug', null, InputOption::VALUE_NONE, "Enable phantomjs debugging");
         $this->addOption('extra-options', null, InputOption::VALUE_REQUIRED, "Extra options to pass to phantomjs.");
+        $this->addOption('enable-logging', null, InputOption::VALUE_NONE, 'Enable logging to the configured log file during tests.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
@@ -58,6 +60,7 @@ class TestsRunUI extends ConsoleCommand
         $debug = $input->getOption('debug');
         // @todo remove piwik-domain fallback in Matomo 4
         $matomoDomain = $input->getOption('matomo-domain') ?: $input->getOption('piwik-domain');
+        $enableLogging = $input->getOption('enable-logging');
 
         if (!$skipDeleteAssets) {
             AssetManager::getInstance()->removeMergedAssets();
@@ -112,6 +115,10 @@ class TestsRunUI extends ConsoleCommand
             $phantomJsOptions[] = "--debug=true";
         }
 
+        if ($enableLogging) {
+            $options[] = '--enable-logging';
+        }
+
         $phantomJsOptions[] = "--ignore-ssl-errors=true";
 
         if ($extraOptions) {
diff --git a/plugins/TestRunner/Commands/TestsSetupFixture.php b/plugins/TestRunner/Commands/TestsSetupFixture.php
index ae2554d52b..9675a6d8be 100644
--- a/plugins/TestRunner/Commands/TestsSetupFixture.php
+++ b/plugins/TestRunner/Commands/TestsSetupFixture.php
@@ -87,6 +87,7 @@ class TestsSetupFixture extends ConsoleCommand
             "Used by UI tests. Sets the \$_SERVER global variable from a JSON string.");
         $this->addOption('plugins', null, InputOption::VALUE_REQUIRED,
             "Used by UI tests. Comma separated list of plugin names to activate and install when setting up a fixture.");
+        $this->addOption('enable-logging', null, InputOption::VALUE_NONE, 'If enabled, tests will log to the configured log file.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
@@ -95,6 +96,10 @@ class TestsSetupFixture extends ConsoleCommand
             define('PIWIK_TEST_MODE', true);
         }
 
+        if ($input->getOption('enable-logging')) {
+            putenv("MATOMO_TESTS_ENABLE_LOGGING=1");
+        }
+
         Environment::setGlobalEnvironmentManipulator(new TestingEnvironmentManipulator(new TestingEnvironmentVariables()));
 
         $serverGlobal = $input->getOption('server-global');
diff --git a/tests/PHPUnit/Framework/Fixture.php b/tests/PHPUnit/Framework/Fixture.php
index dc8983ecec..4f046c6776 100644
--- a/tests/PHPUnit/Framework/Fixture.php
+++ b/tests/PHPUnit/Framework/Fixture.php
@@ -215,6 +215,10 @@ class Fixture extends \PHPUnit_Framework_Assert
             $testEnv->$name = $value;
         }
 
+        if (!empty(getenv('MATOMO_TESTS_ENABLE_LOGGING'))) {
+            $testEnv->environmentVariables['MATOMO_TESTS_ENABLE_LOGGING'] = '1';
+        }
+
         $testEnv->save();
 
         $this->createEnvironmentInstance();
diff --git a/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php b/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php
index b2b89f5ba0..ff75c429de 100644
--- a/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php
+++ b/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php
@@ -90,6 +90,12 @@ class TestingEnvironmentManipulator implements EnvironmentManipulator
             }
         }
 
+        if ($this->vars->environmentVariables) {
+            foreach ($this->vars->environmentVariables as $key => $value) {
+                putenv("$key=$value");
+            }
+        }
+
         if ($this->vars->hostOverride) {
             \Piwik\Url::setHost($this->vars->hostOverride);
         }
diff --git a/tests/PHPUnit/Framework/TestingEnvironmentVariables.php b/tests/PHPUnit/Framework/TestingEnvironmentVariables.php
index ef4b34449e..6ab3ce3ef9 100644
--- a/tests/PHPUnit/Framework/TestingEnvironmentVariables.php
+++ b/tests/PHPUnit/Framework/TestingEnvironmentVariables.php
@@ -24,9 +24,10 @@ class TestingEnvironmentVariables
         $this->reload();
     }
 
-    public function __get($key)
+    public function &__get($key)
     {
-        return isset($this->behaviorOverrideProperties[$key]) ? $this->behaviorOverrideProperties[$key] : null;
+        $result =& $this->behaviorOverrideProperties[$key];
+        return $result;
     }
 
     public function __set($key, $value)
diff --git a/tests/PHPUnit/Integration/ArchiveWebTest.php b/tests/PHPUnit/Integration/ArchiveWebTest.php
index 97ec66646e..21398dfb0c 100644
--- a/tests/PHPUnit/Integration/ArchiveWebTest.php
+++ b/tests/PHPUnit/Integration/ArchiveWebTest.php
@@ -93,7 +93,8 @@ class ArchiveWebTest extends SystemTestCase
     public static function provideContainerConfigBeforeClass()
     {
         return array(
-            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger')
+            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger'),
+            'Tests.log.allowAllHandlers' => true,
         );
     }
 }
diff --git a/tests/PHPUnit/System/TrackerTest.php b/tests/PHPUnit/System/TrackerTest.php
index 65ab91cc08..7f43afbe2f 100644
--- a/tests/PHPUnit/System/TrackerTest.php
+++ b/tests/PHPUnit/System/TrackerTest.php
@@ -392,7 +392,8 @@ class TrackerTest extends IntegrationTestCase
     public static function provideContainerConfigBeforeClass()
     {
         return array(
-            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger')
+            'Psr\Log\LoggerInterface' => \DI\get('Monolog\Logger'),
+            'Tests.log.allowAllHandlers' => true,
         );
     }
 
diff --git a/tests/lib/screenshot-testing/support/test-environment.js b/tests/lib/screenshot-testing/support/test-environment.js
index a812f5d83e..e94dd76dab 100644
--- a/tests/lib/screenshot-testing/support/test-environment.js
+++ b/tests/lib/screenshot-testing/support/test-environment.js
@@ -27,6 +27,7 @@ TestingEnvironment.prototype.reload = function () {
     this['testUseMockAuth'] = true;
     this['configOverride'] = {};
     this['optionsOverride'] = {};
+    this['environmentVariables'] = {};
 
     if (fs.exists(testingEnvironmentOverridePath)) {
         var data = JSON.parse(fs.read(testingEnvironmentOverridePath));
@@ -196,6 +197,10 @@ TestingEnvironment.prototype.setupFixture = function (fixtureClass, done) {
         args.push('--piwik-domain=' + options['piwik-domain']);
     }
 
+    if (options['enable-logging']) {
+        args.push('--enable-logging');
+    }
+
     var self = this;
     this.executeConsoleCommand('tests:setup-fixture', args, function (code) {
         self.reload();