From f3a3de8ea24c102c3905f764a8bd319d277af535 Mon Sep 17 00:00:00 2001 From: MDW Date: Fri, 23 Feb 2024 17:43:18 +0100 Subject: [PATCH] Qual: Fix/complete/refactor ModuleTest case. (#28373) * Fix: Correct CommonClassTest's constructor # Fix: Correct CommonClassTest's constructor Dataproviders did not work because of the issue with the constructor * 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. * 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. * 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. * fixup! Qual: Refactor module name list, add mapping to class name * 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/CodingPhpTest.php | 128 +---------------- test/phpunit/CommonClassTest.class.php | 185 +++++++++++++++++++++++-- test/phpunit/ModulesTest.php | 62 +++++---- 3 files changed, 212 insertions(+), 163 deletions(-) diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index f39e2540ef3..c72f5859ea6 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -618,135 +618,9 @@ class CodingPhpTest extends CommonClassTest //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" ); } } - - 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 17bc6dbc5c7..9e3aaf16569 100644 --- a/test/phpunit/CommonClassTest.class.php +++ b/test/phpunit/CommonClassTest.class.php @@ -37,6 +37,7 @@ if (empty($user->id)) { } $conf->global->MAIN_DISABLE_ALL_MAILS = 1; +use PHPUnit\Framework\TestCase; /** * Class for PHPUnit tests @@ -45,22 +46,31 @@ $conf->global->MAIN_DISABLE_ALL_MAILS = 1; * @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; @@ -71,7 +81,7 @@ class CommonClassTest extends PHPUnit\Framework\TestCase print __METHOD__." db->type=".$db->type." user->id=".$user->id; //print " - db ".$db->db; - print "\n"; + print PHP_EOL; } /** @@ -85,11 +95,13 @@ class CommonClassTest extends PHPUnit\Framework\TestCase $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; + } } /** @@ -104,7 +116,7 @@ class CommonClassTest extends PHPUnit\Framework\TestCase $lines = file($logfile); - $nbLinesToShow = 100; + $nbLinesToShow = $this->nbLinesToShow; if ($t instanceof PHPUnit\Framework\Error\Notice) { $nbLinesToShow = 3; } @@ -115,10 +127,11 @@ class CommonClassTest extends PHPUnit\Framework\TestCase $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); } @@ -137,6 +150,9 @@ class CommonClassTest extends PHPUnit\Framework\TestCase $langs = $this->savlangs; $db = $this->savdb; + if (isset($_ENV['PHPUNIT_DEBUG'])) { + print get_called_class().PHP_EOL; + } print __METHOD__."\n"; //print $db->getVersion()."\n"; } @@ -148,7 +164,9 @@ class CommonClassTest extends PHPUnit\Framework\TestCase */ protected function tearDown(): void { - print __METHOD__."\n"; + if (isset($_ENV['PHPUNIT_DEBUG'])) { + print get_called_class().PHP_EOL; + } } /** @@ -160,7 +178,150 @@ class CommonClassTest extends PHPUnit\Framework\TestCase { global $db; $db->rollback(); - - print __METHOD__."\n"; + if (isset($_ENV['PHPUNIT_DEBUG'])) { + print get_called_class().PHP_EOL; + } } + + /** + * 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', + 'ficheinter' => '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', + ); } diff --git a/test/phpunit/ModulesTest.php b/test/phpunit/ModulesTest.php index 0d36b498c91..2745b875290 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 @@ if (empty($user->id)) { $conf->global->MAIN_DISABLE_ALL_MAILS = 1; +use PHPUnit\Framework\TestCase; + /** * Class for PHPUnit tests * @@ -45,45 +48,56 @@ $conf->global->MAIN_DISABLE_ALL_MAILS = 1; * @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. + 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"; + + if (in_array($modlabel, array('Ldap', 'MailmanSpip'))) { $result = $mod->remove(); - $result = $mod->init(); - - $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(); - } } return 0;