Skip to content

Commit

Permalink
[BUGFIX] Run tests with configured error reporting in PHPunit
Browse files Browse the repository at this point in the history
SystemEnvironmentBuilder calls

 error_reporting(E_ALL & ~(E_STRICT | E_NOTICE | E_DEPRECATED));

which in turn is called on every test to setup the environment.
This basically enforces this exact error reporting and does not
allow to configure the error reporting in phpunit.xml AT ALL.

Until TYPO3 v9, this was covered in each BaseTestCase in
setUpBeforeClass() to re-add E_NOTICE to make all tests run
without notice.

The functionality in SystemEnvironmentBuilder was added
in TYPO3 v6.0 while refactoring TYPO3's Bootstrap to ensure
that no notices in ext_localconf.php or ext_tables.php
were breaking the output. However, nowadays, TYPO3's
error reporting is set up directly after LocalConfiguration.php
inclusion, making this logic obsolete.

The functionality is therefore moved to the Bootstrap at a later point,
which is called for Functional and Acceptance Tests.

Resolves: #90125
Releases: master
Change-Id: I8d6348ffbf6622c03abecedc1cb0ce286ba1044c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62926
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Benni Mack <benni@typo3.org>
  • Loading branch information
bmack committed Feb 14, 2020
1 parent da86e54 commit 9a3f936
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 52 deletions.
4 changes: 4 additions & 0 deletions typo3/sysext/core/Classes/Charset/CharsetConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,10 @@ protected function initCharset($charset)
} elseif ($detectedType === 'whitespaced') {
$regA = [];
preg_match('/[[:space:]]*0x([[:xdigit:]]*)[[:space:]]+0x([[:xdigit:]]*)[[:space:]]+/', $value, $regA);
if (empty($regA)) {
// No match => skip this item
continue;
}
$hexbyte = $regA[1];
$utf8 = 'U+' . $regA[2];
}
Expand Down
20 changes: 20 additions & 0 deletions typo3/sysext/core/Classes/Core/Bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ protected static function setDefaultTimezone()
*/
protected static function initializeErrorHandling()
{
static::initializeBasicErrorReporting();
$displayErrors = 0;
$exceptionHandlerClassName = null;
$productionExceptionHandlerClassName = $GLOBALS['TYPO3_CONF_VARS']['SYS']['productionExceptionHandler'];
Expand Down Expand Up @@ -416,6 +417,25 @@ protected static function initializeErrorHandling()
}
}

/**
* Initialize basic error reporting.
*
* There are a lot of extensions that have no strict / notice / deprecated free
* ext_localconf or ext_tables. Since the final error reporting must be set up
* after those extension files are read, a default configuration is needed to
* suppress error reporting meanwhile during further bootstrap.
*
* Please note: if you comment out this code, TYPO3 would never set any error reporting
* which would need to have TYPO3 Core and ALL used extensions to be notice free and deprecation
* free. However, commenting this out and running functional and acceptance tests shows what
* needs to be done to make TYPO3 Core mostly notice-free (unit tests do not execute this code here).
*/
protected static function initializeBasicErrorReporting(): void
{
// Core should be notice free at least until this point ...
error_reporting(E_ALL & ~(E_STRICT | E_NOTICE | E_DEPRECATED));
}

/**
* Initializes IO and stream wrapper related behavior.
*/
Expand Down
15 changes: 0 additions & 15 deletions typo3/sysext/core/Classes/Core/SystemEnvironmentBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public static function run(int $entryPointLevel = 0, int $requestType = self::RE

self::initializeGlobalVariables();
self::initializeGlobalTimeTrackingVariables();
self::initializeBasicErrorReporting();
self::initializeEnvironment($requestType, $scriptPath, $rootPath);
}

Expand Down Expand Up @@ -264,20 +263,6 @@ protected static function initializeEnvironment(int $requestType, string $script
);
}

/**
* Initialize basic error reporting.
*
* There are a lot of extensions that have no strict / notice / deprecated free
* ext_localconf or ext_tables. Since the final error reporting must be set up
* after those extension files are read, a default configuration is needed to
* suppress error reporting meanwhile during further bootstrap.
*/
protected static function initializeBasicErrorReporting()
{
// Core should be notice free at least until this point ...
error_reporting(E_ALL & ~(E_STRICT | E_NOTICE | E_DEPRECATED));
}

/**
* Determine if the operating system TYPO3 is running on is windows.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function fileDenyPatternMatchesPhpExtensionDataProvider()
{
$fileName = StringUtility::getUniqueId('filename');
$data = [];
$phpExtensions = \TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', 'php,php3,php4,php5,php6,phpsh,phtml,pht', true);
$phpExtensions = ['php', 'php3', 'php4', 'php5', 'php7', 'phpsh', 'phtml', 'pht'];
foreach ($phpExtensions as $extension) {
$data[] = [$fileName . '.' . $extension];
$data[] = [$fileName . '.' . $extension . '.txt'];
Expand Down Expand Up @@ -163,40 +163,4 @@ public function initializeGlobalTimeTrackingVariablesRoundsSimAccessTimeToSixtyS
$this->subject->_call('initializeGlobalTimeTrackingVariables');
self::assertEquals(0, $GLOBALS['SIM_ACCESS_TIME'] % 60);
}

/**
* @test
*/
public function initializeBasicErrorReportingExcludesStrict()
{
$backupReporting = error_reporting();
$this->subject->_call('initializeBasicErrorReporting');
$actualReporting = error_reporting();
error_reporting($backupReporting);
self::assertEquals(0, $actualReporting & E_STRICT);
}

/**
* @test
*/
public function initializeBasicErrorReportingExcludesNotice()
{
$backupReporting = error_reporting();
$this->subject->_call('initializeBasicErrorReporting');
$actualReporting = error_reporting();
error_reporting($backupReporting);
self::assertEquals(0, $actualReporting & E_NOTICE);
}

/**
* @test
*/
public function initializeBasicErrorReportingExcludesDeprecated()
{
$backupReporting = error_reporting();
$this->subject->_call('initializeBasicErrorReporting');
$actualReporting = error_reporting();
error_reporting($backupReporting);
self::assertEquals(0, $actualReporting & E_DEPRECATED);
}
}

0 comments on commit 9a3f936

Please sign in to comment.