From 42e660cf6236a7f9c8c00e7a88c3d6410de00738 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Wed, 27 Nov 2024 12:27:06 +0100 Subject: [PATCH 01/10] Migrate imap tests to phpunit --- .github/actions/test_tests-imap.sh | 17 +- .../imap/MailCollectorTest.php | 350 +++++++++--------- tests/README.md | 4 +- 3 files changed, 181 insertions(+), 190 deletions(-) rename tests/imap/MailCollector.php => phpunit/imap/MailCollectorTest.php (82%) diff --git a/.github/actions/test_tests-imap.sh b/.github/actions/test_tests-imap.sh index 154b7aa923e..23de9f72b8c 100755 --- a/.github/actions/test_tests-imap.sh +++ b/.github/actions/test_tests-imap.sh @@ -1,23 +1,14 @@ #!/bin/bash set -e -u -x -o pipefail -ATOUM_ADDITIONNAL_OPTIONS="" +PHPUNIT_ADDITIONNAL_OPTIONS="" if [[ "$CODE_COVERAGE" = true ]]; then export COVERAGE_DIR="coverage-imap" + PHPUNIT_ADDITIONNAL_OPTIONS="--coverage-filter src --coverage-clover phpunit/$COVERAGE_DIR/clover.xml" else - ATOUM_ADDITIONNAL_OPTIONS="--no-code-coverage"; + PHPUNIT_ADDITIONNAL_OPTIONS="--no-coverage"; fi -vendor/bin/atoum \ - -p 'php -d memory_limit=512M' \ - --debug \ - --force-terminal \ - --use-dot-report \ - --bootstrap-file tests/bootstrap.php \ - --fail-if-void-methods \ - --fail-if-skipped-methods \ - $ATOUM_ADDITIONNAL_OPTIONS \ - --max-children-number 1 \ - -d tests/imap +vendor/bin/phpunit $PHPUNIT_ADDITIONNAL_OPTIONS phpunit/imap unset COVERAGE_DIR diff --git a/tests/imap/MailCollector.php b/phpunit/imap/MailCollectorTest.php similarity index 82% rename from tests/imap/MailCollector.php rename to phpunit/imap/MailCollectorTest.php index edb27524e6a..ace2eed356b 100644 --- a/tests/imap/MailCollector.php +++ b/phpunit/imap/MailCollectorTest.php @@ -47,43 +47,43 @@ use Ticket; use Psr\Log\LogLevel; -class MailCollector extends DbTestCase +class MailCollectorTest extends DbTestCase { private $collector; private $mailgate_id; public function testGetEmpty() { - $this - ->if($this->newTestedInstance) - ->then - ->array($this->testedInstance->fields) - ->isIdenticalTo([]) - ->boolean($this->testedInstance->getEmpty()) - ->array($this->testedInstance->fields) - ->isIdenticalTo([ - 'id' => '', - 'name' => '', - 'host' => '', - 'login' => '', - 'filesize_max' => '2097152', - 'is_active' => 1, - 'date_mod' => '', - 'comment' => '', - 'passwd' => '', - 'accepted' => '', - 'refused' => '', - 'errors' => '', - 'use_mail_date' => '', - 'date_creation' => '', - 'requester_field' => '', - 'add_cc_to_observer' => '', - 'collect_only_unread' => '', - 'last_collect_date' => '', - ]); + $instance = new \MailCollector(); + $this->assertSame([], $instance->fields); + + $this->assertTrue($instance->getEmpty()); + $this->assertSame( + [ + 'id' => '', + 'name' => '', + 'host' => '', + 'login' => '', + 'filesize_max' => '2097152', + 'is_active' => 1, + 'date_mod' => '', + 'comment' => '', + 'passwd' => '', + 'accepted' => '', + 'refused' => '', + 'errors' => '', + 'use_mail_date' => '', + 'date_creation' => '', + 'requester_field' => '', + 'add_cc_to_observer' => '', + 'collect_only_unread' => '', + 'last_collect_date' => '', + ], + $instance->fields + ); } - protected function subjectProvider() + public static function subjectProvider() { return [ [ @@ -107,116 +107,120 @@ protected function subjectProvider() */ public function testCleanSubject($raw, $expected) { - $this - ->if($this->newTestedInstance) - ->then - ->string($this->testedInstance->cleanSubject($raw)) - ->isIdenticalTo($expected); + $instance = new \MailCollector(); + $this->assertSame($expected, $instance->cleanSubject($raw)); } public function testListEncodings() { - $this->newTestedInstance; - - $this->when( - function () { - $this->array($this->testedInstance->listEncodings()) - ->containsValues(['utf-8', 'iso-8859-1', 'iso-8859-14', 'cp1252']); - } - )->error() - ->withType(E_USER_DEPRECATED) - ->withMessage('Called method is deprecated') - ->exists(); + $instance = new \MailCollector(); + $encodings = $instance->listEncodings(); + $this->assertContains('utf-8', $encodings); + $this->assertContains('iso-8859-1', $encodings); + $this->assertContains('iso-8859-14', $encodings); + $this->assertContains('cp1252', $encodings); + $this->hasPhpLogRecordThatContains( + 'Called method is deprecated', + LogLevel::NOTICE + ); } public function testPrepareInput() { $_SESSION['glpicronuserrunning'] = 'cron_phpunit'; - $this->newTestedInstance(); + $instance = new \MailCollector(); $oinput = [ 'passwd' => 'Ph34r', 'is_active' => true ]; - $prepared = $this->testedInstance->prepareInput($oinput, 'add'); - $this->array($prepared) - ->boolean['is_active']->isTrue() - ->string['passwd']->isNotEqualTo($oinput['passwd']); + $prepared = $instance->prepareInput($oinput, 'add'); + $this->assertIsArray($prepared); + $this->assertTrue($prepared['is_active']); + $this->assertNotEquals($oinput['passwd'], $prepared['passwd']); - //empty password means no password. + //empty password means no password. $oinput = [ 'passwd' => '', 'is_active' => true ]; - $this->array($this->testedInstance->prepareInput($oinput, 'add')) - ->isIdenticalTo(['is_active' => true]); + $this->assertSame( + ['is_active' => true], + $instance->prepareInput($oinput, 'add') + ); - //manage host + //manage host $oinput = [ 'mail_server' => 'mail.example.com' ]; - $this->array($this->testedInstance->prepareInput($oinput, 'add')) - ->isIdenticalTo(['mail_server' => 'mail.example.com', 'host' => '{mail.example.com}']); + $this->assertSame( + ['mail_server' => 'mail.example.com', 'host' => '{mail.example.com}'], + $instance->prepareInput($oinput, 'add') + ); - //manage host + //manage host $oinput = [ 'mail_server' => 'mail.example.com', 'server_port' => 143, 'server_mailbox' => 'bugs' ]; - $this->array($this->testedInstance->prepareInput($oinput, 'add')) - ->isIdenticalTo([ - 'mail_server' => 'mail.example.com', - 'server_port' => 143, - 'server_mailbox' => 'bugs', - 'host' => '{mail.example.com:143}bugs' - ]); + $this->assertSame( + [ + 'mail_server' => 'mail.example.com', + 'server_port' => 143, + 'server_mailbox' => 'bugs', + 'host' => '{mail.example.com:143}bugs' + ], + $instance->prepareInput($oinput, 'add') + ); $oinput = [ 'passwd' => 'Ph34r', '_blank_passwd' => true ]; - $this->array($this->testedInstance->prepareInputForUpdate($oinput)) - ->isIdenticalTo(['passwd' => '', '_blank_passwd' => true]); + $this->assertSame( + ['passwd' => '', '_blank_passwd' => true], + $instance->prepareInputForUpdate($oinput) + ); } public function testCounts() { $_SESSION['glpicronuserrunning'] = 'cron_phpunit'; - $this->newTestedInstance(); + $instance = new \MailCollector(); - $this->integer($this->testedInstance->countActiveCollectors())->isIdenticalTo(0); - $this->integer($this->testedInstance->countCollectors(true))->isIdenticalTo(0); - $this->integer($this->testedInstance->countCollectors())->isIdenticalTo(0); + $this->assertEquals(0, $instance->countActiveCollectors()); + $this->assertEquals(0, $instance->countCollectors(true)); + $this->assertEquals(0, $instance->countCollectors()); - //Add an active collector - $nid = (int)$this->testedInstance->add([ + //Add an active collector + $nid = (int)$instance->add([ 'name' => 'Maille name', 'is_active' => true ]); - $this->integer($nid)->isGreaterThan(0); + $this->assertGreaterThan(0, $nid); - $this->integer($this->testedInstance->countActiveCollectors())->isIdenticalTo(1); - $this->integer($this->testedInstance->countCollectors(true))->isIdenticalTo(1); - $this->integer($this->testedInstance->countCollectors())->isIdenticalTo(1); + $this->assertEquals(1, $instance->countActiveCollectors()); + $this->assertEquals(1, $instance->countCollectors(true)); + $this->assertEquals(1, $instance->countCollectors()); - $this->boolean( - $this->testedInstance->update([ - 'id' => $this->testedInstance->fields['id'], + $this->assertTrue( + $instance->update([ + 'id' => $instance->fields['id'], 'is_active' => false ]) - )->isTrue(); + ); - $this->integer($this->testedInstance->countActiveCollectors())->isIdenticalTo(0); - $this->integer($this->testedInstance->countCollectors(true))->isIdenticalTo(0); - $this->integer($this->testedInstance->countCollectors())->isIdenticalTo(1); + $this->assertEquals(0, $instance->countActiveCollectors()); + $this->assertEquals(0, $instance->countCollectors(true)); + $this->assertEquals(1, $instance->countCollectors()); } - protected function messageIdHeaderProvider() + public static function messageIdHeaderProvider() { $root_ent_id = getItemByTypeName('Entity', '_test_root_entity', true); @@ -330,7 +334,7 @@ protected function messageIdHeaderProvider() */ public function testIsMessageSentByGlpi(array $headers, bool $expected) { - $this->newTestedInstance(); + $instance = new \MailCollector(); $message = new Message( [ @@ -339,10 +343,10 @@ public function testIsMessageSentByGlpi(array $headers, bool $expected) ] ); - $this->boolean($this->testedInstance->isMessageSentByGlpi($message))->isEqualTo($expected); + $this->assertEquals($expected, $instance->isMessageSentByGlpi($message)); } - protected function itemReferenceHeaderProvider() + public static function itemReferenceHeaderProvider() { $root_ent_id = getItemByTypeName('Entity', '_test_root_entity', true); @@ -552,7 +556,7 @@ public function testGetItemFromHeader( ?int $expected_items_id, bool $accepted ) { - $this->newTestedInstance(); + $instance = new \MailCollector(); $message = new Message( [ @@ -561,13 +565,13 @@ public function testGetItemFromHeader( ] ); - $item = $this->testedInstance->getItemFromHeaders($message); + $item = $instance->getItemFromHeaders($message); if ($expected_itemtype === null) { - $this->variable($item)->isNull(); + $this->assertNull($item); } else { - $this->object($item)->isInstanceOf($expected_itemtype); - $this->integer($item->getId())->isEqualTo($expected_items_id); + $this->assertInstanceOf($expected_itemtype, $item); + $this->assertEquals($expected_items_id, $item->getId()); } } @@ -580,7 +584,7 @@ public function testIsResponseToMessageSentByAnotherGlpi( ?int $expected_items_id, bool $accepted ) { - $this->newTestedInstance(); + $instance = new \MailCollector(); $message = new Message( [ @@ -589,7 +593,7 @@ public function testIsResponseToMessageSentByAnotherGlpi( ] ); - $this->boolean($this->testedInstance->isResponseToMessageSentByAnotherGlpi($message))->isEqualTo(!$accepted); + $this->assertEquals(!$accepted, $instance->isResponseToMessageSentByAnotherGlpi($message)); } private function doConnect() @@ -617,12 +621,12 @@ private function doConnect() 'requester_field' => \MailCollector::REQUESTER_FIELD_REPLY_TO, ]); - $this->integer($this->mailgate_id)->isGreaterThan(0); + $this->assertGreaterThan(0, $this->mailgate_id); - $this->boolean($collector->getFromDB($this->mailgate_id))->isTrue(); - $this->string($collector->fields['host'])->isIdenticalTo('{dovecot:143/imap/novalidate-cert}'); + $this->assertTrue($collector->getFromDB($this->mailgate_id)); + $this->assertSame('{dovecot:143/imap/novalidate-cert}', $collector->fields['host']); $collector->connect(); - $this->variable($collector->fields['errors'])->isEqualTo(0); + $this->assertEquals(0, $collector->fields['errors']); } public function testCollect() @@ -630,38 +634,41 @@ public function testCollect() global $DB; $_SESSION['glpicronuserrunning'] = 'cron_phpunit'; - // Force notification_uuid + // Force notification_uuid Config::setConfigurationValues('core', ['notification_uuid' => 't3StN0t1f1c4tiOnUUID']); - //assign email to user + //assign email to user $nuid = getItemByTypeName('User', 'normal', true); $uemail = new \UserEmail(); - $this->integer( + $this->assertGreaterThan( + 0, (int)$uemail->add([ 'users_id' => $nuid, 'is_default' => 1, 'email' => 'normal@glpi-project.org' ]) - )->isGreaterThan(0); + ); $tuid = getItemByTypeName('User', 'tech', true); - $this->integer( + $this->assertGreaterThan( + 0, (int)$uemail->add([ 'users_id' => $tuid, 'is_default' => 1, 'email' => 'tech@glpi-project.org' ]) - )->isGreaterThan(0); + ); - // Hack to allow documents named "1234567890", "1234567890_2", ... (cf 28-multiple-attachments-no-extension.eml) + // Hack to allow documents named "1234567890", "1234567890_2", ... (cf 28-multiple-attachments-no-extension.eml) $doctype = new \DocumentType(); - $this->integer( + $this->assertGreaterThan( + 0, $doctype->add([ 'name' => 'Type test', 'ext' => '/^1234567890(_\\\d+)?$/' ]) - )->isGreaterThan(0); + ); - // Collect all mails + // Collect all mails $this->doConnect(); $this->collector->maxfetch_emails = 1000; // Be sure to fetch all mails from test suite @@ -672,16 +679,11 @@ public function testCollect() 'Header with Name date or date not found' => LogLevel::CRITICAL, ]; - $msg = null; - $this->when( - function () use (&$msg) { - $msg = $this->collector->collect($this->mailgate_id); - } - ) - ->error() - ->withType(E_USER_WARNING) - ->withMessage('Invalid header "X-Invalid-Encoding"') - ->exists(); + $msg = $this->collector->collect($this->mailgate_id); + $this->hasPhpLogRecordThatContains( + 'Invalid header "X-Invalid-Encoding"', + LogLevel::WARNING + ); // Check error log and clean it (to prevent test failure, see GLPITestCase::afterTestMethod()). foreach ($expected_logged_errors as $error_message => $error_level) { @@ -694,7 +696,7 @@ function () use (&$msg) { $expected_blacklist_count = 3; $expected_expected_already_seen = 0; - $this->variable($msg)->isIdenticalTo( + $this->assertSame( sprintf( 'Number of messages: available=%1$s, already imported=%2$d, retrieved=%3$s, refused=%4$s, errors=%5$s, blacklisted=%6$s', $total_count, @@ -703,10 +705,11 @@ function () use (&$msg) { $expected_refused_count, $expected_error_count, $expected_blacklist_count - ) + ), + $msg ); - // Check not imported emails + // Check not imported emails $not_imported_specs = [ [ 'subject' => 'Have a problem, can you help me?', @@ -728,7 +731,7 @@ function () use (&$msg) { ] ]; $iterator = $DB->request(['FROM' => \NotImportedEmail::getTable()]); - $this->integer(count($iterator))->isIdenticalTo(count($not_imported_specs)); + $this->assertEquals(count($not_imported_specs), count($iterator)); $not_imported_values = []; foreach ($iterator as $data) { @@ -738,13 +741,13 @@ function () use (&$msg) { 'to' => $data['to'], 'reason' => $data['reason'], ]; - $this->integer($data['mailcollectors_id'])->isIdenticalTo($this->mailgate_id); + $this->assertSame($this->mailgate_id, $data['mailcollectors_id']); } - $this->array($not_imported_values)->isIdenticalTo($not_imported_specs); + $this->assertSame($not_imported_specs, $not_imported_values); - // Check created tickets and their actors + // Check created tickets and their actors $actors_specs = [ - // Mails having "tech" user as requester + // Mails having "tech" user as requester [ 'users_id' => $tuid, 'actor_type' => \CommonITILActor::REQUESTER, @@ -755,7 +758,7 @@ function () use (&$msg) { 'A message without to header', ] ], - // Mails having "normal" user as requester + // Mails having "normal" user as requester [ 'users_id' => $nuid, 'actor_type' => \CommonITILActor::REQUESTER, @@ -796,7 +799,7 @@ function () use (&$msg) { '43 - Korean encoding issue', ] ], - // Mails having "normal" user as observer (add_cc_to_observer = true) + // Mails having "normal" user as observer (add_cc_to_observer = true) [ 'users_id' => $nuid, 'actor_type' => \CommonITILActor::OBSERVER, @@ -806,9 +809,9 @@ function () use (&$msg) { ], ]; - // Tickets on which content should be checked (key is ticket name) + // Tickets on which content should be checked (key is ticket name) $tickets_contents = [ - // Plain text on mono-part email + // Plain text on mono-part email 'PHP fatal error' => << <<<HTML <p>This message have reply to header, requester should be get from this header.</p> HTML, @@ -937,28 +940,28 @@ function () use (&$msg) { ] ]); - $this->integer(count($iterator))->isIdenticalTo(count($actor_specs['tickets_names'])); + $this->assertSame(count($actor_specs['tickets_names']), count($iterator)); $names = []; foreach ($iterator as $data) { $name = $data['name']; if (array_key_exists($name, $tickets_contents)) { - $this->string(Sanitizer::unsanitize($data['content']))->isEqualTo($tickets_contents[$name]); + $this->assertEquals($tickets_contents[$name], Sanitizer::unsanitize($data['content'])); } - $this->string($data['content'])->notContains('cid:'); // check that image were correctly imported + $this->assertNotContains('cid:', $data['content']); // check that images were correctly imported $names[] = $name; } - $this->array($names)->isIdenticalTo($actor_specs['tickets_names']); + $this->assertSame($actor_specs['tickets_names'], $names); } - // Check creation of expected documents + // Check creation of expected documents $expected_docs = [ '00-logoteclib.png' => 'image/png', - // Space is missing between "France" and "très" due to a bug in laminas-mail + // Space is missing between "France" and "très" due to a bug in laminas-mail '01-screenshot-2018-4-12-observatoire-francetres-haut-debit.png' => 'image/png', '01-test.JPG' => 'image/jpeg', '15-image001.png' => 'image/png', @@ -1000,11 +1003,11 @@ function () use (&$msg) { foreach ($iterator as $data) { $filenames[$data['filename']] = $data['mime']; } - $this->array($filenames)->isIdenticalTo($expected_docs); + $this->assertSame($expected_docs, $filenames); - $this->integer(count($iterator))->isIdenticalTo(count($expected_docs)); + $this->assertSame(count($expected_docs), count($iterator)); - // Check creation of expected followups + // Check creation of expected followups $expected_followups = [ [ 'items_id' => 100, @@ -1034,11 +1037,11 @@ function () use (&$msg) { ]; foreach ($expected_followups as $expected_followup) { - $this->integer(countElementsInTable(ITILFollowup::getTable(), Sanitizer::sanitize($expected_followup)))->isEqualTo(1); + $this->assertEquals(1, countElementsInTable(ITILFollowup::getTable(), Sanitizer::sanitize($expected_followup))); } } - protected function mailServerProtocolsProvider() + public static function mailServerProtocolsProvider() { return [ [ @@ -1091,12 +1094,12 @@ public function testGetMailServerProtocols( ) { $type = \Toolbox::parseMailServerConnectString($cnx_string)['type']; - $this->string($type)->isEqualTo($expected_type); + $this->assertEquals($expected_type, $type); if ($expected_protocol !== null) { - $this->object(\Toolbox::getMailServerProtocolInstance($type))->isInstanceOf($expected_protocol); + $this->assertInstanceOf($expected_protocol, \Toolbox::getMailServerProtocolInstance($type)); } else { - $this->variable(\Toolbox::getMailServerProtocolInstance($type))->isNull(); + $this->assertNull(\Toolbox::getMailServerProtocolInstance($type)); } $params = [ @@ -1105,14 +1108,14 @@ public function testGetMailServerProtocols( 'password' => 'applesauce', ]; if ($expected_storage !== null) { - $this->object(\Toolbox::getMailServerStorageInstance($type, $params))->isInstanceOf($expected_storage); + $this->assertInstanceOf($expected_storage, \Toolbox::getMailServerStorageInstance($type, $params)); } else { - $this->variable(\Toolbox::getMailServerStorageInstance($type, $params))->isNull(); + $this->assertNull(\Toolbox::getMailServerStorageInstance($type, $params)); } } - protected function mailServerProtocolsHookProvider() + public static function mailServerProtocolsHookProvider() { // Create valid classes eval(<<<CLASS @@ -1129,7 +1132,7 @@ public function close() {} ); return [ - // Check that invalid hook result does not alter core protocols specs + // Check that invalid hook result does not alter core protocols specs [ 'hook_result' => 'invalid result', 'type' => 'imap', @@ -1137,7 +1140,7 @@ public function close() {} 'expected_protocol' => 'Laminas\Mail\Protocol\Imap', 'expected_storage' => 'Laminas\Mail\Storage\Imap', ], - // Check that hook cannot alter core protocols specs + // Check that hook cannot alter core protocols specs [ 'hook_result' => [ 'imap' => [ @@ -1151,7 +1154,7 @@ public function close() {} 'expected_protocol' => 'Laminas\Mail\Protocol\Imap', 'expected_storage' => 'Laminas\Mail\Storage\Imap', ], - // Check that hook cannot alter core protocols specs + // Check that hook cannot alter core protocols specs [ 'hook_result' => [ 'pop' => [ @@ -1165,7 +1168,7 @@ public function close() {} 'expected_protocol' => 'Laminas\Mail\Protocol\Pop3', 'expected_storage' => 'Laminas\Mail\Storage\Pop3', ], - // Check that class must exists + // Check that class must exist [ 'hook_result' => [ 'custom-protocol' => [ @@ -1179,7 +1182,7 @@ public function close() {} 'expected_protocol' => null, 'expected_storage' => null, ], - // Check that class must implements expected functions + // Check that class must implement expected functions [ 'hook_result' => [ 'custom-protocol' => [ @@ -1193,7 +1196,7 @@ public function close() {} 'expected_protocol' => null, 'expected_storage' => null, ], - // Check valid case using class names + // Check valid case using class names [ 'hook_result' => [ 'custom-protocol' => [ @@ -1207,7 +1210,7 @@ public function close() {} 'expected_protocol' => 'PluginTesterFakeProtocol', 'expected_storage' => 'PluginTesterFakeStorage', ], - // Check valid case using callback + // Check valid case using callback [ 'hook_result' => [ 'custom-protocol' => [ @@ -1246,22 +1249,21 @@ public function testGetAdditionnalMailServerProtocols( return $hook_result; }; - // Get protocol + // Get protocol $protocol = null; $getProtocol = function () use ($type, &$protocol) { $protocol = \Toolbox::getMailServerProtocolInstance($type); }; + + $getProtocol(); if ($expected_warning !== null) { - $this->when($getProtocol) - ->error() - ->withType(E_USER_WARNING) - ->withMessage($expected_warning) - ->exists(); - } else { - $getProtocol(); + $this->hasPhpLogRecordThatContains( + $expected_warning, + LogLevel::WARNING + ); } - // Get storage + // Get storage $storage = null; $getStorage = function () use ($type, &$storage) { $params = [ @@ -1271,28 +1273,26 @@ public function testGetAdditionnalMailServerProtocols( ]; $storage = \Toolbox::getMailServerStorageInstance($type, $params); }; + $getStorage(); if ($expected_warning !== null) { - $this->when($getStorage) - ->error() - ->withType(E_USER_WARNING) - ->withMessage($expected_warning) - ->exists(); - } else { - $getStorage(); + $this->hasPhpLogRecordThatContains( + $expected_warning, + LogLevel::WARNING + ); } $PLUGIN_HOOKS = $hooks_backup; if ($expected_protocol !== null) { - $this->object($protocol)->isInstanceOf($expected_protocol); + $this->assertInstanceOf($expected_protocol, $protocol); } else { - $this->variable($protocol)->isNull(); + $this->assertNull($protocol); } if ($expected_storage !== null) { - $this->object($storage)->isInstanceOf($expected_storage); + $this->assertInstanceOf($expected_storage, $storage); } else { - $this->variable($storage)->isNull(); + $this->assertNull($storage); } } } diff --git a/tests/README.md b/tests/README.md index 85954dc70f7..30a92b40540 100644 --- a/tests/README.md +++ b/tests/README.md @@ -49,8 +49,8 @@ Running the test suite on developpement env ------------------------------------------- There are multiple directories for tests: -- `tests/functional` for unit and functional tests; -- `tests/imap` for Mail collector tests; +- `tests/functional` and `tests/phpunit` for unit and functional tests; +- `phpunit/imap` for Mail collector tests; - `tests/LDAP` for LDAP connection tests; - `tests/web` for API tests. From 16a68c376b2029b80d04ecacb5edbfb22f6d4449 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski <johan@x-tnd.be> Date: Wed, 27 Nov 2024 12:30:49 +0100 Subject: [PATCH 02/10] Run imap tests early --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf1727e9596..85d34b9370a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,6 +194,11 @@ jobs: run: | .github/actions/init_initialize-9.5-db.sh docker compose exec -T app .github/actions/test_update-from-9.5.sh + - name: "IMAP tests" + if: "${{ success() || failure() }}" + run: | + .github/actions/init_initialize-imap-fixtures.sh + docker compose exec -T app .github/actions/test_tests-imap.sh - name: "PHPUnit tests" if: "${{ success() || failure() }}" run: | @@ -211,11 +216,6 @@ jobs: run: | .github/actions/init_initialize-ldap-fixtures.sh docker compose exec -T app .github/actions/test_tests-ldap.sh - - name: "IMAP tests" - if: "${{ success() || failure() }}" - run: | - .github/actions/init_initialize-imap-fixtures.sh - docker compose exec -T app .github/actions/test_tests-imap.sh - name: "WEB tests" if: "${{ success() || failure() }}" run: | From fee061326b123b9d01b90f3b6ec9022bf535a4d2 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski <johan@x-tnd.be> Date: Wed, 27 Nov 2024 12:46:15 +0100 Subject: [PATCH 03/10] Try to fix --- phpunit/imap/MailCollectorTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpunit/imap/MailCollectorTest.php b/phpunit/imap/MailCollectorTest.php index ace2eed356b..739d1ee7359 100644 --- a/phpunit/imap/MailCollectorTest.php +++ b/phpunit/imap/MailCollectorTest.php @@ -1256,12 +1256,12 @@ public function testGetAdditionnalMailServerProtocols( }; $getProtocol(); - if ($expected_warning !== null) { + /*if ($expected_warning !== null) { $this->hasPhpLogRecordThatContains( $expected_warning, LogLevel::WARNING ); - } + }*/ // Get storage $storage = null; From eb5fab5dd266bad235c1256d6a5b41a42f3b15f5 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski <johan@x-tnd.be> Date: Wed, 27 Nov 2024 13:15:22 +0100 Subject: [PATCH 04/10] Fix --- phpunit/imap/MailCollectorTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/phpunit/imap/MailCollectorTest.php b/phpunit/imap/MailCollectorTest.php index 739d1ee7359..10b2dc893d1 100644 --- a/phpunit/imap/MailCollectorTest.php +++ b/phpunit/imap/MailCollectorTest.php @@ -599,8 +599,7 @@ public function testIsResponseToMessageSentByAnotherGlpi( private function doConnect() { if (null === $this->collector) { - $this->newTestedInstance(); - $collector = $this->testedInstance; + $collector = new \MailCollector(); $this->collector = $collector; } else { $collector = $this->collector; From e6fd8ede2ec79f362fbab3ac10ae4ae92c62ee84 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski <johan@x-tnd.be> Date: Wed, 27 Nov 2024 13:41:25 +0100 Subject: [PATCH 05/10] Fixes --- phpunit/imap/MailCollectorTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpunit/imap/MailCollectorTest.php b/phpunit/imap/MailCollectorTest.php index 10b2dc893d1..b8ff29e5e91 100644 --- a/phpunit/imap/MailCollectorTest.php +++ b/phpunit/imap/MailCollectorTest.php @@ -949,7 +949,7 @@ public function testCollect() $this->assertEquals($tickets_contents[$name], Sanitizer::unsanitize($data['content'])); } - $this->assertNotContains('cid:', $data['content']); // check that images were correctly imported + $this->assertStringNotContainsString('cid:', $data['content']); // check that images were correctly imported $names[] = $name; } @@ -1273,12 +1273,12 @@ public function testGetAdditionnalMailServerProtocols( $storage = \Toolbox::getMailServerStorageInstance($type, $params); }; $getStorage(); - if ($expected_warning !== null) { + /*if ($expected_warning !== null) { $this->hasPhpLogRecordThatContains( $expected_warning, LogLevel::WARNING ); - } + }*/ $PLUGIN_HOOKS = $hooks_backup; From 32b4a4ece8419f564eeb4b2f07051dd0da7c069c Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski <johan@x-tnd.be> Date: Wed, 27 Nov 2024 14:01:31 +0100 Subject: [PATCH 06/10] Drop eval --- phpunit/imap/MailCollectorTest.php | 20 +++------ tests/fixtures/plugintesterfakeprotocol.php | 48 +++++++++++++++++++++ tests/fixtures/plugintesterfakestorage.php | 40 +++++++++++++++++ 3 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 tests/fixtures/plugintesterfakeprotocol.php create mode 100644 tests/fixtures/plugintesterfakestorage.php diff --git a/phpunit/imap/MailCollectorTest.php b/phpunit/imap/MailCollectorTest.php index b8ff29e5e91..adf79edd802 100644 --- a/phpunit/imap/MailCollectorTest.php +++ b/phpunit/imap/MailCollectorTest.php @@ -1116,19 +1116,8 @@ public function testGetMailServerProtocols( public static function mailServerProtocolsHookProvider() { - // Create valid classes - eval(<<<CLASS -class PluginTesterFakeProtocol implements Glpi\Mail\Protocol\ProtocolInterface { - public function setNoValidateCert(bool \$novalidatecert) {} - public function connect(\$host, \$port = null, \$ssl = false) {} - public function login(\$user, \$password) {} -} -class PluginTesterFakeStorage extends Laminas\Mail\Storage\Imap { - public function __construct(\$params) {} - public function close() {} -} -CLASS - ); + require_once FIXTURE_DIR . '/plugintesterfakeprotocol.php'; + require_once FIXTURE_DIR . '/plugintesterfakestorage.php'; return [ // Check that invalid hook result does not alter core protocols specs @@ -1242,6 +1231,9 @@ public function testGetAdditionnalMailServerProtocols( ) { global $PLUGIN_HOOKS; + require_once FIXTURE_DIR . '/plugintesterfakeprotocol.php'; + require_once FIXTURE_DIR . '/plugintesterfakestorage.php'; + $hooks_backup = $PLUGIN_HOOKS; $PLUGIN_HOOKS['mail_server_protocols']['tester'] = function () use ($hook_result) { @@ -1249,7 +1241,7 @@ public function testGetAdditionnalMailServerProtocols( }; // Get protocol - $protocol = null; + $protocol = null; $getProtocol = function () use ($type, &$protocol) { $protocol = \Toolbox::getMailServerProtocolInstance($type); }; diff --git a/tests/fixtures/plugintesterfakeprotocol.php b/tests/fixtures/plugintesterfakeprotocol.php new file mode 100644 index 00000000000..148a28b6d17 --- /dev/null +++ b/tests/fixtures/plugintesterfakeprotocol.php @@ -0,0 +1,48 @@ +<?php + +/** +* --------------------------------------------------------------------- +* +* GLPI - Gestionnaire Libre de Parc Informatique +* +* http://glpi-project.org +* +* @copyright 2015-2024 Teclib' and contributors. +* @licence https://www.gnu.org/licenses/gpl-3.0.html +* +* --------------------------------------------------------------------- +* +* LICENSE +* +* This file is part of GLPI. +* +* 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 +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <https://www.gnu.org/licenses/>. +* +* --------------------------------------------------------------------- +*/ + +class PluginTesterFakeProtocol implements Glpi\Mail\Protocol\ProtocolInterface +{ + public function setNoValidateCert(bool $novalidatecert) + { + } + + public function connect($host, $port = null, $ssl = false) + { + } + + public function login($user, $password) + { + } +} \ No newline at end of file diff --git a/tests/fixtures/plugintesterfakestorage.php b/tests/fixtures/plugintesterfakestorage.php new file mode 100644 index 00000000000..2af9375862d --- /dev/null +++ b/tests/fixtures/plugintesterfakestorage.php @@ -0,0 +1,40 @@ +<?php + +/** +* --------------------------------------------------------------------- +* +* GLPI - Gestionnaire Libre de Parc Informatique +* +* http://glpi-project.org +* +* @copyright 2015-2024 Teclib' and contributors. +* @licence https://www.gnu.org/licenses/gpl-3.0.html +* +* --------------------------------------------------------------------- +* +* LICENSE +* +* This file is part of GLPI. +* +* 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 +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see <https://www.gnu.org/licenses/>. +* +* --------------------------------------------------------------------- +*/ + +class PluginTesterFakeStorage extends Laminas\Mail\Storage\Imap { + public function __construct($params) { + } + public function close() { + } +} From eda85faa035eb62c98e515f0bce175ea750cb61a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= <cedric.anne@gmail.com> Date: Wed, 27 Nov 2024 22:05:53 +0100 Subject: [PATCH 07/10] fix plugin loading and class files location --- phpunit/imap/MailCollectorTest.php | 26 ++++++++----------- .../tester/inc/fakeprotocol.class.php} | 0 .../tester/inc/fakestorage.class.php} | 0 3 files changed, 11 insertions(+), 15 deletions(-) rename tests/fixtures/{plugintesterfakeprotocol.php => plugins/tester/inc/fakeprotocol.class.php} (100%) rename tests/fixtures/{plugintesterfakestorage.php => plugins/tester/inc/fakestorage.class.php} (100%) diff --git a/phpunit/imap/MailCollectorTest.php b/phpunit/imap/MailCollectorTest.php index adf79edd802..9936fb2c43d 100644 --- a/phpunit/imap/MailCollectorTest.php +++ b/phpunit/imap/MailCollectorTest.php @@ -1116,9 +1116,6 @@ public function testGetMailServerProtocols( public static function mailServerProtocolsHookProvider() { - require_once FIXTURE_DIR . '/plugintesterfakeprotocol.php'; - require_once FIXTURE_DIR . '/plugintesterfakestorage.php'; - return [ // Check that invalid hook result does not alter core protocols specs [ @@ -1189,14 +1186,14 @@ public static function mailServerProtocolsHookProvider() 'hook_result' => [ 'custom-protocol' => [ 'label' => 'Custom email protocol', - 'protocol' => 'PluginTesterFakeProtocol', - 'storage' => 'PluginTesterFakeStorage', + 'protocol' => \PluginTesterFakeProtocol::class, + 'storage' => \PluginTesterFakeStorage::class, ], ], 'type' => 'custom-protocol', 'expected_warning' => null, - 'expected_protocol' => 'PluginTesterFakeProtocol', - 'expected_storage' => 'PluginTesterFakeStorage', + 'expected_protocol' => \PluginTesterFakeProtocol::class, + 'expected_storage' => \PluginTesterFakeStorage::class, ], // Check valid case using callback [ @@ -1213,8 +1210,8 @@ public static function mailServerProtocolsHookProvider() ], 'type' => 'custom-protocol', 'expected_warning' => null, - 'expected_protocol' => 'PluginTesterFakeProtocol', - 'expected_storage' => 'PluginTesterFakeStorage', + 'expected_protocol' => \PluginTesterFakeProtocol::class, + 'expected_storage' => \PluginTesterFakeStorage::class, ], ]; } @@ -1231,8 +1228,7 @@ public function testGetAdditionnalMailServerProtocols( ) { global $PLUGIN_HOOKS; - require_once FIXTURE_DIR . '/plugintesterfakeprotocol.php'; - require_once FIXTURE_DIR . '/plugintesterfakestorage.php'; + (new \Plugin())->init(true); // The `tester` plugin must be considered as loaded/active. $hooks_backup = $PLUGIN_HOOKS; @@ -1247,12 +1243,12 @@ public function testGetAdditionnalMailServerProtocols( }; $getProtocol(); - /*if ($expected_warning !== null) { + if ($expected_warning !== null) { $this->hasPhpLogRecordThatContains( $expected_warning, LogLevel::WARNING ); - }*/ + } // Get storage $storage = null; @@ -1265,12 +1261,12 @@ public function testGetAdditionnalMailServerProtocols( $storage = \Toolbox::getMailServerStorageInstance($type, $params); }; $getStorage(); - /*if ($expected_warning !== null) { + if ($expected_warning !== null) { $this->hasPhpLogRecordThatContains( $expected_warning, LogLevel::WARNING ); - }*/ + } $PLUGIN_HOOKS = $hooks_backup; diff --git a/tests/fixtures/plugintesterfakeprotocol.php b/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php similarity index 100% rename from tests/fixtures/plugintesterfakeprotocol.php rename to tests/fixtures/plugins/tester/inc/fakeprotocol.class.php diff --git a/tests/fixtures/plugintesterfakestorage.php b/tests/fixtures/plugins/tester/inc/fakestorage.class.php similarity index 100% rename from tests/fixtures/plugintesterfakestorage.php rename to tests/fixtures/plugins/tester/inc/fakestorage.class.php From 26284a6515aa82ba7141e7f2f50c6d3a77d1f71e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= <cedric.anne@gmail.com> Date: Wed, 27 Nov 2024 22:17:16 +0100 Subject: [PATCH 08/10] put back previous test order --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 85d34b9370a..bf1727e9596 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,11 +194,6 @@ jobs: run: | .github/actions/init_initialize-9.5-db.sh docker compose exec -T app .github/actions/test_update-from-9.5.sh - - name: "IMAP tests" - if: "${{ success() || failure() }}" - run: | - .github/actions/init_initialize-imap-fixtures.sh - docker compose exec -T app .github/actions/test_tests-imap.sh - name: "PHPUnit tests" if: "${{ success() || failure() }}" run: | @@ -216,6 +211,11 @@ jobs: run: | .github/actions/init_initialize-ldap-fixtures.sh docker compose exec -T app .github/actions/test_tests-ldap.sh + - name: "IMAP tests" + if: "${{ success() || failure() }}" + run: | + .github/actions/init_initialize-imap-fixtures.sh + docker compose exec -T app .github/actions/test_tests-imap.sh - name: "WEB tests" if: "${{ success() || failure() }}" run: | From 619217d768d7acab0415599a31f8b7c966af66b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= <cedric.anne@gmail.com> Date: Wed, 27 Nov 2024 22:21:07 +0100 Subject: [PATCH 09/10] fix readme --- tests/README.md | 2 +- tests/fixtures/plugins/tester/inc/fakeprotocol.class.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/README.md b/tests/README.md index 30a92b40540..fb99222109d 100644 --- a/tests/README.md +++ b/tests/README.md @@ -49,7 +49,7 @@ Running the test suite on developpement env ------------------------------------------- There are multiple directories for tests: -- `tests/functional` and `tests/phpunit` for unit and functional tests; +- `tests/functional` and `phpunit/functional` for unit and functional tests; - `phpunit/imap` for Mail collector tests; - `tests/LDAP` for LDAP connection tests; - `tests/web` for API tests. diff --git a/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php b/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php index 148a28b6d17..baacef29ac4 100644 --- a/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php +++ b/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php @@ -45,4 +45,4 @@ public function connect($host, $port = null, $ssl = false) public function login($user, $password) { } -} \ No newline at end of file +} From be753a70fc23f982386e312d4167db3deeb15f25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= <cedric.anne@gmail.com> Date: Wed, 27 Nov 2024 22:34:19 +0100 Subject: [PATCH 10/10] lint --- .../plugins/tester/inc/fakeprotocol.class.php | 61 ++++++++-------- .../plugins/tester/inc/fakestorage.class.php | 71 ++++++++++--------- 2 files changed, 69 insertions(+), 63 deletions(-) diff --git a/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php b/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php index baacef29ac4..9681b83876f 100644 --- a/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php +++ b/tests/fixtures/plugins/tester/inc/fakeprotocol.class.php @@ -1,36 +1,37 @@ <?php /** -* --------------------------------------------------------------------- -* -* GLPI - Gestionnaire Libre de Parc Informatique -* -* http://glpi-project.org -* -* @copyright 2015-2024 Teclib' and contributors. -* @licence https://www.gnu.org/licenses/gpl-3.0.html -* -* --------------------------------------------------------------------- -* -* LICENSE -* -* This file is part of GLPI. -* -* 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 -* the Free Software Foundation, either version 3 of the License, or -* (at your option) any later version. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -* You should have received a copy of the GNU General Public License -* along with this program. If not, see <https://www.gnu.org/licenses/>. -* -* --------------------------------------------------------------------- -*/ + * --------------------------------------------------------------------- + * + * GLPI - Gestionnaire Libre de Parc Informatique + * + * http://glpi-project.org + * + * @copyright 2015-2024 Teclib' and contributors. + * @copyright 2003-2014 by the INDEPNET Development Team. + * @licence https://www.gnu.org/licenses/gpl-3.0.html + * + * --------------------------------------------------------------------- + * + * LICENSE + * + * This file is part of GLPI. + * + * 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 + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. + * + * --------------------------------------------------------------------- + */ class PluginTesterFakeProtocol implements Glpi\Mail\Protocol\ProtocolInterface { diff --git a/tests/fixtures/plugins/tester/inc/fakestorage.class.php b/tests/fixtures/plugins/tester/inc/fakestorage.class.php index 2af9375862d..4c97e1433e2 100644 --- a/tests/fixtures/plugins/tester/inc/fakestorage.class.php +++ b/tests/fixtures/plugins/tester/inc/fakestorage.class.php @@ -1,40 +1,45 @@ <?php /** -* --------------------------------------------------------------------- -* -* GLPI - Gestionnaire Libre de Parc Informatique -* -* http://glpi-project.org -* -* @copyright 2015-2024 Teclib' and contributors. -* @licence https://www.gnu.org/licenses/gpl-3.0.html -* -* --------------------------------------------------------------------- -* -* LICENSE -* -* This file is part of GLPI. -* -* 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 -* the Free Software Foundation, either version 3 of the License, or -* (at your option) any later version. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -* GNU General Public License for more details. -* -* You should have received a copy of the GNU General Public License -* along with this program. If not, see <https://www.gnu.org/licenses/>. -* -* --------------------------------------------------------------------- -*/ + * --------------------------------------------------------------------- + * + * GLPI - Gestionnaire Libre de Parc Informatique + * + * http://glpi-project.org + * + * @copyright 2015-2024 Teclib' and contributors. + * @copyright 2003-2014 by the INDEPNET Development Team. + * @licence https://www.gnu.org/licenses/gpl-3.0.html + * + * --------------------------------------------------------------------- + * + * LICENSE + * + * This file is part of GLPI. + * + * 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 + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. + * + * --------------------------------------------------------------------- + */ -class PluginTesterFakeStorage extends Laminas\Mail\Storage\Imap { - public function __construct($params) { +class PluginTesterFakeStorage extends Laminas\Mail\Storage\Imap +{ + public function __construct($params) + { } - public function close() { + + public function close() + { } }