From 0b26d450145353ba16bf6cec2af54bc067484d7c Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Wed, 25 Oct 2023 17:50:41 +0200 Subject: [PATCH 01/20] :white_check_mark: Prerequisites for boosting tests --- .../src/BaseTestCase/ItopTestCase.php | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index cd5bcb0a5..583ed8e65 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -26,6 +26,8 @@ abstract class ItopTestCase extends TestCase { const TEST_LOG_DIR = 'test'; + protected static $aBackupStaticProperties = []; + /** @noinspection UsingInclusionOnceReturnValueInspection avoid errors for approot includes */ protected function setUp(): void { @@ -254,6 +256,42 @@ abstract class ItopTestCase extends TestCase return $oProperty->getValue($oObject); } + /** + * Backup every static property of the class (even protected ones) + * @param string $sClass + * + * @return void + * + * @since 3.2.0 + */ + public static function BackupStaticProperties($sClass) + { + $class = new \ReflectionClass($sClass); + foreach ($class->getProperties() as $property) { + if (!$property->isStatic()) continue; + $property->setAccessible(true); + static::$aBackupStaticProperties[$sClass][$property->getName()] = $property->getValue(); + } + } + + /** + * Restore every static property of the class (even protected ones) + * @param string $sClass + * + * @return void + * + * @since 3.2.0 + */ + public static function RestoreStaticProperties($sClass) + { + $class = new \ReflectionClass($sClass); + foreach ($class->getProperties() as $property) { + if (!$property->isStatic()) continue; + $property->setAccessible(true); + $property->setValue(static::$aBackupStaticProperties[$sClass][$property->getName()]); + } + } + /** * @param string $sClass * @param string $sProperty @@ -300,4 +338,76 @@ abstract class ItopTestCase extends TestCase $oProperty->setValue($value); } + public static function RecurseRmdir($dir) + { + if (is_dir($dir)) { + $objects = scandir($dir); + foreach ($objects as $object) { + if ($object != "." && $object != "..") { + if (is_dir($dir.DIRECTORY_SEPARATOR.$object)) { + static::RecurseRmdir($dir.DIRECTORY_SEPARATOR.$object); + } else { + unlink($dir.DIRECTORY_SEPARATOR.$object); + } + } + } + rmdir($dir); + } + } + + public static function CreateTmpdir() { + $sTmpDir=tempnam(sys_get_temp_dir(),''); + if (file_exists($sTmpDir)) + { + unlink($sTmpDir); + } + mkdir($sTmpDir); + if (is_dir($sTmpDir)) + { + return $sTmpDir; + } + + return sys_get_temp_dir(); + } + + public function RecurseMkdir($sDir){ + if (strpos($sDir, DIRECTORY_SEPARATOR) === 0){ + $sPath = DIRECTORY_SEPARATOR; + } else { + $sPath = ""; + } + + foreach (explode(DIRECTORY_SEPARATOR, $sDir) as $sSubDir){ + if (($sSubDir === '..')) { + break; + } + + if (( trim($sSubDir) === '' ) || ( $sSubDir === '.' )) { + continue; + } + + $sPath .= $sSubDir . DIRECTORY_SEPARATOR; + if (!is_dir($sPath)) { + var_dump($sPath); + @mkdir($sPath); + } + } + + } + + public static function RecurseCopy($src,$dst) { + $dir = opendir($src); + @mkdir($dst); + while(false !== ( $file = readdir($dir)) ) { + if (( $file != '.' ) && ( $file != '..' )) { + if ( is_dir($src . '/' . $file) ) { + static::RecurseCopy($src . DIRECTORY_SEPARATOR . $file,$dst . DIRECTORY_SEPARATOR . $file); + } + else { + copy($src . DIRECTORY_SEPARATOR . $file,$dst . DIRECTORY_SEPARATOR . $file); + } + } + } + closedir($dir); + } } From 037dfe1df6b3af8a9df6b5e5926f38b638c51846 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Wed, 25 Oct 2023 17:51:12 +0200 Subject: [PATCH 02/20] :white_check_mark: Optimize tests execution time --- .../ApplicationExtensionTest.php | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/application/applicationextension/ApplicationExtensionTest.php b/tests/php-unit-tests/unitary-tests/application/applicationextension/ApplicationExtensionTest.php index 997a52258..e921b52f0 100644 --- a/tests/php-unit-tests/unitary-tests/application/applicationextension/ApplicationExtensionTest.php +++ b/tests/php-unit-tests/unitary-tests/application/applicationextension/ApplicationExtensionTest.php @@ -36,20 +36,18 @@ class ApplicationExtensionTest extends ItopCustomDatamodelTestCase * - Add the API to the provider * - Add a class extending / implementing the API in ./Delta/application-extension-usages-in-snippets.xml * - * @param string $sAPIFQCN - * @param string $sCallMethod - * * @return void - * @dataProvider ExtensionAPIRegisteredAndCalledProvider */ - public function testExtensionAPIRegisteredAndCalled(string $sAPIFQCN, string $sCallMethod) + public function testExtensionAPIRegisteredAndCalled() { - if ($sCallMethod === static::ENUM_API_CALL_METHOD_ENUMPLUGINS) { - $iExtendingClassesCount = count(MetaModel::EnumPlugins($sAPIFQCN)); - } else { - $iExtendingClassesCount = count(utils::GetClassesForInterface($sAPIFQCN, '', ['[\\\\/]lib[\\\\/]', '[\\\\/]node_modules[\\\\/]', '[\\\\/]test[\\\\/]', '[\\\\/]tests[\\\\/]'])); + foreach ($this->ExtensionAPIRegisteredAndCalledProvider() as list($sAPIFQCN, $sCallMethod)) { + if ($sCallMethod === static::ENUM_API_CALL_METHOD_ENUMPLUGINS) { + $iExtendingClassesCount = count(MetaModel::EnumPlugins($sAPIFQCN)); + } else { + $iExtendingClassesCount = count(utils::GetClassesForInterface($sAPIFQCN, '', ['[\\\\/]lib[\\\\/]', '[\\\\/]node_modules[\\\\/]', '[\\\\/]test[\\\\/]', '[\\\\/]tests[\\\\/]'])); + } + $this->assertGreaterThan(0, $iExtendingClassesCount, "Found no class extending the $sAPIFQCN API"); } - $this->assertGreaterThan(0, $iExtendingClassesCount, "Found no class extending the $sAPIFQCN API"); } public function ExtensionAPIRegisteredAndCalledProvider(): array From cf774cdb9023da8c43122a3cd8dfb95d196d1254 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Wed, 25 Oct 2023 22:18:05 +0200 Subject: [PATCH 03/20] :white_check_mark: Explain why process isolation is a must --- .../application/Session/SessionTest.php | 13 ++----------- .../unitary-tests/core/apcEmulationTest.php | 2 +- .../sources/application/status/StatusIncTest.php | 2 +- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/application/Session/SessionTest.php b/tests/php-unit-tests/unitary-tests/application/Session/SessionTest.php index 5bfd8467b..1321d77a8 100644 --- a/tests/php-unit-tests/unitary-tests/application/Session/SessionTest.php +++ b/tests/php-unit-tests/unitary-tests/application/Session/SessionTest.php @@ -6,7 +6,7 @@ use Combodo\iTop\Application\Helper\Session; use Combodo\iTop\Test\UnitTest\ItopTestCase; /** - * @runTestsInSeparateProcesses + * @runClassInSeparateProcess Required because PHPUnit outputs something earlier, thus causing the headers to be sent */ class SessionTest extends ItopTestCase { @@ -24,18 +24,9 @@ class SessionTest extends ItopTestCase /** * @covers \Combodo\iTop\Application\Helper\Session::Start - */ - public function testStart() - { - $this->assertNull(Session::$iSessionId); - Session::Start(); - $this->assertNotNull(Session::$iSessionId); - } - - /** * @covers \Combodo\iTop\Application\Helper\Session::WriteClose */ - public function testWriteClose() + public function testStartWriteClose() { $this->assertNull(Session::$iSessionId); Session::Start(); diff --git a/tests/php-unit-tests/unitary-tests/core/apcEmulationTest.php b/tests/php-unit-tests/unitary-tests/core/apcEmulationTest.php index 6dc3d95ee..3623fa6a4 100644 --- a/tests/php-unit-tests/unitary-tests/core/apcEmulationTest.php +++ b/tests/php-unit-tests/unitary-tests/core/apcEmulationTest.php @@ -32,7 +32,7 @@ define('UNIT_MAX_CACHE_FILES', 10); /** - * @runTestsInSeparateProcesses + * @runTestsInSeparateProcesses Required (at least) to mock the MetaModel and utils class */ class apcEmulationTest extends ItopTestCase { diff --git a/tests/php-unit-tests/unitary-tests/sources/application/status/StatusIncTest.php b/tests/php-unit-tests/unitary-tests/sources/application/status/StatusIncTest.php index 0ed673c3f..5a9c705ff 100644 --- a/tests/php-unit-tests/unitary-tests/sources/application/status/StatusIncTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/application/status/StatusIncTest.php @@ -66,7 +66,7 @@ class StatusIncTest extends ItopTestCase { } /** - * @runInSeparateProcess + * @runInSeparateProcess Required because Status constructor invokes MetaModel::Startup... which does nothing when already loaded */ public function testStatusStartupWrongDbPwd() { From 7fbc211c43c20a7668eca8eb8938a69039ccf368 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Wed, 25 Oct 2023 22:57:03 +0200 Subject: [PATCH 04/20] :white_check_mark: Optimize tests execution time (x50 / eval is way faster than exec) --- .../DictionariesConsistencyTest.php | 52 +++++++++++++++---- .../fr.dictionary.itop.core.OK.php | 3 +- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/tests/php-unit-tests/integration-tests/DictionariesConsistencyTest.php b/tests/php-unit-tests/integration-tests/DictionariesConsistencyTest.php index 1eaf4b6fd..c1bf7ba9e 100644 --- a/tests/php-unit-tests/integration-tests/DictionariesConsistencyTest.php +++ b/tests/php-unit-tests/integration-tests/DictionariesConsistencyTest.php @@ -18,6 +18,17 @@ namespace Combodo\iTop\Test\UnitTest\Integration; use Combodo\iTop\Test\UnitTest\ItopTestCase; + +/** + * Wrapper to load dictionnary files without altering the main dictionnary + * Eval will be called within the current namespace (this is done by adding a "namespace" statement) + */ +class Dict +{ + public static function Add($sLanguageCode, $sEnglishLanguageDesc, $sLocalizedLanguageDesc, $aEntries) + { + } +} /** * @group beforeSetup */ @@ -142,17 +153,40 @@ class DictionariesConsistencyTest extends ItopTestCase /** * @param string $sDictFile complete path for the file to check * @param bool $bIsSyntaxValid expected assert value - * - * @uses `php -l` - * @uses \assertEquals() */ private function CheckDictionarySyntax(string $sDictFile, $bIsSyntaxValid = true): void { - exec("php -l {$sDictFile}", $output, $return); - - $bDictFileSyntaxOk = ($return === 0); - - $sMessage = "File `{$sDictFile}` syntax didn't matched expectations\nparsing results=".var_export($output, true); - self::assertEquals($bIsSyntaxValid, $bDictFileSyntaxOk, $sMessage); + $sPHP = file_get_contents($sDictFile); + // Strip php tag to allow "eval" + $sPHP = substr(trim($sPHP), strlen(' No syntax error + if (!$bIsSyntaxValid) { + $this->fail("Failed to detect syntax error in dictionary `{$sDictFile}` (which is known as being INCORRECT)"); + } + } + catch (\Error $e) { + if ($bIsSyntaxValid) { + $iLine = $e->getLine() - $iLineShift; + $this->fail("Invalid dictionary: {$e->getMessage()} in {$sDictFile}:{$iLine}"); + } + } + catch (\Exception $e) { + if ($bIsSyntaxValid) { + $iLine = $e->getLine() - $iLineShift; + $sExceptionClass = get_class($e); + $this->fail("Exception thrown from dictionary: '$sExceptionClass: {$e->getMessage()}' in {$sDictFile}:{$iLine}"); + } + } + $this->assertTrue(true); } } diff --git a/tests/php-unit-tests/integration-tests/dictionaries-test/fr.dictionary.itop.core.OK.php b/tests/php-unit-tests/integration-tests/dictionaries-test/fr.dictionary.itop.core.OK.php index 3054d70fc..0658e7c86 100644 --- a/tests/php-unit-tests/integration-tests/dictionaries-test/fr.dictionary.itop.core.OK.php +++ b/tests/php-unit-tests/integration-tests/dictionaries-test/fr.dictionary.itop.core.OK.php @@ -1,10 +1,11 @@ + 'l\'échappement : bon exemple 1', + 'MyDictKey1' => 'l\'échappement : bon exemple 1'.ITOP_APPLICATION, 'MyDictKey2' => 'l\'échappement : bon exemple 2', 'MyDictKey3' => 'l\'échappement : bon exemple 3', )); From b5c46ccd4ab7cc72f105017cb9eee87b0d71025b Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Wed, 25 Oct 2023 22:59:03 +0200 Subject: [PATCH 05/20] :white_check_mark: Optimize tests execution time (x10 / no need for a systematic check of date formats, which was ok as a first approach) --- .../unitary-tests/core/ExpressionEvaluateTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/core/ExpressionEvaluateTest.php b/tests/php-unit-tests/unitary-tests/core/ExpressionEvaluateTest.php index d6f2826fd..fe4bca7ba 100644 --- a/tests/php-unit-tests/unitary-tests/core/ExpressionEvaluateTest.php +++ b/tests/php-unit-tests/unitary-tests/core/ExpressionEvaluateTest.php @@ -490,7 +490,7 @@ class ExpressionEvaluateTest extends ItopDataTestCase { $oExpression = new FunctionExpression('DATE_FORMAT', array(new ScalarExpression($sDate), new ScalarExpression("%$sFormat"))); $itopExpressionResult = $oExpression->Evaluate(array()); - static::assertSame($aMysqlDateFormatRsultsForAllFormats[$sFormat], $itopExpressionResult, "Format %$sFormat not matching MySQL for '$sDate'"); + $this->assertSame($aMysqlDateFormatRsultsForAllFormats[$sFormat], $itopExpressionResult, "Format %$sFormat not matching MySQL for '$sDate'"); } } } @@ -536,7 +536,7 @@ class ExpressionEvaluateTest extends ItopDataTestCase public function EveryTimeFormatOnDateRangeProvider() { return array( - '10 years, day by day' => array('2000-01-01', 'P1D', 365 * 10), + '10 years, each 17 days' => array('2000-01-01', 'P17D', 365 * 10 / 17), '1 day, hour by hour' => array('2000-01-01 00:01:02', 'PT1H', 24), '1 hour, minute by minute' => array('2000-01-01 00:01:02', 'PT1M', 60), '1 minute, second by second' => array('2000-01-01 00:01:02', 'PT1S', 60), From 7f245a15be703f4ace9803d4714e6298f6038a10 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Wed, 25 Oct 2023 23:01:05 +0200 Subject: [PATCH 06/20] :white_check_mark: Optimize tests execution time (suppress meaningless test and merge two test in one, while preserving test coverage) --- .../sources/application/status/StatusTest.php | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/sources/application/status/StatusTest.php b/tests/php-unit-tests/unitary-tests/sources/application/status/StatusTest.php index 03ef79468..e8a589a17 100644 --- a/tests/php-unit-tests/unitary-tests/sources/application/status/StatusTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/application/status/StatusTest.php @@ -18,14 +18,6 @@ class StatusTest extends ItopTestCase require_once APPROOT.'core/config.class.inc.php'; // for constants } - public function testStatusWrongUrl() { - $sPath = APPROOT.'/status_wrong.php'; - - exec("php $sPath", $aOutput, $iRet); - $this->assertNotEquals(0, $iRet, "Problem executing status page: $sPath, $iRet, aOutput:\n" . var_export($aOutput, true)); - - } - protected function GetPHPCommand() { $this->RequireOnceItopFile('application/utils.inc.php'); @@ -33,23 +25,14 @@ class StatusTest extends ItopTestCase return $oConfig->Get('php_path'); } - public function testStatusGood() { - $sPath = APPROOT.'/webservices/status.php'; - - $sPHP = $this->GetPHPCommand(); - exec("$sPHP $sPath", $aOutput, $iRet); - $this->assertEquals(0, $iRet, "Problem executing status page: $sPath, $iRet, aOutput:\n".var_export($aOutput, true)); - } - - /** - * - */ - public function testStatusGoodWithJson() + public function testStatusPageRepliesAsExpected() { $sPath = APPROOT.'/webservices/status.php'; $sPHP = $this->GetPHPCommand(); exec("$sPHP $sPath", $aOutput, $iRet); + $this->assertEquals(0, $iRet, "Problem executing status page: $sPath, $iRet, aOutput:\n".var_export($aOutput, true)); + $sAdditionalInfo = "aOutput:\n".var_export($aOutput, true).'.'; //Check response From 8893cdac1d633ec7d76a8a163f38542728448bb1 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Wed, 25 Oct 2023 23:08:26 +0200 Subject: [PATCH 07/20] :white_check_mark: Optimize tests execution time (no need for process isolation as long as we leave the premises clean) --- core/log.class.inc.php | 8 +++++--- .../core/Log/ExceptionLogTest.php | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/core/log.class.inc.php b/core/log.class.inc.php index 0a584dd3c..a0f5c7526 100644 --- a/core/log.class.inc.php +++ b/core/log.class.inc.php @@ -1,5 +1,5 @@ InvokeNonPublicStaticMethod('ExceptionLog', 'GetLastEventIssue', []); if (0 == $iExpectedDbWriteNumber) { - $this->assertNull($oExpectedLastEventIssue); + if (!is_null($oExpectedLastEventIssue)) { + $this->fail("Level '$sLevel': unexpected EventIssue"); + } } else { - $this->assertInstanceOf(\EventIssue::class, $oExpectedLastEventIssue); + $this->assertInstanceOf(\EventIssue::class, $oExpectedLastEventIssue, "Level '$sLevel': missing EventIssue"); $this->assertEquals($aExpectedFileContext, $oExpectedLastEventIssue->Get('data')); } } } + /** + * @return array[] + * aLevels: log levels to iterate on (AND name of the method that will be called on the underlying FileLog) + * aExceptions: For each log level => Exception to generate + * sChannel: Expected 2nd argument to the FileLog::{Level} + * aExpectedWriteNumber: For each log level => Number of times the method FileLog::{Level} will be called + * logLevelMin: Configuration / log_level_min + * iExpectedDbWriteNumber: For each log level => 1 if at least ONE EventIssue has been recorded into the DB + * logLevelMinWriteInDb: Configuration / log_level_min.write_in_db + */ public function logProvider() { return [ From 73bed04555fdcba8e1b1cd8c2d5966692ac42619 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 10:47:11 +0200 Subject: [PATCH 08/20] :white_check_mark: Twig tests not executed --- .../sources/application/TwigBase/Twig/TwigTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/php-unit-tests/unitary-tests/sources/application/TwigBase/Twig/TwigTest.php b/tests/php-unit-tests/unitary-tests/sources/application/TwigBase/Twig/TwigTest.php index 6cb27fbff..5d93259f6 100644 --- a/tests/php-unit-tests/unitary-tests/sources/application/TwigBase/Twig/TwigTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/application/TwigBase/Twig/TwigTest.php @@ -23,6 +23,7 @@ class TwigTest extends ItopDataTestCase */ public function testTemplate($sFileName, $sExpected) { + $this->expectNotToPerformAssertions(); $sId = 'TestTwig'; $oAppExtension = new AppExtension(); @@ -42,7 +43,7 @@ class TwigTest extends ItopDataTestCase } } - public static function testTemplateProvider() + public static function TemplateProvider() { $aReturn = array(); $aReturn['filter_system'] = [ From 7419749ba6b70ad3747ee42ac8271f6c3c03213b Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 20:51:28 +0200 Subject: [PATCH 09/20] :white_check_mark: Prerequisites for boosting tests --- tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index 583ed8e65..9158c9a61 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -370,7 +370,7 @@ abstract class ItopTestCase extends TestCase return sys_get_temp_dir(); } - public function RecurseMkdir($sDir){ + public static function RecurseMkdir($sDir){ if (strpos($sDir, DIRECTORY_SEPARATOR) === 0){ $sPath = DIRECTORY_SEPARATOR; } else { From 90006667fe8f9405e42d5130f3b58c90be4f8c49 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 20:58:26 +0200 Subject: [PATCH 10/20] :white_check_mark: Optimize tests execution time (copy fixture files only when necessary) --- .../application/ThemeHandlerTest.php | 150 ++++++++++-------- 1 file changed, 87 insertions(+), 63 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/application/ThemeHandlerTest.php b/tests/php-unit-tests/unitary-tests/application/ThemeHandlerTest.php index b8412201a..e889d61fa 100644 --- a/tests/php-unit-tests/unitary-tests/application/ThemeHandlerTest.php +++ b/tests/php-unit-tests/unitary-tests/application/ThemeHandlerTest.php @@ -12,46 +12,65 @@ use ThemeHandler; class ThemeHandlerTest extends ItopTestCase { const PATTERN = '|\\\/var[^"]+testimages|'; - + private $oCompileCSSServiceMock; private $sCompiledThemesDirAbsPath; private $sCssAbsPath; private $sDmCssAbsPath; private $sJsonThemeParamFile; - private $sTmpDir; - private $aDirsToCleanup= []; + static private $sTmpDir = null; + static private $aDirsToCleanup = []; + static private $sAbsoluteImagePath; + + + static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + + static::$sTmpDir = static::CreateTmpdir().'/'; + static::$aDirsToCleanup[] = static::$sTmpDir; + + static::$sAbsoluteImagePath = APPROOT.'tests/php-unit-tests/unitary-tests/application/theme-handler/copied/testimages/'; + static::RecurseMkdir(static::$sAbsoluteImagePath); + + // Required by testCompileThemesxxx - copy images in test dir + static::RecurseCopy(APPROOT.'tests/php-unit-tests/unitary-tests/application/theme-handler/expected/testimages/', static::$sAbsoluteImagePath); + + static::$aDirsToCleanup[] = dirname(static::$sAbsoluteImagePath); + } + + static function tearDownAfterClass(): void + { + foreach (static::$aDirsToCleanup as $sDir) + { + static::RecurseRmdir($sDir); + } + + parent::tearDownAfterClass(); + } + + protected static function InitCSSDirectory() + { + static::RecurseCopy(APPROOT."/tests/php-unit-tests/unitary-tests/application/theme-handler/expected/css", static::$sTmpDir."/branding/css"); + } public function setUp(): void { parent::setUp(); - $this->RequireOnceItopFile('application/themehandler.class.inc.php'); - $this->RequireOnceItopFile('setup/modelfactory.class.inc.php'); - $this->RequireOnceUnitTestFile('../setup/SubMFCompiler.php'); $this->oCompileCSSServiceMock = $this->createMock('CompileCSSService'); ThemeHandler::mockCompileCSSService($this->oCompileCSSServiceMock); - $this->sTmpDir = $this->CreateTmpdir().'/'; - $this->aDirsToCleanup[] = $this->sTmpDir; - - - $this->sCompiledThemesDirAbsPath = $this->sTmpDir."branding/themes/"; - $this->recurseMkdir($this->sCompiledThemesDirAbsPath."basque-red/"); + $this->sCompiledThemesDirAbsPath = static::$sTmpDir."branding/themes/"; + static::RecurseMkdir($this->sCompiledThemesDirAbsPath."basque-red/"); $this->sCssAbsPath = $this->sCompiledThemesDirAbsPath.'basque-red/main.css'; $this->sDmCssAbsPath = $this->sCompiledThemesDirAbsPath.'datamodel-compiled-scss-rules.scss'; $this->sJsonThemeParamFile = $this->sCompiledThemesDirAbsPath.'basque-red/theme-parameters.json'; - $this->RecurseCopy(APPROOT."/tests/php-unit-tests/unitary-tests/application/theme-handler/expected/css", $this->sTmpDir."/branding/css"); } public function tearDown(): void { parent::tearDown(); - - foreach ($this->aDirsToCleanup as $sDir) - { - echo $sDir; - $this->RecurseRmdir($sDir); - } } function KeepSignatureDiff($sSignature1, $sSignature2) : string { @@ -83,14 +102,14 @@ class ThemeHandlerTest extends ItopTestCase $aDiffOuput[$sKey] = $aCurrentDiffVal; } } else if ($oVal1 !== $aSignature2[$sKey]){ - $aDiffOuput[$sKey] = "expected:$oVal1 | actual:$aSignature2[$sKey]"; + $aDiffOuput[$sKey] = "expected:$oVal1 | actual:$aSignature2[$sKey]"; } } return json_encode($aDiffOuput, true); } - function recurseMkdir($dir) + public static function RecurseMkdir($dir) { if (is_dir($dir)) { @@ -98,7 +117,7 @@ class ThemeHandlerTest extends ItopTestCase } $sParentDir = dirname($dir); - if (!$this->recurseMkdir($sParentDir)) + if (!static::RecurseMkdir($sParentDir)) { return false; } @@ -141,6 +160,8 @@ JSON; */ public function testCompileThemeWithoutCssFile_FocusOnParamAttribute($readFromParamAttributeFromJson=false) { + static::InitCSSDirectory(); + $sExpectJsonFilePath = APPROOT.'tests/php-unit-tests/unitary-tests/application/theme-handler/expected/themes/basque-red/theme-parameters.json'; $sExpectedThemeParamJson = file_get_contents($sExpectJsonFilePath); $aThemeParameters = json_decode($sExpectedThemeParamJson, true); @@ -160,11 +181,11 @@ JSON; if($readFromParamAttributeFromJson) { copy($sExpectJsonFilePath, $this->sJsonThemeParamFile); - $this->assertTrue(ThemeHandler::CompileTheme('basque-red', true, "COMPILATIONTIMESTAMP", null, [$this->sTmpDir.'/branding/themes/'], $this->sTmpDir)); + $this->assertTrue(ThemeHandler::CompileTheme('basque-red', true, "COMPILATIONTIMESTAMP", null, [static::$sTmpDir.'/branding/themes/'], static::$sTmpDir)); } else { - $this->assertTrue(ThemeHandler::CompileTheme('basque-red', true, "COMPILATIONTIMESTAMP", $aThemeParameters, [$this->sTmpDir.'/branding/themes/'], $this->sTmpDir)); + $this->assertTrue(ThemeHandler::CompileTheme('basque-red', true, "COMPILATIONTIMESTAMP", $aThemeParameters, [static::$sTmpDir.'/branding/themes/'], static::$sTmpDir)); } $this->assertTrue(is_file($this->sCssAbsPath)); $this->assertEquals($sExpectedThemeParamJson, file_get_contents($this->sJsonThemeParamFile)); @@ -189,14 +210,14 @@ JSON; */ public function testCompileThemesEmptyArray($ThemeParametersJson, $CompileCount=0) { - $sCssPath = $this->sTmpDir . '/branding/themes/basque-red/main.css'; + $sCssPath = static::$sTmpDir . '/branding/themes/basque-red/main.css'; copy(APPROOT . 'tests/php-unit-tests/unitary-tests/application/theme-handler/expected/themes/basque-red/main.css', $sCssPath); $this->oCompileCSSServiceMock->expects($this->exactly($CompileCount)) ->method("CompileCSSFromSASS") ->willReturn("====CSSCOMPILEDCONTENT===="); - $this->assertEquals($CompileCount!=0,ThemeHandler::CompileTheme('basque-red', true, "COMPILATIONTIMESTAMP", json_decode($ThemeParametersJson, true), [$this->sTmpDir.'/branding/themes/'], $this->sTmpDir)); + $this->assertEquals($CompileCount!=0,ThemeHandler::CompileTheme('basque-red', true, "COMPILATIONTIMESTAMP", json_decode($ThemeParametersJson, true), [static::$sTmpDir.'/branding/themes/'], static::$sTmpDir)); } public function CompileThemesProviderEmptyArray() @@ -213,6 +234,14 @@ JSON; /** * @return array + * mixed $ThemeParametersJson + * int $iCompileCSSFromSASSCount + * boolean $bMissingFile + * boolean $bFilesTouchedRecently + * boolean $bFileMd5sumModified + * null $sFileToTest + * null $sExpectedMainCssPath + * bool $bSetup */ public function CompileThemesProvider() { @@ -260,26 +289,24 @@ JSON; */ public function testCompileThemes($ThemeParametersJson, $iCompileCSSFromSASSCount, $bMissingFile=false, $bFilesTouchedRecently=false, $bFileMd5sumModified=false, $sFileToTest=null, $sExpectedMainCssPath=null, $bSetup=true) { + static::InitCSSDirectory(); + $sAfterReplacementCssVariableMd5sum=''; - if (is_file($this->sTmpDir.'/'.$sFileToTest)) + if (is_file(static::$sTmpDir.'/'.$sFileToTest)) { - $sFileToTest = $this->sTmpDir.'/'.$sFileToTest; + $sFileToTest = static::$sTmpDir.'/'.$sFileToTest; } else { $sFileToTest = APPROOT.'/'.$sFileToTest; } - //copy images in test dir - $sAbsoluteImagePath = APPROOT.'tests/php-unit-tests/unitary-tests/application/theme-handler/copied/testimages/'; - $this->recurseMkdir($sAbsoluteImagePath); - $this->aDirsToCleanup[] = dirname($sAbsoluteImagePath); - $this->RecurseCopy(APPROOT.'tests/php-unit-tests/unitary-tests/application/theme-handler/expected/testimages/', $sAbsoluteImagePath); + // Backup the file to test + copy($sFileToTest, static::$sTmpDir.'/file-to-test-backup'); //change approot-relative in css-variable to use absolute path - $sCssVarPath = $this->sTmpDir."/branding/css/DO_NOT_CHANGE.css-variables.scss"; + $sCssVarPath = static::$sTmpDir."/branding/css/DO_NOT_CHANGE.css-variables.scss"; $sBeforeReplacementCssVariableMd5sum = md5_file($sCssVarPath); - echo 'BEFORE :'.$sBeforeReplacementCssVariableMd5sum.' '.$sCssVarPath.' '; $sCssVariableContent = file_get_contents($sCssVarPath); - $sLine = '$approot-relative: "'.$sAbsoluteImagePath.'" !default;'; + $sLine = '$approot-relative: "'.static::$sAbsoluteImagePath.'" !default;'; $sCssVariableContent = preg_replace("/\\\$approot-relative: \"(.*)\"/", $sLine, $sCssVariableContent); file_put_contents($sCssVarPath, $sCssVariableContent); if ($bMissingFile) @@ -296,30 +323,23 @@ JSON; //change cssvar md5sum + image absolute paths $sMainCssContent = file_get_contents(APPROOT."tests/php-unit-tests/unitary-tests/application/theme-handler/expected/themes/basque-red/main_testcompilethemes.css"); $sMainCssContent = preg_replace('/MD5SUM/', $sAfterReplacementCssVariableMd5sum, $sMainCssContent); - $sReplacement = rtrim($sAbsoluteImagePath, '/'); + $sReplacement = rtrim(static::$sAbsoluteImagePath, '/'); $sReplacement=preg_replace('|\/|', '\/', $sReplacement); $sMainCssContent = preg_replace(static::PATTERN, $sReplacement, $sMainCssContent); - $cssPath = $this->sTmpDir . '/branding/themes/basque-red/main.css'; - echo 'PUT md5sum: '.$sAfterReplacementCssVariableMd5sum.' in '.$cssPath.' '; + $cssPath = static::$sTmpDir . '/branding/themes/basque-red/main.css'; file_put_contents($cssPath, $sMainCssContent); //should be after main.css modification to make sure precompilation check will be performed if ($bFilesTouchedRecently) { - sleep(1); - touch($sFileToTest); + touch($sFileToTest, time() + 2, time() + 2); } //same: it should be after main.css modification if ($bFileMd5sumModified) { - $sMd5sum = md5_file($sFileToTest); - echo ' BEFORE touch: ' . $sMd5sum .' ' . $sFileToTest; - sleep(1); file_put_contents($sFileToTest, "###\n".file_get_contents($sFileToTest)); - - $sMd5sum = md5_file($sFileToTest); - echo ' AFTER touch: ' . $sMd5sum .' ' . $sFileToTest; + touch($sFileToTest, time() + 2, time() + 2); } if (is_file($sCssVarPath)) @@ -332,7 +352,7 @@ JSON; ->willReturn("====CSSCOMPILEDCONTENT===="); $aThemeParameters = json_decode($ThemeParametersJson, true); - $this->assertEquals($iCompileCSSFromSASSCount!=0, ThemeHandler::CompileTheme('basque-red', $bSetup, "COMPILATIONTIMESTAMP", $aThemeParameters, [$this->sTmpDir.'/branding/themes/'], $this->sTmpDir)); + $this->assertEquals($iCompileCSSFromSASSCount!=0, ThemeHandler::CompileTheme('basque-red', $bSetup, "COMPILATIONTIMESTAMP", $aThemeParameters, [static::$sTmpDir.'/branding/themes/'], static::$sTmpDir)); if ($iCompileCSSFromSASSCount==1) { @@ -347,9 +367,11 @@ JSON; $aReplacements = [$sReplacement, $sAfterReplacementCssVariableMd5sum]; $aReplacements[] = md5(json_encode($aThemeParameters['variables'])); $aReplacements[] = $sAfterReplacementCssVariableMd5sum; - var_dump($aReplacements); $this->DoInnerJsonValidation($sExpectedMainCssFile, $cssPath, $aPatterns, $aReplacements); } + + // Restore the file to test (possible improvement: do that in tearDown) + copy(static::$sTmpDir.'/file-to-test-backup', $sFileToTest); } public function DoInnerJsonValidation($sExpectedCssFile, $sActualCssFile, $aPatterns, $aReplacements) @@ -396,8 +418,8 @@ JSON; '\'abc/\'+ $approot-relative + "css/ui-lightness/images/toutou.png?v=" + $version', "\$approot-relative + \"css/ui-lightness/images/toto.png?v=\" + \$version", '$approot-relative + \'css/ui-lightness/images/titi.gif?v=\' + $version1', - '"data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7?v=" + $version', - '$approot-relative + \'node_modules/raleway-webfont/fonts/Raleway-Thin.jpeg\'', + '"data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7?v=" + $version', + '$approot-relative + \'node_modules/raleway-webfont/fonts/Raleway-Thin.jpeg\'', ]; $aIncludedUrls['aCompleteUrls']; @@ -432,13 +454,13 @@ SCSS; $aExpectedFoundVariables = [ 'gabu' => 'zomeu', 'toto' => 'titi', - 'approot-relative' => '../../../../../', - 'approot-relative2' => '../../', - 'gray-base' => '#000', - 'a' => 'b', - 'content-color' => '#eeeeee', - 'default-font-family' => 'Trebuchet MS,Tahoma,Verdana,Arial,sans-serif', - 'icons-filter' => 'hue-rotate(0deg)', + 'approot-relative' => '../../../../../', + 'approot-relative2' => '../../', + 'gray-base' => '#000', + 'a' => 'b', + 'content-color' => '#eeeeee', + 'default-font-family' => 'Trebuchet MS,Tahoma,Verdana,Arial,sans-serif', + 'icons-filter' => 'hue-rotate(0deg)', 'toto' => 'titi', ]; $this->assertEquals($aExpectedFoundVariables, $aFoundVariables); @@ -460,10 +482,10 @@ $icons-filter: hue-rotate(0deg) !default; $toto : titi; SCSS; - file_put_contents($this->sTmpDir . DIRECTORY_SEPARATOR . 'css-variable.scss', $sContent); + file_put_contents(static::$sTmpDir . DIRECTORY_SEPARATOR . 'css-variable.scss', $sContent); $aVariables = ThemeHandler::GetVariablesFromFile( [ 'css-variable.scss' ], - [ $this->sTmpDir ] + [ static::$sTmpDir ] ); $aExpectedVariables = [ @@ -512,8 +534,10 @@ SCSS; public function testGetIncludedImages() { - $aStylesheetFile=glob($this->sTmpDir."/branding/css/*.scss"); - $aStylesheetFile[]=$this->sTmpDir."/branding/css/ui-lightness/DO_NOT_CHANGE.jqueryui.scss"; + static::InitCSSDirectory(); + + $aStylesheetFile=glob(static::$sTmpDir."/branding/css/*.scss"); + $aStylesheetFile[]=static::$sTmpDir."/branding/css/ui-lightness/DO_NOT_CHANGE.jqueryui.scss"; $expectJsonFilePath = APPROOT.'tests/php-unit-tests/unitary-tests/application/theme-handler/expected/themes/basque-red/theme-parameters.json'; $expectedThemeParamJson = file_get_contents($expectJsonFilePath); $aThemeParametersVariables = json_decode($expectedThemeParamJson, true); @@ -538,7 +562,7 @@ SCSS; * @throws \Exception */ public function testFindStylesheetFile(string $sFileToFind, array $aAllImports){ - $sImportsPath = $this->sTmpDir.'branding/'; + $sImportsPath = static::$sTmpDir.'branding/'; // Windows compat O:) $sFileToFind = $this->UpdateDirSep($sFileToFind); From d6415042ae4ea930b39e8fb3b76d77bc20ad2822 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 21:10:07 +0200 Subject: [PATCH 11/20] :white_check_mark: Optimize tests execution time (no need for process isolation as long as we leave the premises clean) --- addons/userrights/userrightsprofile.class.inc.php | 12 ++++++------ .../unitary-tests/core/UserRightsTest.php | 4 +--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/addons/userrights/userrightsprofile.class.inc.php b/addons/userrights/userrightsprofile.class.inc.php index f91149843..c41aa5083 100644 --- a/addons/userrights/userrightsprofile.class.inc.php +++ b/addons/userrights/userrightsprofile.class.inc.php @@ -490,6 +490,7 @@ class UserRightsProfile extends UserRightsAddOnAPI } protected $m_aUserOrgs = array(); // userid -> array of orgid + protected $m_aAdministrators = null; // [user id] // Built on demand, could be optimized if necessary (doing a query for each attribute that needs to be read) protected $m_aObjectActionGrants = array(); @@ -546,6 +547,7 @@ class UserRightsProfile extends UserRightsAddOnAPI // Cache $this->m_aObjectActionGrants = array(); + $this->m_aAdministrators = null; } public function LoadCache() @@ -688,12 +690,10 @@ class UserRightsProfile extends UserRightsAddOnAPI */ private function GetAdministrators() { - static $aAdministrators = null; - - if ($aAdministrators === null) + if ($this->m_aAdministrators === null) { // Find all administrators - $aAdministrators = array(); + $this->m_aAdministrators = array(); $oAdministratorsFilter = new DBObjectSearch('User'); $oLnkFilter = new DBObjectSearch('URP_UserProfile'); $oExpression = new FieldExpression('profileid', 'URP_UserProfile'); @@ -706,10 +706,10 @@ class UserRightsProfile extends UserRightsAddOnAPI $oSet->OptimizeColumnLoad(array('User' => array('login'))); while($oUser = $oSet->Fetch()) { - $aAdministrators[] = $oUser->GetKey(); + $this->m_aAdministrators[] = $oUser->GetKey(); } } - return $aAdministrators; + return $this->m_aAdministrators; } /** diff --git a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php index b9ee0dcb5..af6eb3e74 100644 --- a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php +++ b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php @@ -50,7 +50,6 @@ class UserRightsTest extends ItopDataTestCase try { utils::GetConfig()->SetModuleSetting('authent-local', 'password_validation.pattern', ''); - self::CreateUser('admin', 1); } catch (CoreCannotSaveObjectException $e) { } @@ -487,8 +486,7 @@ class UserRightsTest extends ItopDataTestCase ]; } /** - * @runInSeparateProcess - *@dataProvider NonAdminCannotListAdminProfilesProvider + * @dataProvider NonAdminCannotListAdminProfilesProvider */ public function testNonAdminCannotListAdminProfiles($bHideAdministrators, $iExpectedCount) { From 29e9a06dc1eaaf693165854623b29ab03a5985fc Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 21:10:47 +0200 Subject: [PATCH 12/20] :white_check_mark: Optimize tests execution time (no need for process isolation as long as we leave the premises clean) --- application/utils.inc.php | 7 +++++-- .../php-unit-tests/unitary-tests/application/utilsTest.php | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/application/utils.inc.php b/application/utils.inc.php index dbb595d72..6086f3874 100644 --- a/application/utils.inc.php +++ b/application/utils.inc.php @@ -155,6 +155,8 @@ class utils private static $iNextId = 0; + private static $m_sAppRootUrl = null; + protected static function LoadParamFile($sParamFile) { if (!file_exists($sParamFile)) { @@ -992,7 +994,7 @@ class utils */ public static function GetAbsoluteUrlAppRoot($bForceTrustProxy = false) { - static $sUrl = null; + $sUrl = static::$m_sAppRootUrl; if ($sUrl === null || $bForceTrustProxy) { $sUrl = self::GetConfig()->Get('app_root_url'); @@ -1013,8 +1015,9 @@ class utils } $sUrl = str_replace(SERVER_NAME_PLACEHOLDER, $sServerName, $sUrl); } + static::$m_sAppRootUrl = $sUrl; } - return $sUrl; + return static::$m_sAppRootUrl; } /** diff --git a/tests/php-unit-tests/unitary-tests/application/utilsTest.php b/tests/php-unit-tests/unitary-tests/application/utilsTest.php index 116f621c1..790991ab9 100644 --- a/tests/php-unit-tests/unitary-tests/application/utilsTest.php +++ b/tests/php-unit-tests/unitary-tests/application/utilsTest.php @@ -25,7 +25,6 @@ use Combodo\iTop\Test\UnitTest\ItopTestCase; use utils; /** - * @runClassInSeparateProcess * @covers utils */ class utilsTest extends ItopTestCase @@ -245,7 +244,6 @@ class utilsTest extends ItopTestCase } /** - * @runInSeparateProcess * @dataProvider GetAbsoluteUrlAppRootPersistency */ public function testGetAbsoluteUrlAppRootPersistency($bBehindReverseProxy,$bForceTrustProxy1 ,$sExpectedAppRootUrl1,$bForceTrustProxy2 , $sExpectedAppRootUrl2,$bForceTrustProxy3 , $sExpectedAppRootUrl3) @@ -274,6 +272,9 @@ class utilsTest extends ItopTestCase $this->assertEquals($sExpectedAppRootUrl2, utils::GetAbsoluteUrlAppRoot($bForceTrustProxy2)); $this->assertEquals($sExpectedAppRootUrl3, utils::GetAbsoluteUrlAppRoot($bForceTrustProxy3)); + + // Leave the place clean + static::SetNonPublicStaticProperty('utils', 'm_sAppRootUrl', null); } From c0931af91af38414797c6a53406b8ef123ad1489 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 21:15:57 +0200 Subject: [PATCH 13/20] :white_check_mark: Optimize tests execution time (no need for process isolation as long as we leave the premises clean, set file modification date instead of waiting for 1 second) --- .../unitary-tests/setup/MFCompilerTest.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/setup/MFCompilerTest.php b/tests/php-unit-tests/unitary-tests/setup/MFCompilerTest.php index e3e0ca40f..7e8308c83 100644 --- a/tests/php-unit-tests/unitary-tests/setup/MFCompilerTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/MFCompilerTest.php @@ -9,7 +9,6 @@ use SubMFCompiler; use utils; /** - * @runClassInSeparateProcess * @covers \MFCompiler::UseLatestPrecompiledFile */ class MFCompilerTest extends ItopTestCase { @@ -57,23 +56,25 @@ class MFCompilerTest extends ItopTestCase { self::$aFoldersToCleanup = [ $sTempTargetDir, $sExtensionTargetDir, $sDatamodel2xTargetDir ]; + // Sometime in the past + $iTimeStart = time() - 100; + self::$aRessources['sPostCompilation1'] = tempnam($sTempTargetDir, $sPrefix); - sleep(1); + touch(self::$aRessources['sPostCompilation1'], $iTimeStart+=2); //datamodel XML file in extension folder self::$aRessources['sPrecompiledInExtensionFile1'] = tempnam($sExtensionTargetDir, $sPrefix); + touch(self::$aRessources['sPrecompiledInExtensionFile1'], $iTimeStart+=2); self::$aRessources['sPrecompiledInExtensionFileUri1'] = "UseLatestPrecompiledFileProvider" . DIRECTORY_SEPARATOR . basename(self::$aRessources['sPrecompiledInExtensionFile1']); //datamodel XML file in source dir /datamodels/2.x folder self::$aRessources['sPrecompiledInDataModelXXFile1'] = tempnam($sDatamodel2xTargetDir, $sPrefix); + touch(self::$aRessources['sPrecompiledInDataModelXXFile1'], $iTimeStart+=2); self::$aRessources['sPrecompiledInDataModelXXFileUri1'] = "UseLatestPrecompiledFileProvider" . DIRECTORY_SEPARATOR . basename(self::$aRessources['sPrecompiledInDataModelXXFile1']); - sleep(1); - - //generate ressources from a previous setup: called postcompiled self::$aRessources['sPostCompilation2'] = tempnam($sTempTargetDir, $sPrefix); - sleep(1); + touch(self::$aRessources['sPostCompilation2'], $iTimeStart+=2); //simulate copy of /data/models.2.x or extensions ressources during setup in a temp directory self::$aRessources['sCopiedExtensionFile1'] = $sTempTargetDir . DIRECTORY_SEPARATOR . basename(self::$aRessources['sPrecompiledInExtensionFile1']); @@ -118,9 +119,8 @@ class MFCompilerTest extends ItopTestCase { * @param ?string $sExpectedReturn */ public function testUseLatestPrecompiledFile(string $sTempTargetDir, string $sPrecompiledFileUri, string $sPostCompilationLatestPrecompiledFile, string $sThemeDir, ?string $sExpectedReturn, bool $bDisableThemePrecompilationViaConf = false){ - if ($bDisableThemePrecompilationViaConf){ - utils::GetConfig()->Set('theme.enable_precompilation', false); - } + // Enable or disable precompilation depending on the test case + utils::GetConfig()->Set('theme.enable_precompilation', !$bDisableThemePrecompilationViaConf); $sRes = $this->oMFCompiler->UseLatestPrecompiledFile($sTempTargetDir, $sPrecompiledFileUri, $sPostCompilationLatestPrecompiledFile, $sThemeDir); $this->assertEquals($sExpectedReturn, $sRes); } From 1a9049d27751bb5adcf86f1bad62a554e5cbc8c9 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 21:16:24 +0200 Subject: [PATCH 14/20] :white_check_mark: Optimize tests execution time (no need for process isolation as long as we leave the premises clean) --- tests/php-unit-tests/unitary-tests/webservices/RestTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/webservices/RestTest.php b/tests/php-unit-tests/unitary-tests/webservices/RestTest.php index a7871888b..ebce3f472 100644 --- a/tests/php-unit-tests/unitary-tests/webservices/RestTest.php +++ b/tests/php-unit-tests/unitary-tests/webservices/RestTest.php @@ -12,8 +12,6 @@ use utils; * @group itopRequestMgmt * @group restApi * @group defaultProfiles - * - * @runClassInSeparateProcess */ class RestTest extends ItopDataTestCase { From 442721bcb510fac4e436f1c444e7e0d7b6223e34 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 21:22:54 +0200 Subject: [PATCH 15/20] :white_check_mark: Optimize tests execution time (no need for process isolation as long as we leave the premises clean) --- .../unitary-tests/webservices/CliResetSessionTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/webservices/CliResetSessionTest.php b/tests/php-unit-tests/unitary-tests/webservices/CliResetSessionTest.php index 985e70577..f7d06ae45 100644 --- a/tests/php-unit-tests/unitary-tests/webservices/CliResetSessionTest.php +++ b/tests/php-unit-tests/unitary-tests/webservices/CliResetSessionTest.php @@ -8,9 +8,6 @@ use Exception; use MetaModel; -/** - * @runTestsInSeparateProcesses - */ class CliResetSessionTest extends ItopDataTestCase { const USE_TRANSACTION = false; From 798cd10d6b7cf75e912c12225ddb3b6fc7c3d5ed Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 21:23:47 +0200 Subject: [PATCH 16/20] :white_check_mark: Optimize tests execution time (no need for process isolation as long as we leave the premises clean) --- .../unitary-tests/core/dictApcuTest.php | 56 +++++++++++++------ .../unitary-tests/core/dictTest.php | 15 +++-- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/core/dictApcuTest.php b/tests/php-unit-tests/unitary-tests/core/dictApcuTest.php index d8f790c88..081152542 100644 --- a/tests/php-unit-tests/unitary-tests/core/dictApcuTest.php +++ b/tests/php-unit-tests/unitary-tests/core/dictApcuTest.php @@ -30,9 +30,6 @@ use Combodo\iTop\Test\UnitTest\ItopTestCase; use Dict; -/** - * @runTestsInSeparateProcesses - */ class dictApcuTest extends ItopTestCase { private $sEnvName; @@ -45,9 +42,16 @@ class dictApcuTest extends ItopTestCase $this->RequireOnceItopFile('core'.DIRECTORY_SEPARATOR.'apc-service.class.inc.php'); - $this->sEnvName = time(); + // This id will be used as path to the dictionary files + // It must be unique enough for the magic of Dict to operate (due to the use of require_once to load dictionaries) + $this->sEnvName = uniqid(); $_SESSION['itop_env'] = $this->sEnvName; + // Preserve the dictionary for test that will be executed later on + static::BackupStaticProperties('Dict'); + + // Reset and prepare the dictionary + static::SetNonPublicStaticProperty('Dict', 'm_aData', []); $this->oApcService = $this->createMock(\ApcService::class); Dict::SetApcService($this->oApcService); Dict::EnableCache('toto'); @@ -57,7 +61,8 @@ class dictApcuTest extends ItopTestCase $this->InitDictionnaries(); } - private function InitDictionnaries(){ + private function InitDictionnaries() + { clearstatcache(); $this->sDictionaryFolder = APPROOT."env-$this->sEnvName" . DIRECTORY_SEPARATOR . "dictionaries"; @mkdir($this->sDictionaryFolder, 0777, true); @@ -82,7 +87,8 @@ STR; clearstatcache(); } - private function InitDictionnary($sDictionaryFolder, $sLanguageCode, $sLanguageCodeInFilename, $sLabels){ + private function InitDictionnary($sDictionaryFolder, $sLanguageCode, $sLanguageCodeInFilename, $sLabels) + { $sContent = <<sEnvName".DIRECTORY_SEPARATOR."dictionaries"); rmdir(APPROOT."env-$this->sEnvName"); + static::RestoreStaticProperties('Dict'); + parent::tearDown(); } - public function InitLangIfNeeded_NoApcProvider(){ + public function InitLangIfNeeded_NoApcProvider() + { return [ 'apcu mocked' => [ 'bApcuMocked' => true ], 'integration test - apcu service like in production - install php-apcu before' => [ 'bApcuMocked' => false ], @@ -127,7 +137,8 @@ PHP; /** * @dataProvider InitLangIfNeeded_NoApcProvider */ - public function testInitLangIfNeeded_NoApc($bApcuMocked){ + public function testInitLangIfNeeded_NoApc($bApcuMocked) + { if ($bApcuMocked) { $this->oApcService->expects($this->any()) ->method('function_exists') @@ -163,7 +174,8 @@ PHP; $this->assertEquals('not_defined_label', Dict::S('not_defined_label')); } - public function testInitLangIfNeeded_Apc_LanguageMismatchDictionnary(){ + public function testInitLangIfNeeded_Apc_LanguageMismatchDictionnary() + { //language mismatch!! $sLabels = << 'de1', @@ -185,7 +197,8 @@ STR; $this->assertEquals('label1', Dict::S('label1')); } - public function testInitLangIfNeeded_Apc_BrokenUserDictionnary(){ + public function testInitLangIfNeeded_Apc_BrokenUserDictionnary() + { $this->InitBrokenDictionnary($this->sDictionaryFolder, 'DE DE', 'de-de'); $this->oApcService->expects($this->any()) @@ -205,7 +218,8 @@ STR; $this->assertEquals('ru1', Dict::S('label1')); } - public function testInitLangIfNeeded_Apc_BrokenDictionnary_UserAndDefault(){ + public function testInitLangIfNeeded_Apc_BrokenDictionnary_UserAndDefault() + { $this->InitBrokenDictionnary($this->sDictionaryFolder, 'DE DE', 'de-de'); $this->InitBrokenDictionnary($this->sDictionaryFolder, 'RU RU', 'ru-ru'); @@ -224,7 +238,8 @@ STR; $this->assertEquals('en1', Dict::S('label1')); } - public function testInitLangIfNeeded_Apc_BrokenDictionnary_ALL(){ + public function testInitLangIfNeeded_Apc_BrokenDictionnary_ALL() + { $this->InitBrokenDictionnary($this->sDictionaryFolder, 'DE DE', 'de-de'); $this->InitBrokenDictionnary($this->sDictionaryFolder, 'RU RU', 'ru-ru'); $this->InitBrokenDictionnary($this->sDictionaryFolder, 'EN US', 'en-us'); @@ -244,7 +259,8 @@ STR; $this->assertEquals('label1', Dict::S('label1')); } - public function testInitLangIfNeeded_ApcFromCache_PropertyInUserDictionnary(){ + public function testInitLangIfNeeded_ApcFromCache_PropertyInUserDictionnary() + { $this->oApcService->expects($this->any()) ->method('function_exists') ->willReturn(true); @@ -262,7 +278,8 @@ STR; $this->assertEquals('fr1', Dict::S('label1')); } - public function testInitLangIfNeeded_ApcStore_PropertyInUserDictionnary(){ + public function testInitLangIfNeeded_ApcStore_PropertyInUserDictionnary() + { $this->oApcService->expects($this->any()) ->method('function_exists') ->willReturn(true); @@ -341,7 +358,8 @@ STR; $this->assertEquals('en2', Dict::S('label2')); } - public function testInitLangIfNeeded_Apc_PropertyInDictDefaultLanguageDictionnary(){ + public function testInitLangIfNeeded_Apc_PropertyInDictDefaultLanguageDictionnary() + { $this->oApcService->expects($this->any()) ->method('function_exists') ->willReturn(true); @@ -362,7 +380,8 @@ STR; $this->assertEquals('en3', Dict::S('label3')); } - public function testInitLangIfNeeded_ApcCorrupted_PropertyInDictDefaultLanguageDictionnary(){ + public function testInitLangIfNeeded_ApcCorrupted_PropertyInDictDefaultLanguageDictionnary() + { $this->oApcService->expects($this->any()) ->method('function_exists') ->willReturn(true); @@ -380,7 +399,8 @@ STR; $this->assertEquals('label3', Dict::S('label3')); } - public function testInitLangIfNeeded_Apc_PropertyNotFound(){ + public function testInitLangIfNeeded_Apc_PropertyNotFound() + { $this->oApcService->expects($this->any()) ->method('function_exists') ->willReturn(true); diff --git a/tests/php-unit-tests/unitary-tests/core/dictTest.php b/tests/php-unit-tests/unitary-tests/core/dictTest.php index b7cffc17a..528ca5c34 100644 --- a/tests/php-unit-tests/unitary-tests/core/dictTest.php +++ b/tests/php-unit-tests/unitary-tests/core/dictTest.php @@ -30,9 +30,6 @@ use Combodo\iTop\Test\UnitTest\ItopTestCase; use Dict; use Exception; -/** - * @runClassInSeparateProcess - */ class dictTest extends ItopTestCase { private $sEnvName; @@ -42,7 +39,9 @@ class dictTest extends ItopTestCase $this->RequireOnceItopFile('core'.DIRECTORY_SEPARATOR.'apc-service.class.inc.php'); - $this->sEnvName = time(); + // This id will be used as path to the dictionary files + // It must be unique enough for the magic of Dict to operate (due to the use of require_once to load dictionaries) + $this->sEnvName = uniqid(); $sDictionaryFolder = APPROOT."env-$this->sEnvName".DIRECTORY_SEPARATOR."dictionaries"; @mkdir($sDictionaryFolder, 0777, true); @@ -68,6 +67,12 @@ PHP; file_put_contents($sDictionaryFolder.DIRECTORY_SEPARATOR."en-en.dict.php", $sContent); $_SESSION['itop_env'] = $this->sEnvName; + + // Preserve the dictionary for test that will be executed later on + static::BackupStaticProperties('Dict'); + + // Reset the dictionary + static::SetNonPublicStaticProperty('Dict', 'm_aData', []); } protected function tearDown(): void @@ -78,6 +83,8 @@ PHP; rmdir(APPROOT."env-$this->sEnvName".DIRECTORY_SEPARATOR."dictionaries"); rmdir(APPROOT."env-$this->sEnvName"); + static::RestoreStaticProperties('Dict'); + parent::tearDown(); } From fba668207f8cda81cc4e2397c30f293ee57c8dc7 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 26 Oct 2023 21:35:52 +0200 Subject: [PATCH 17/20] :white_check_mark: Optimize tests execution time (test rework and defensive cleanup) --- .../core/CMDBSource/CMDBSourceTest.php | 7 ++++ .../core/CMDBSource/TransactionsTest.php | 37 +++++++------------ 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/core/CMDBSource/CMDBSourceTest.php b/tests/php-unit-tests/unitary-tests/core/CMDBSource/CMDBSourceTest.php index 62a6af66d..f0286dd5d 100644 --- a/tests/php-unit-tests/unitary-tests/core/CMDBSource/CMDBSourceTest.php +++ b/tests/php-unit-tests/unitary-tests/core/CMDBSource/CMDBSourceTest.php @@ -26,6 +26,13 @@ class CMDBSourceTest extends ItopTestCase $this->RequireOnceItopFile('/core/cmdbsource.class.inc.php'); } + protected function tearDown(): void + { + DbConnectionWrapper::SetDbConnectionMockForQuery(); + + parent::tearDown(); + } + /** * @covers CMDBSource::IsSameFieldTypes * @dataProvider compareFieldTypesProvider diff --git a/tests/php-unit-tests/unitary-tests/core/CMDBSource/TransactionsTest.php b/tests/php-unit-tests/unitary-tests/core/CMDBSource/TransactionsTest.php index 17de3252d..d4459dd36 100644 --- a/tests/php-unit-tests/unitary-tests/core/CMDBSource/TransactionsTest.php +++ b/tests/php-unit-tests/unitary-tests/core/CMDBSource/TransactionsTest.php @@ -257,36 +257,27 @@ class TransactionsTest extends ItopTestCase /** * @return void - * @doesNotPerformAssertions */ - public function testTransactionOpenedThenClosed() + public function testIsInsideTransaction() { - CMDBSource::Query('START TRANSACTION;'); - CMDBSource::Query('COMMIT;'); - } + static::assertFalse(CMDBSource::IsInsideTransaction(), 'Should not be already inside a transaction'); - /** - * This will throw an exception in the tearDown method. - * This cannot be detected nor by `@expectedException` nor `expectException` method, so we have a specific tearDown impl - * - * @return void - * @doesNotPerformAssertions - */ - public function testTransactionOpenedNotClosed() - { + // First, with a transaction ended by a "COMMIT" statement CMDBSource::Query('START TRANSACTION;'); + static::assertTrue(CMDBSource::IsInsideTransaction(), 'Should be inside a translation'); + CMDBSource::Query('COMMIT;'); + static::assertFalse(CMDBSource::IsInsideTransaction(), 'Should not be inside a transaction anymore'); + + // Second, with a transaction ended by a "ROLLBACK" statement + CMDBSource::Query('START TRANSACTION;'); + static::assertTrue(CMDBSource::IsInsideTransaction(), 'Should be inside a translation (again)'); + CMDBSource::Query('ROLLBACK;'); + static::assertFalse(CMDBSource::IsInsideTransaction(), 'Should not be inside a transaction anymore'); } protected function tearDown(): void { - try { - DbConnectionWrapper::SetDbConnectionMockForQuery(); - parent::tearDown(); - } - catch (MySQLTransactionNotClosedException $e) { - if ($this->getName() === 'testTransactionOpenedNotClosed') { - $this->debug('Executing the testTransactionOpenNoClose method throws a '.MySQLTransactionNotClosedException::class.' exception in tearDown'); - } - } + DbConnectionWrapper::SetDbConnectionMockForQuery(); + parent::tearDown(); } } \ No newline at end of file From 7e8589ba9558c266555e2be5b916d9a1555fd096 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Fri, 27 Oct 2023 09:21:36 +0200 Subject: [PATCH 18/20] :white_check_mark: Fix regression introduced with the optimization done in d641504. Cope with the fact that sometimes the admin account already exists, sometimes not. --- .../unitary-tests/core/UserRightsTest.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php index af6eb3e74..59cbdae88 100644 --- a/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php +++ b/tests/php-unit-tests/unitary-tests/core/UserRightsTest.php @@ -48,11 +48,7 @@ class UserRightsTest extends ItopDataTestCase { parent::setUp(); - try { - utils::GetConfig()->SetModuleSetting('authent-local', 'password_validation.pattern', ''); - } - catch (CoreCannotSaveObjectException $e) { - } + utils::GetConfig()->SetModuleSetting('authent-local', 'password_validation.pattern', ''); } public static $aClasses = [ @@ -102,6 +98,15 @@ class UserRightsTest extends ItopDataTestCase public function testLogin($sLogin, $bResult) { $_SESSION = []; + if ($sLogin == 'admin') { + // Fixture data required in this case only + try { + self::CreateUser('admin', 1); + } + catch (CoreCannotSaveObjectException $e) { + // The admin account could exist, depending on where and when the test suite is executed + } + } $this->assertEquals($bResult, UserRights::Login($sLogin)); $this->assertEquals($bResult, UserRights::IsLoggedIn()); UserRights::Logoff(); From 15148f7d1dd5221489f06ec1aacd6af90ce5883c Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Fri, 27 Oct 2023 10:35:12 +0200 Subject: [PATCH 19/20] :white_check_mark: Fix regression introduced with the optimization done in 798cd10, and seen only if APC is enabled --- tests/php-unit-tests/unitary-tests/core/dictTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/core/dictTest.php b/tests/php-unit-tests/unitary-tests/core/dictTest.php index 528ca5c34..05b7360d5 100644 --- a/tests/php-unit-tests/unitary-tests/core/dictTest.php +++ b/tests/php-unit-tests/unitary-tests/core/dictTest.php @@ -70,9 +70,6 @@ PHP; // Preserve the dictionary for test that will be executed later on static::BackupStaticProperties('Dict'); - - // Reset the dictionary - static::SetNonPublicStaticProperty('Dict', 'm_aData', []); } protected function tearDown(): void From 8fa9336568c5d6641169aac22a95cfa04f5f24bc Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Fri, 27 Oct 2023 11:21:56 +0200 Subject: [PATCH 20/20] :white_check_mark: Fix regression introduced with the optimization done in 15148f7, and seen only in the context of the CI --- tests/php-unit-tests/unitary-tests/core/dictTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/php-unit-tests/unitary-tests/core/dictTest.php b/tests/php-unit-tests/unitary-tests/core/dictTest.php index 05b7360d5..2209d28b9 100644 --- a/tests/php-unit-tests/unitary-tests/core/dictTest.php +++ b/tests/php-unit-tests/unitary-tests/core/dictTest.php @@ -97,6 +97,9 @@ PHP; public function testInitLangIfNeeded_NoApc() { + // Reset the dictionary + static::SetNonPublicStaticProperty('Dict', 'm_aData', []); + $oApcService = $this->createMock(\ApcService::class); Dict::SetApcService($oApcService); Dict::EnableCache('toto');