From 4774c41573d21ff5173bf40b79bbfd812ef21544 Mon Sep 17 00:00:00 2001 From: MDW Date: Fri, 23 Feb 2024 12:35:38 +0100 Subject: [PATCH 1/6] Fix: Correct CommonClassTest's constructor # Fix: Correct CommonClassTest's constructor Dataproviders did not work because of the issue with the constructor --- test/phpunit/CommonClassTest.class.php | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/phpunit/CommonClassTest.class.php b/test/phpunit/CommonClassTest.class.php index 17bc6dbc5c747..8bc09744f32b9 100644 --- a/test/phpunit/CommonClassTest.class.php +++ b/test/phpunit/CommonClassTest.class.php @@ -37,6 +37,7 @@ } $conf->global->MAIN_DISABLE_ALL_MAILS = 1; +use PHPUnit\Framework\TestCase; /** * Class for PHPUnit tests @@ -45,22 +46,31 @@ * @backupStaticAttributes enabled * @remarks backupGlobals must be disabled to have db,conf,user and lang not erased. */ -class CommonClassTest extends PHPUnit\Framework\TestCase +abstract class CommonClassTest extends TestCase { protected $savconf; protected $savuser; protected $savlangs; protected $savdb; + /** + * Number of Dolibarr log lines to show in case of error + * + * @var integer + */ + public $nbLinesToShow = 100; + /** * Constructor * We save global variables into local variables * - * @param string $name Name + * @param string $name Name + * @param array $data Test data + * @param string $dataName Test data name. */ - public function __construct($name = '') + public function __construct($name = null, array $data = array(), $dataName = '') { - parent::__construct($name); + parent::__construct($name, $data, $dataName); //$this->sharedFixture global $conf,$user,$langs,$db; @@ -104,7 +114,7 @@ protected function onNotSuccessfulTest(Throwable $t): void $lines = file($logfile); - $nbLinesToShow = 100; + $nbLinesToShow = $this->nbLinesToShow; if ($t instanceof PHPUnit\Framework\Error\Notice) { $nbLinesToShow = 3; } From 3e0b7f396fd6e75bcdba74317066922a8dd03dbd Mon Sep 17 00:00:00 2001 From: MDW Date: Fri, 23 Feb 2024 11:12:56 +0100 Subject: [PATCH 2/6] Qual: Refactor module name list, add mapping to class name # Qual: Refactor module name list, add mapping to class name Based on ModuleTest and search for modules in the code, complete the list of modules and map to the class names. This will allow reuse in the ModuleTest. --- test/phpunit/CodingPhpTest.php | 126 ---------------------- test/phpunit/CommonClassTest.class.php | 142 +++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 126 deletions(-) diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index f39e2540ef3a5..79e4f8214d966 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -623,130 +623,4 @@ private function assertModuleIsOk($module_name, $message = '') ); } } - - const DEPRECATED_MODULE_MAPPING = array( - 'actioncomm' => 'agenda', - 'adherent' => 'member', - 'adherent_type' => 'member_type', - 'banque' => 'bank', - 'categorie' => 'category', - 'commande' => 'order', - 'contrat' => 'contract', - 'entrepot' => 'stock', - 'expedition' => 'delivery_note', - 'facture' => 'invoice', - 'fichinter' => 'intervention', - 'product_fournisseur_price' => 'productsupplierprice', - 'product_price' => 'productprice', - 'projet' => 'project', - 'propale' => 'propal', - 'socpeople' => 'contact', - ); - const VALID_MODULE_MAPPING = array( - 'agenda' => 1, - 'member' => 1, - 'member_type' => 1, - 'bank' => 1, - 'category' => 1, - 'delivery_note' => 1, - 'order' => 1, - 'contract' => 1, - 'stock' => 1, - 'invoice' => 1, - 'intervention' => 1, - 'productsupplierprice' => 1, - 'productprice' => 1, - 'project' => 1, - 'propal' => 1, - 'contact' => 1, - 'accounting' => 1, - 'ai' => 1, - 'anothermodule' => 1, - 'api' => 1, - 'asset' => 1, - 'barcode' => 1, - 'blockedlog' => 1, - 'bom' => 1, - 'bookcal' => 1, - 'bookmark' => 1, - 'cashdesk' => 1, - 'clicktodial' => 1, - 'comptabilite' => 1, - 'cron' => 1, - 'datapolicy' => 1, - 'debugbar' => 1, - 'deplacement' => 1, - 'don' => 1, - 'dynamicprices' => 1, - 'ecm' => 1, - 'ecotax' => 1, - 'emailcollector' => 1, - 'eventorganization' => 1, - 'expensereport' => 1, - 'export' => 1, - 'externalsite' => 1, - 'fckeditor' => 1, - 'ficheinter' => 1, - 'fournisseur' => 1, - 'ftp' => 1, - 'google' => 1, - 'gravatar' => 1, - 'holiday' => 1, - 'hrm' => 1, - 'import' => 1, - 'incoterm' => 1, - 'intracommreport' => 1, - 'knowledgemanagement' => 1, - 'label' => 1, - 'ldap' => 1, - 'loan' => 1, - 'mailing' => 1, - 'mailman' => 1, - 'mailmanspip' => 1, - 'margin' => 1, - 'memcached' => 1, - 'modulebuilder' => 1, - 'mrp' => 1, - 'multicompany' => 1, - 'multicurrency' => 1, - 'mymodule' => 1, - 'notification' => 1, - 'numberwords' => 1, - 'openstreetmap' => 1, - 'opensurvey' => 1, - 'partnership' => 1, - 'paybox' => 1, - 'paymentbybanktransfer' => 1, - 'paypal' => 1, - 'paypalplus' => 1, - 'prelevement' => 1, - 'product' => 1, - 'productbatch' => 1, - 'receiptprinter' => 1, - 'reception' => 1, - 'recruitment' => 1, - 'resource' => 1, - 'salaries' => 1, - 'service' => 1, - 'socialnetworks' => 1, - 'societe' => 1, - 'stocktransfer' => 1, - 'stripe' => 1, - 'supplier_invoice' => 1, - 'supplier_order' => 1, - 'supplier_proposal' => 1, - 'syslog' => 1, - 'takepos' => 1, - 'tax' => 1, - 'ticket' => 1, - 'user' => 1, - 'variants' => 1, - 'webhook' => 1, - 'webportal' => 1, - 'webservices' => 1, - 'website' => 1, - 'workflow' => 1, - 'workstation' => 1, - 'zapier' => 1, - ); } diff --git a/test/phpunit/CommonClassTest.class.php b/test/phpunit/CommonClassTest.class.php index 8bc09744f32b9..133754bd47a78 100644 --- a/test/phpunit/CommonClassTest.class.php +++ b/test/phpunit/CommonClassTest.class.php @@ -173,4 +173,146 @@ public static function tearDownAfterClass(): void print __METHOD__."\n"; } + + /** + * Map deprecated module names to new module names + */ + const DEPRECATED_MODULE_MAPPING = array( + 'actioncomm' => 'agenda', + 'adherent' => 'member', + 'adherent_type' => 'member_type', + 'banque' => 'bank', + 'categorie' => 'category', + 'commande' => 'order', + 'contrat' => 'contract', + 'entrepot' => 'stock', + 'expedition' => 'delivery_note', + 'facture' => 'invoice', + 'fichinter' => 'intervention', + 'product_fournisseur_price' => 'productsupplierprice', + 'product_price' => 'productprice', + 'projet' => 'project', + 'propale' => 'propal', + 'socpeople' => 'contact', + ); + + /** + * Map module names to the 'class' name (the class is: mod) + * Value is null when the module is not internal to the default + * Dolibarr setup. + */ + const VALID_MODULE_MAPPING = array( + 'accounting' => 'Accounting', + 'agenda' => 'Agenda', + 'ai' => 'Ai', + 'anothermodule' => null, // Not used in code, used in translations.lang + 'api' => 'Api', + 'asset' => 'Asset', + 'bank' => 'Banque', + 'barcode' => 'Barcode', + 'blockedlog' => 'BlockedLog', + 'bom' => 'Bom', + 'bookcal' => 'BookCal', + 'bookmark' => 'Bookmark', + 'cashdesk' => null, + 'category' => 'Categorie', + 'clicktodial' => 'ClickToDial', + 'TBD_COLLAB' => 'Collab', // TODO: fill in proper name + 'comptabilite' => 'Comptabilite', + 'contact' => null, // TODO: fill in proper class + 'contract' => 'Contrat', + 'cron' => 'Cron', + 'datapolicy' => 'DataPolicy', + 'TBD_DAV' => 'Dav', // TODO: fill in proper name + 'debugbar' => 'DebugBar', + 'delivery_note' => 'Expedition', + 'deplacement' => 'Deplacement', + "TBD_DocGen" => 'DocumentGeneration', // TODO: fill in proper name + 'don' => 'Don', + 'dynamicprices' => 'DynamicPrices', + 'ecm' => 'ECM', + 'ecotax' => null, // TODO: External module ? + 'emailcollector' => 'EmailCollector', + 'eventorganization' => 'EventOrganization', + 'expensereport' => 'ExpenseReport', + 'export' => 'Export', + 'TBD_EXTERNALRSS' => 'ExternalRss', // TODO: fill in proper name + 'externalsite' => 'ExternalSite', + 'fckeditor' => 'Fckeditor', + 'fournisseur' => 'Fournisseur', + 'ftp' => 'FTP', + 'TBD_GEOIPMAXMIND' => 'GeoIPMaxmind', // TODO: fill in proper name + 'google' => null, // External ? + 'gravatar' => 'Gravatar', + 'holiday' => 'Holiday', + 'hrm' => 'HRM', + 'import' => 'Import', + 'incoterm' => 'Incoterm', + 'intervention' => 'Ficheinter', + 'intracommreport' => 'Intracommreport', + 'invoice' => 'Facture', + 'knowledgemanagement' => 'KnowledgeManagement', + 'label' => 'Label', + 'ldap' => 'Ldap', + 'loan' => 'Loan', + 'mailing' => 'Mailing', + 'mailman' => null, // Same module as mailmanspip -> MailmanSpip ?? + 'mailmanspip' => 'MailmanSpip', + 'margin' => 'Margin', + 'member' => 'Adherent', + 'member_type' => null, // TODO: External module ? + 'memcached' => null, // TODO: External module? + 'modulebuilder' => 'ModuleBuilder', + 'mrp' => 'Mrp', + 'multicompany' => null, // Not provided by default, no module tests + 'multicurrency' => 'MultiCurrency', + 'mymodule' => null, // modMyModule - Name used in module builder (avoid false positives) + 'notification' => 'Notification', + 'numberwords' => null, // Not provided by default, no module tests + 'TBD_OAUTH' => 'Oauth', // TODO: set proper name + 'openstreetmap' => null, // External module? + 'opensurvey' => 'OpenSurvey', + 'order' => 'Commande', + 'partnership' => 'Partnership', + 'paybox' => 'Paybox', + 'paymentbybanktransfer' => 'PaymentByBankTransfer', + 'paypal' => 'Paypal', + 'paypalplus' => null, + 'prelevement' => 'Prelevement', + 'TBD_PRINTING' => 'Printing', // TODO: set proper name + 'product' => 'Product', + 'productbatch' => 'ProductBatch', + 'productprice' => null, + 'productsupplierprice' => null, + 'project' => 'Projet', + 'propal' => 'Propale', + 'receiptprinter' => 'ReceiptPrinter', + 'reception' => 'Reception', + 'recruitment' => 'Recruitment', + 'resource' => 'Resource', + 'salaries' => 'Salaries', + 'service' => 'Service', + 'socialnetworks' => 'SocialNetworks', + 'societe' => 'Societe', + 'stock' => 'Stock', + 'stocktransfer' => 'StockTransfer', + 'stripe' => 'Stripe', + 'supplier_invoice' => null, // Special case, uses invoice + 'supplier_order' => null, // Special case, uses invoice + 'supplier_proposal' => 'SupplierProposal', + 'syslog' => 'Syslog', + 'takepos' => 'TakePos', + 'tax' => 'Tax', + 'ticket' => 'Ticket', + 'user' => 'User', + 'variants' => 'Variants', + 'webhook' => 'Webhook', + 'webportal' => 'WebPortal', + 'webservices' => 'WebServices', + 'TBD_WS_CLIENT' => 'WebServicesClient', // TODO: set proper name + 'website' => 'Website', + 'workflow' => 'Workflow', + 'workstation' => 'Workstation', + 'zapier' => 'Zapier', + ); } From e41d058032d76efb31c93aad9a8f580e2309f870 Mon Sep 17 00:00:00 2001 From: MDW Date: Fri, 23 Feb 2024 11:35:24 +0100 Subject: [PATCH 3/6] Qual: Refactor ModulesInit test # Qual: Refactor ModulesInit test Use the updated common module mapping list, more complete than the original list. Also refactor the test to use a data provider. --- test/phpunit/ModulesTest.php | 60 ++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/test/phpunit/ModulesTest.php b/test/phpunit/ModulesTest.php index 0d36b498c91a6..2745b87529028 100644 --- a/test/phpunit/ModulesTest.php +++ b/test/phpunit/ModulesTest.php @@ -1,6 +1,7 @@ * Copyright (C) 2023 Alexandre Janniaux + * Copyright (C) 2024 MDW * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -38,6 +39,8 @@ $conf->global->MAIN_DISABLE_ALL_MAILS = 1; +use PHPUnit\Framework\TestCase; + /** * Class for PHPUnit tests * @@ -45,45 +48,56 @@ * @backupStaticAttributes enabled * @remarks backupGlobals must be disabled to have db,conf,user and lang not erased. */ -class ModulesTest extends CommonClassTest +class ModulesTest extends CommonClassTest // TestCase //CommonClassTest { + /** + * Return list of modules for which to test initialisation + * + * @return array List of module labels to test (class is mod) + */ + public function moduleInitListProvider() + { + $full_list = self::VALID_MODULE_MAPPING; + $filtered_list = array_map(function ($value) { + return array($value); + }, array_filter($full_list, function ($value) { + return $value !== null; + })); + return $filtered_list; + } + /** * testModulesInit * + * @param string $modlabel Module label (class is mod) + * * @return int + * + * @dataProvider moduleInitListProvider */ - public function testModulesInit() + public function testModulesInit(string $modlabel) { global $conf,$user,$langs,$db; + $conf = $this->savconf; $user = $this->savuser; $langs = $this->savlangs; $db = $this->savdb; - $modulelist = array('Accounting','Adherent','Agenda','Api','Asset','Banque','Barcode','BlockedLog','Bom','Bookmark', - 'Categorie','ClickToDial','Collab','Commande','Comptabilite','Contrat','Cron','DataPolicy','Dav','DebugBar','Deplacement','DocumentGeneration','Don','DynamicPrices', - 'ECM','EmailCollector','EventOrganization','Expedition','ExpenseReport','Export','ExternalRss','ExternalSite', - 'Facture','Fckeditor','Ficheinter','Fournisseur','FTP','GeoIPMaxmind','Gravatar','Holiday','HRM','Import','Incoterm','Intracommreport', - 'KnowledgeManagement','Label','Ldap','Loan', - 'Mailing','MailmanSpip','Margin','ModuleBuilder','Mrp','MultiCurrency', - 'Notification','Oauth','OpenSurvey','Paybox','PaymentByBankTransfer','Paypal','Prelevement','Printing','Product','ProductBatch','Projet','Propale', - 'ReceiptPrinter','Reception','Recruitment','Resource', - 'Salaries','Service','SocialNetworks','Societe','Stock','Stripe','SupplierProposal','Syslog', - 'TakePos','Tax','Ticket','User','Variants','WebServices','WebServicesClient','Website','Workflow','Workstation','Zapier'); - foreach ($modulelist as $modlabel) { - require_once DOL_DOCUMENT_ROOT.'/core/modules/mod'.$modlabel.'.class.php'; - $class = 'mod'.$modlabel; - $mod = new $class($db); + $this->nbLinesToShow = 0; // Only 3 lines of the log. - $result = $mod->remove(); - $result = $mod->init(); + require_once DOL_DOCUMENT_ROOT.'/core/modules/mod'.$modlabel.'.class.php'; + $class = 'mod'.$modlabel; + $mod = new $class($db); + + $result = $mod->remove(); + $result = $mod->init(); - $this->assertLessThan($result, 0, $modlabel." ".$mod->error); - print __METHOD__." test remove/init for module ".$modlabel.", result=".$result."\n"; + $this->assertLessThan($result, 0, $modlabel." ".$mod->error); + print __METHOD__." test remove/init for module ".$modlabel.", result=".$result."\n"; - if (in_array($modlabel, array('Ldap', 'MailmanSpip'))) { - $result = $mod->remove(); - } + if (in_array($modlabel, array('Ldap', 'MailmanSpip'))) { + $result = $mod->remove(); } return 0; From a0f77516298835ca4b547a4300a7456f497a1551 Mon Sep 17 00:00:00 2001 From: MDW Date: Fri, 23 Feb 2024 13:01:00 +0100 Subject: [PATCH 4/6] Fix: valid module test must now use array_key_exists # Fix: valid module test must now use array_key_exists Because of the introduction of null key values, isset on the array no longer works, using array_key_exists to test if the modulename is valid. --- test/phpunit/CodingPhpTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 79e4f8214d966..c72f5859ea679 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -618,7 +618,7 @@ private function assertModuleIsOk($module_name, $message = '') //trigger_error("Deprecated module name, use '$new_name': $message", E_USER_DEPRECATED); } else { $this->assertTrue( - isset(self::VALID_MODULE_MAPPING[$module_name]), + array_key_exists($module_name, self::VALID_MODULE_MAPPING), "Unknown module: $message" ); } From 64402345f4928e3e4fb471615a41ddd2cb5efe7d Mon Sep 17 00:00:00 2001 From: MDW Date: Fri, 23 Feb 2024 13:09:58 +0100 Subject: [PATCH 5/6] fixup! Qual: Refactor module name list, add mapping to class name --- test/phpunit/CommonClassTest.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/phpunit/CommonClassTest.class.php b/test/phpunit/CommonClassTest.class.php index 133754bd47a78..61cb5a27aa914 100644 --- a/test/phpunit/CommonClassTest.class.php +++ b/test/phpunit/CommonClassTest.class.php @@ -188,7 +188,7 @@ public static function tearDownAfterClass(): void 'entrepot' => 'stock', 'expedition' => 'delivery_note', 'facture' => 'invoice', - 'fichinter' => 'intervention', + 'ficheinter' => 'intervention', 'product_fournisseur_price' => 'productsupplierprice', 'product_price' => 'productprice', 'projet' => 'project', From 823d128ea1860e445abdf1af1d288283b4e32a49 Mon Sep 17 00:00:00 2001 From: MDW Date: Fri, 23 Feb 2024 13:18:06 +0100 Subject: [PATCH 6/6] Qual: Less verbosity for tests # Qual: Less verbosity for tests The verbosity on setup/teardown/... is not really usefull and makes the log less readable. Reducing the verbosity while allowing to set an environment variable PHPUNIT_DEBUG to enable it. --- test/phpunit/CommonClassTest.class.php | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/test/phpunit/CommonClassTest.class.php b/test/phpunit/CommonClassTest.class.php index 61cb5a27aa914..9e3aaf1656956 100644 --- a/test/phpunit/CommonClassTest.class.php +++ b/test/phpunit/CommonClassTest.class.php @@ -81,7 +81,7 @@ public function __construct($name = null, array $data = array(), $dataName = '') print __METHOD__." db->type=".$db->type." user->id=".$user->id; //print " - db ".$db->db; - print "\n"; + print PHP_EOL; } /** @@ -95,11 +95,13 @@ public static function setUpBeforeClass(): void $db->begin(); // This is to have all actions inside a transaction even if test launched without suite. if (!isModEnabled('agenda')) { - print __METHOD__." module agenda must be enabled.\n"; + print get_called_class()." module agenda must be enabled.".PHP_EOL; die(1); } - print __METHOD__."\n"; + if (isset($_ENV['PHPUNIT_DEBUG'])) { + print get_called_class().PHP_EOL; + } } /** @@ -125,10 +127,11 @@ protected function onNotSuccessfulTest(Throwable $t): void $last_lines = array_slice($lines, $first_line, $nbLinesToShow); // Show log file - print "\n----- Test fails. Show last ".$nbLinesToShow." lines of dolibarr.log file -----\n"; + print PHP_EOL."----- Test fails. Show last ".$nbLinesToShow." lines of dolibarr.log file -----".PHP_EOL; foreach ($last_lines as $line) { print $line . "
"; } + print PHP_EOL; parent::onNotSuccessfulTest($t); } @@ -147,6 +150,9 @@ protected function setUp(): void $langs = $this->savlangs; $db = $this->savdb; + if (isset($_ENV['PHPUNIT_DEBUG'])) { + print get_called_class().PHP_EOL; + } print __METHOD__."\n"; //print $db->getVersion()."\n"; } @@ -158,7 +164,9 @@ protected function setUp(): void */ protected function tearDown(): void { - print __METHOD__."\n"; + if (isset($_ENV['PHPUNIT_DEBUG'])) { + print get_called_class().PHP_EOL; + } } /** @@ -170,8 +178,9 @@ public static function tearDownAfterClass(): void { global $db; $db->rollback(); - - print __METHOD__."\n"; + if (isset($_ENV['PHPUNIT_DEBUG'])) { + print get_called_class().PHP_EOL; + } } /**