From 92b61c7491cf3d93c730007b1206941e13ec68bf Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Mon, 10 Jan 2022 17:09:43 +0100 Subject: [PATCH 01/10] =?UTF-8?q?N=C2=B04558=20Rename=20\LogChannels::CMDB?= =?UTF-8?q?SOURCE=20to=20CMDB=5FSOURCE=20to=20match=20existing=20constant?= =?UTF-8?q?=20in=20support/3.0=20branch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbsource.class.inc.php | 4 ++-- core/log.class.inc.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index f77d024cb..668ed60b8 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -794,10 +794,10 @@ class CMDBSource $bHasExistingTransactions = self::IsInsideTransaction(); if (!$bHasExistingTransactions) { - IssueLog::Trace("START TRANSACTION was sent to the DB", LogChannels::CMDBSOURCE, ['stacktrace' => $aStackTrace]); + IssueLog::Trace("START TRANSACTION was sent to the DB", LogChannels::CMDB_SOURCE, ['stacktrace' => $aStackTrace]); self::DBQuery('START TRANSACTION'); } else { - IssueLog::Trace("START TRANSACTION ignored as a transaction is already opened", LogChannels::CMDBSOURCE, ['stacktrace' => $aStackTrace]); + IssueLog::Trace("START TRANSACTION ignored as a transaction is already opened", LogChannels::CMDB_SOURCE, ['stacktrace' => $aStackTrace]); } self::AddTransactionLevel(); diff --git a/core/log.class.inc.php b/core/log.class.inc.php index 457862ff1..c7a56c277 100644 --- a/core/log.class.inc.php +++ b/core/log.class.inc.php @@ -545,7 +545,7 @@ class LogChannels * @var string * @since 2.7.7 N°4558 use this new channel when logging DB transactions */ - const CMDBSOURCE = 'cmdbsource'; + const CMDB_SOURCE = 'cmdbsource'; const DEADLOCK = 'DeadLock'; From 930b224ca296a73be86d4975a216ecfc42ad569a Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Tue, 11 Jan 2022 15:36:40 +0100 Subject: [PATCH 02/10] =?UTF-8?q?:bulb:=20N=C2=B04624=20phpdoc=20for=20Ito?= =?UTF-8?q?pDataTestCase?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/ItopDataTestCase.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/ItopDataTestCase.php b/test/ItopDataTestCase.php index 70b0a06f4..3b505bf7f 100644 --- a/test/ItopDataTestCase.php +++ b/test/ItopDataTestCase.php @@ -54,9 +54,16 @@ define('TAG_CLASS', 'FAQ'); define('TAG_ATTCODE', 'domains'); /** + * Helper class to extend for tests needing access to iTop's metamodel + * + * **⚠ Warning** Each class extending this one needs to add the following annotations : + * * @runTestsInSeparateProcesses * @preserveGlobalState disabled * @backupGlobals disabled + * + * @since 2.7.7 3.0.1 3.1.0 N°4624 processIsolation is disabled by default and must be enabled in each test needing it (basically all tests using + * iTop datamodel) */ class ItopDataTestCase extends ItopTestCase { From 64736f1463de4cf5f3e77eb0292287d121ac75ee Mon Sep 17 00:00:00 2001 From: Molkobain Date: Tue, 11 Jan 2022 15:38:54 +0100 Subject: [PATCH 03/10] Fix unit test provider --- test/setup/iTopDesignFormat/iTopDesignFormatTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/setup/iTopDesignFormat/iTopDesignFormatTest.php b/test/setup/iTopDesignFormat/iTopDesignFormatTest.php index 9e1ed1171..7b94ddd0f 100644 --- a/test/setup/iTopDesignFormat/iTopDesignFormatTest.php +++ b/test/setup/iTopDesignFormat/iTopDesignFormatTest.php @@ -27,7 +27,7 @@ class TestForITopDesignFormatClass extends ItopTestCase /** * @covers iTopDesignFormat::Convert - * @dataProvider testMigrationMethodProvider + * @dataProvider MigrationMethodProvider * * @param string $sTargetVersion * @param string $sInputXmlFileName example "1.7_to_1.6.input" @@ -60,7 +60,7 @@ class TestForITopDesignFormatClass extends ItopTestCase return file_get_contents($sCurrentPath.DIRECTORY_SEPARATOR.$sFileName.'.xml'); } - public function testMigrationMethodProvider() + public function MigrationMethodProvider() { return array( '1.7 to 1.6' => array('1.6', '1.7_to_1.6.input', '1.7_to_1.6.expected'), From b11bf308814e162cb34270990c9c490fc5b93e2f Mon Sep 17 00:00:00 2001 From: Molkobain Date: Tue, 11 Jan 2022 15:48:35 +0100 Subject: [PATCH 04/10] =?UTF-8?q?Unit=20tests:=20Activate=20tests=20that?= =?UTF-8?q?=20were=20never=20ran...=20=F0=9F=A5=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that testGetMysqlCliTlsOptions will fail --- test/phpunit.xml.dist | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/phpunit.xml.dist b/test/phpunit.xml.dist index 6682ccfbd..67df3662c 100644 --- a/test/phpunit.xml.dist +++ b/test/phpunit.xml.dist @@ -53,6 +53,9 @@ itop-tickets + + itop-config + application @@ -63,7 +66,7 @@ status - setup/SetupUtilsTest.php + setup integration From 6855c2f83a96b6a66da5130f3608198af2e7d2fc Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Tue, 11 Jan 2022 15:39:21 +0100 Subject: [PATCH 05/10] =?UTF-8?q?N=C2=B04624=20restore=20backupGlobals=20t?= =?UTF-8?q?o=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/phpunit.xml.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/phpunit.xml.dist b/test/phpunit.xml.dist index 67df3662c..293cc826f 100644 --- a/test/phpunit.xml.dist +++ b/test/phpunit.xml.dist @@ -19,7 +19,7 @@ Date: Tue, 11 Jan 2022 18:13:13 +0100 Subject: [PATCH 06/10] Unit tests: Fix invalid/duplicate class name --- test/itop-config/Validator/iTopConfigSyntaxValidatorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/itop-config/Validator/iTopConfigSyntaxValidatorTest.php b/test/itop-config/Validator/iTopConfigSyntaxValidatorTest.php index dc8a87a1c..74e779c38 100644 --- a/test/itop-config/Validator/iTopConfigSyntaxValidatorTest.php +++ b/test/itop-config/Validator/iTopConfigSyntaxValidatorTest.php @@ -13,7 +13,7 @@ use Combodo\iTop\Test\UnitTest\ItopTestCase; use PhpParser\Node; use PhpParser\PrettyPrinter\Standard; -class iTopConfigAstValidatorTest extends ItopTestCase +class iTopConfigSyntaxValidatorTest extends ItopTestCase { public function setUp() From b3bf516b20fa8a01dfaaa91a792200d14f33a565 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Wed, 12 Jan 2022 08:24:28 +0100 Subject: [PATCH 07/10] :bulb: Fix PHPDoc for \DBBackup::GetMysqlCliTlsOptions --- setup/backup.class.inc.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/setup/backup.class.inc.php b/setup/backup.class.inc.php index a24262922..8287997a4 100644 --- a/setup/backup.class.inc.php +++ b/setup/backup.class.inc.php @@ -462,13 +462,12 @@ EOF; /** - * @see https://dev.mysql.com/doc/refman/5.6/en/encrypted-connection-options.html - * * @param Config $oConfig * * @return string TLS arguments for CLI programs such as mysqldump. Empty string if the config does not use TLS. * - * @since 2.5.0 + * @since 2.5.0 N°1260 + * @link https://dev.mysql.com/doc/refman/5.6/en/connection-options.html#encrypted-connection-options "Command Options for Encrypted Connections" */ public static function GetMysqlCliTlsOptions($oConfig) { From a663e9fdedd3eaf1595bdb53e8b6baa5573215a1 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Wed, 12 Jan 2022 08:40:04 +0100 Subject: [PATCH 08/10] :white_check_mark: Fix DBBackupTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DB connection dependency was added in a222ead4 (N°2336) in \DBBackup::GetMysqlCliTlsOptions but test wasn't updated accordingly :/^ The test wasn't ran on Jenkins until b11bf308, so we saw the regression only yesterday :( This is now fixed ! 🥳 --- core/cmdbsource.class.inc.php | 10 +++++++++- setup/backup.class.inc.php | 3 +++ test/setup/DBBackupTest.php | 35 +++++++++++++++++++++++++---------- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index 668ed60b8..66fd4bb70 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -447,6 +447,12 @@ class CMDBSource } + /** + * @return string + * @throws \MySQLException + * + * @uses \CMDBSource::QueryToCol() so needs a connection opened ! + */ public static function GetDBVersion() { $aVersions = self::QueryToCol('SELECT Version() as version', 'version'); @@ -464,8 +470,10 @@ class CMDBSource /** * Get the DB vendor between MySQL and its main forks * @return string + * + * @uses \CMDBSource::GetServerVariable() so needs a connection opened ! */ - static public function GetDBVendor() + public static function GetDBVendor() { $sDBVendor = static::ENUM_DB_VENDOR_MYSQL; diff --git a/setup/backup.class.inc.php b/setup/backup.class.inc.php index 8287997a4..64d4ff1c0 100644 --- a/setup/backup.class.inc.php +++ b/setup/backup.class.inc.php @@ -466,6 +466,9 @@ EOF; * * @return string TLS arguments for CLI programs such as mysqldump. Empty string if the config does not use TLS. * + * @uses \CMDBSource::GetDBVendor() so needs a connection opened ! + * @uses \CMDBSource::GetDBVersion() so needs a connection opened ! + * * @since 2.5.0 N°1260 * @link https://dev.mysql.com/doc/refman/5.6/en/connection-options.html#encrypted-connection-options "Command Options for Encrypted Connections" */ diff --git a/test/setup/DBBackupTest.php b/test/setup/DBBackupTest.php index 798d453a5..176f02e5d 100644 --- a/test/setup/DBBackupTest.php +++ b/test/setup/DBBackupTest.php @@ -2,9 +2,10 @@ namespace Combodo\iTop\Test\UnitTest\Core; +use CMDBSource; use Combodo\iTop\Test\UnitTest\ItopTestCase; -use Config; use DBBackup; +use utils; /** * @runTestsInSeparateProcesses @@ -21,21 +22,35 @@ class DBBackupTest extends ItopTestCase require_once(APPROOT.'core/cmdbsource.class.inc.php'); // DBBackup dependency } + /** + * @throws \CoreException + * @throws \ConfigException + * @throws \MySQLException + */ public function testGetMysqlCliTlsOptions() { - $oConfig = new Config(); - $oConfig->Set('db_tls.enabled', false); + $oConfigToTest = utils::GetConfig(); - $sCliArgsNoTls = DBBackup::GetMysqlCliTlsOptions($oConfig); + // No TLS connection = no additional CLI args ! + $oConfigToTest->Set('db_tls.enabled', false); + $sCliArgsNoTls = DBBackup::GetMysqlCliTlsOptions($oConfigToTest); $this->assertEmpty($sCliArgsNoTls); - $oConfig->Set('db_tls.enabled', true); - $sCliArgsMinCfg = DBBackup::GetMysqlCliTlsOptions($oConfig); - $this->assertEquals(' --ssl', $sCliArgsMinCfg); + // We need a connection to the DB, so let's open it ! + $oConfigOnDisk = utils::GetConfig(); + CMDBSource::InitFromConfig($oConfigOnDisk); + // TLS connection configured = we need one CLI arg + $oConfigToTest->Set('db_tls.enabled', true); + $sCliArgsMinCfg = DBBackup::GetMysqlCliTlsOptions($oConfigToTest); + // depending on the MySQL version, we would have `--ssl` or `--ssl-mode=VERIFY_CA` + $this->assertStringStartsWith(' --ssl', $sCliArgsMinCfg); + + // TLS connection configured + CA option = we need multiple CLI args $sTestCa = 'my_test_ca'; - $oConfig->Set('db_tls.ca', $sTestCa); - $sCliArgsCapathCfg = DBBackup::GetMysqlCliTlsOptions($oConfig); - $this->assertEquals(' --ssl --ssl-ca="'.$sTestCa.'"', $sCliArgsCapathCfg); + $oConfigToTest->Set('db_tls.ca', $sTestCa); + $sCliArgsCapathCfg = DBBackup::GetMysqlCliTlsOptions($oConfigToTest); + $this->assertStringStartsWith(' --ssl', $sCliArgsMinCfg); + $this->assertStringEndsWith('--ssl-ca="'.$sTestCa.'"', $sCliArgsCapathCfg); } } From 0ee6c60e94efcd3712403b8432da778b113532cd Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Wed, 12 Jan 2022 09:12:04 +0100 Subject: [PATCH 09/10] :white_check_mark: Fix DBBackupTest (again :/) Was working on Windows but not on Linux... --- setup/backup.class.inc.php | 2 +- test/setup/DBBackupTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/setup/backup.class.inc.php b/setup/backup.class.inc.php index 64d4ff1c0..d6cc51e96 100644 --- a/setup/backup.class.inc.php +++ b/setup/backup.class.inc.php @@ -255,7 +255,7 @@ class DBBackup return $aRet; } - protected static function EscapeShellArg($sValue) + public static function EscapeShellArg($sValue) { // Note: See comment from the 23-Apr-2004 03:30 in the PHP documentation // It suggests to rely on pctnl_* function instead of using escapeshellargs diff --git a/test/setup/DBBackupTest.php b/test/setup/DBBackupTest.php index 176f02e5d..99452f888 100644 --- a/test/setup/DBBackupTest.php +++ b/test/setup/DBBackupTest.php @@ -51,6 +51,6 @@ class DBBackupTest extends ItopTestCase $oConfigToTest->Set('db_tls.ca', $sTestCa); $sCliArgsCapathCfg = DBBackup::GetMysqlCliTlsOptions($oConfigToTest); $this->assertStringStartsWith(' --ssl', $sCliArgsMinCfg); - $this->assertStringEndsWith('--ssl-ca="'.$sTestCa.'"', $sCliArgsCapathCfg); + $this->assertStringEndsWith('--ssl-ca='.DBBackup::EscapeShellArg($sTestCa), $sCliArgsCapathCfg); } } From 693a861e7de678fcb8b5233ec0fa5ecac276abb4 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Wed, 12 Jan 2022 09:41:31 +0100 Subject: [PATCH 10/10] :recycle: Refactor DBBackuptest Split each test in a dedicated method --- test/setup/DBBackupTest.php | 51 ++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/test/setup/DBBackupTest.php b/test/setup/DBBackupTest.php index 99452f888..7f767f439 100644 --- a/test/setup/DBBackupTest.php +++ b/test/setup/DBBackupTest.php @@ -14,43 +14,76 @@ use utils; */ class DBBackupTest extends ItopTestCase { + /** + * @throws \CoreException + * @throws \MySQLException + * @throws \ConfigException + */ protected function setUp() { parent::setUp(); require_once(APPROOT.'core/config.class.inc.php'); require_once(APPROOT.'setup/backup.class.inc.php'); require_once(APPROOT.'core/cmdbsource.class.inc.php'); // DBBackup dependency + + // We need a connection to the DB, so let's open it ! + // We are using the default config file... as the server might not be configured for all the combination we are testing + // For example dev env and ci env won't accept TLS connection + $oConfigOnDisk = utils::GetConfig(); + CMDBSource::InitFromConfig($oConfigOnDisk); } /** + * No TLS connection = no additional CLI args ! + * * @throws \CoreException * @throws \ConfigException * @throws \MySQLException */ - public function testGetMysqlCliTlsOptions() + public function testGetMysqlCliTlsOptionsNoTls() { $oConfigToTest = utils::GetConfig(); - // No TLS connection = no additional CLI args ! $oConfigToTest->Set('db_tls.enabled', false); $sCliArgsNoTls = DBBackup::GetMysqlCliTlsOptions($oConfigToTest); + $this->assertEmpty($sCliArgsNoTls); + } - // We need a connection to the DB, so let's open it ! - $oConfigOnDisk = utils::GetConfig(); - CMDBSource::InitFromConfig($oConfigOnDisk); - - // TLS connection configured = we need one CLI arg + /** + * TLS connection configured = we need one CLI arg + * + * @return void + * @throws \ConfigException + * @throws \CoreException + */ + public function testGetMysqlCliTlsOptionsWithTlsNoCa() + { + $oConfigToTest = utils::GetConfig(); $oConfigToTest->Set('db_tls.enabled', true); $sCliArgsMinCfg = DBBackup::GetMysqlCliTlsOptions($oConfigToTest); + // depending on the MySQL version, we would have `--ssl` or `--ssl-mode=VERIFY_CA` $this->assertStringStartsWith(' --ssl', $sCliArgsMinCfg); + } - // TLS connection configured + CA option = we need multiple CLI args + /** + * TLS connection configured + CA option = we need multiple CLI args + * + * @return void + * @throws \ConfigException + * @throws \CoreException + */ + public function testGetMysqlCliTlsOptionsWithTlsAndCa() + { + $oConfigToTest = utils::GetConfig(); $sTestCa = 'my_test_ca'; + + $oConfigToTest->Set('db_tls.enabled', true); $oConfigToTest->Set('db_tls.ca', $sTestCa); $sCliArgsCapathCfg = DBBackup::GetMysqlCliTlsOptions($oConfigToTest); - $this->assertStringStartsWith(' --ssl', $sCliArgsMinCfg); + + $this->assertStringStartsWith(' --ssl', $sCliArgsCapathCfg); $this->assertStringEndsWith('--ssl-ca='.DBBackup::EscapeShellArg($sTestCa), $sCliArgsCapathCfg); } }