From 2c8acd3462423711e6efd7c5c73320b08f010e60 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 7 Feb 2024 21:04:57 +0800 Subject: [PATCH 1/6] MDL-66903 testing: Reset CFG and component after test This change moves the reset of global test state to the finally section rather than doing it only if the test passes. Previously if a test which modifies the `core_component` internals failed, it would not reset the internal state and impact subsequent tests. --- lib/phpunit/classes/advanced_testcase.php | 24 +++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index 2068ce650fd..000e07a4c31 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -57,7 +57,7 @@ abstract class advanced_testcase extends base_testcase { * Runs the bare test sequence. */ final public function runBare(): void { - global $DB; + global $CFG, $DB; if (phpunit_util::$lastdbwrites != $DB->perf_get_writes()) { // This happens when previous test does not reset, we can not use transactions. @@ -70,20 +70,17 @@ abstract class advanced_testcase extends base_testcase { try { $this->setCurrentTimeStart(); parent::runBare(); - // Set DB reference in case somebody mocked it in test. - $DB = phpunit_util::get_global_backup('DB'); - - // Deal with any debugging messages. - $debugerror = phpunit_util::display_debugging_messages(true); - $this->resetDebugging(); - if (!empty($debugerror)) { - trigger_error('Unexpected debugging() call detected.' . "\n" . $debugerror, E_USER_NOTICE); - } } catch (Exception $ex) { $e = $ex; } catch (Throwable $ex) { // Engine errors in PHP7 throw exceptions of type Throwable (this "catch" will be ignored in PHP5). $e = $ex; + } finally { + // Reset global state after test and test failure. + $CFG = phpunit_util::get_global_backup('CFG'); + $DB = phpunit_util::get_global_backup('DB'); + // This is _hacky_. We need to reset the autoloader, and this is the only way to do so right now. + (new ReflectionProperty(\core_component::class, 'plugintypes'))->setValue(null, null); } if (isset($e)) { @@ -92,6 +89,13 @@ abstract class advanced_testcase extends base_testcase { throw $e; } + // Deal with any debugging messages. + $debugerror = phpunit_util::display_debugging_messages(true); + $this->resetDebugging(); + if (!empty($debugerror)) { + trigger_error('Unexpected debugging() call detected.' . "\n" . $debugerror, E_USER_NOTICE); + } + if (!$this->testdbtransaction || $this->testdbtransaction->is_disposed()) { $this->testdbtransaction = null; } From 3c03e9448c0ca185e61e523e959ee79834af89d4 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 7 Feb 2024 21:07:05 +0800 Subject: [PATCH 2/6] MDL-66903 testing: Fix whitespace --- lib/tests/component_test.php | 216 +++++++++++++++++------------------ 1 file changed, 108 insertions(+), 108 deletions(-) diff --git a/lib/tests/component_test.php b/lib/tests/component_test.php index ebc70af8e7d..98ca138454d 100644 --- a/lib/tests/component_test.php +++ b/lib/tests/component_test.php @@ -730,54 +730,54 @@ final class component_test extends advanced_testcase { 'overlap' => 'lib/tests/fixtures/component/overlap', ]; return [ - 'PSR-0 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_main', - 'includedfiles' => "{$directory}psr0/main.php", - ], - 'PSR-0 Classloading - Sub namespace - underscores' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_subnamespace_example', - 'includedfiles' => "{$directory}psr0/subnamespace/example.php", - ], - 'PSR-0 Classloading - Sub namespace - slashes' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0\\subnamespace\\slashes', - 'includedfiles' => "{$directory}psr0/subnamespace/slashes.php", - ], - 'PSR-4 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\main', - 'includedfiles' => "{$directory}psr4/main.php", - ], - 'PSR-4 Classloading - Sub namespace' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\example', - 'includedfiles' => "{$directory}psr4/subnamespace/example.php", - ], - 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\underscore_example', - 'includedfiles' => "{$directory}psr4/subnamespace/underscore_example.php", - ], - 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap\\subnamespace\\example', - 'includedfiles' => "{$directory}overlap/subnamespace/example.php", - ], - 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap_subnamespace_example2', - 'includedfiles' => "{$directory}overlap/subnamespace/example2.php", - ], + 'PSR-0 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_main', + 'includedfiles' => "{$directory}psr0/main.php", + ], + 'PSR-0 Classloading - Sub namespace - underscores' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_subnamespace_example', + 'includedfiles' => "{$directory}psr0/subnamespace/example.php", + ], + 'PSR-0 Classloading - Sub namespace - slashes' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0\\subnamespace\\slashes', + 'includedfiles' => "{$directory}psr0/subnamespace/slashes.php", + ], + 'PSR-4 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\main', + 'includedfiles' => "{$directory}psr4/main.php", + ], + 'PSR-4 Classloading - Sub namespace' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\example', + 'includedfiles' => "{$directory}psr4/subnamespace/example.php", + ], + 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\underscore_example', + 'includedfiles' => "{$directory}psr4/subnamespace/underscore_example.php", + ], + 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap\\subnamespace\\example', + 'includedfiles' => "{$directory}overlap/subnamespace/example.php", + ], + 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap_subnamespace_example2', + 'includedfiles' => "{$directory}overlap/subnamespace/example2.php", + ], ]; } @@ -829,66 +829,66 @@ final class component_test extends advanced_testcase { 'overlap' => 'lib/tests/fixtures/component/overlap', ]; return [ - 'PSR-0 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_main', - 'file' => "{$directory}psr0/main.php", - ], - 'PSR-0 Classloading - Sub namespace - underscores' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_subnamespace_example', - 'file' => "{$directory}psr0/subnamespace/example.php", - ], - 'PSR-0 Classloading - Sub namespace - slashes' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0\\subnamespace\\slashes', - 'file' => "{$directory}psr0/subnamespace/slashes.php", - ], - 'PSR-0 Classloading - non-existent file' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr0_subnamespace_nonexistent_file', - 'file' => false, - ], - 'PSR-4 Classloading - Root' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\main', - 'file' => "{$directory}psr4/main.php", - ], - 'PSR-4 Classloading - Sub namespace' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\example', - 'file' => "{$directory}psr4/subnamespace/example.php", - ], - 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\underscore_example', - 'file' => "{$directory}psr4/subnamespace/underscore_example.php", - ], - 'PSR-4 Classloading - non-existent file' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'psr4\\subnamespace\\nonexistent', - 'file' => false, - ], - 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap\\subnamespace\\example', - 'file' => "{$directory}overlap/subnamespace/example.php", - ], - 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ - 'psr0' => $psr0, - 'psr4' => $psr4, - 'classname' => 'overlap_subnamespace_example2', - 'file' => "{$directory}overlap/subnamespace/example2.php", - ], + 'PSR-0 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_main', + 'file' => "{$directory}psr0/main.php", + ], + 'PSR-0 Classloading - Sub namespace - underscores' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_subnamespace_example', + 'file' => "{$directory}psr0/subnamespace/example.php", + ], + 'PSR-0 Classloading - Sub namespace - slashes' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0\\subnamespace\\slashes', + 'file' => "{$directory}psr0/subnamespace/slashes.php", + ], + 'PSR-0 Classloading - non-existent file' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr0_subnamespace_nonexistent_file', + 'file' => false, + ], + 'PSR-4 Classloading - Root' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\main', + 'file' => "{$directory}psr4/main.php", + ], + 'PSR-4 Classloading - Sub namespace' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\example', + 'file' => "{$directory}psr4/subnamespace/example.php", + ], + 'PSR-4 Classloading - Ensure underscores are not converted to paths' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\underscore_example', + 'file' => "{$directory}psr4/subnamespace/underscore_example.php", + ], + 'PSR-4 Classloading - non-existent file' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'psr4\\subnamespace\\nonexistent', + 'file' => false, + ], + 'Overlap - Ensure no unexpected problems with PSR-4 when overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap\\subnamespace\\example', + 'file' => "{$directory}overlap/subnamespace/example.php", + ], + 'Overlap - Ensure no unexpected problems with PSR-0 overlapping namespaces.' => [ + 'psr0' => $psr0, + 'psr4' => $psr4, + 'classname' => 'overlap_subnamespace_example2', + 'file' => "{$directory}overlap/subnamespace/example2.php", + ], 'PSR-4 namespaces can come from multiple sources - first source' => [ 'psr0' => $psr0, 'psr4' => [ From 406dcd25668041e44374b038306562e94b1847c6 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 19 Dec 2023 22:59:15 +0800 Subject: [PATCH 3/6] MDL-66903 testing: Add support for a \tests\ namespace during tests This commit: - introduces a \tests\ sub-namespace for use in unit tests only - the path to this the tests/classes directory of the owning parent - files here are excluded from unit test runs This is agreed per policy in MDL-80855. --- .upgradenotes/MDL-66903-2024070502035383.yml | 10 +++ lib/classes/component.php | 32 +++++++++ lib/phpunit/classes/util.php | 2 + lib/tests/component_test.php | 63 ++++++++++++++++++ phpunit.xml.dist | 70 ++++++++++++++++++++ 5 files changed, 177 insertions(+) create mode 100644 .upgradenotes/MDL-66903-2024070502035383.yml diff --git a/.upgradenotes/MDL-66903-2024070502035383.yml b/.upgradenotes/MDL-66903-2024070502035383.yml new file mode 100644 index 00000000000..255dd885601 --- /dev/null +++ b/.upgradenotes/MDL-66903-2024070502035383.yml @@ -0,0 +1,10 @@ +issueNumber: MDL-66903 +notes: + core: + - message: > + Added the ability for unit tests to autoload classes in the + `\[component]\tests\` + + namespace from the `[path/to/component]/tests/classes` directory. + type: improved + diff --git a/lib/classes/component.php b/lib/classes/component.php index 0c6063c9eca..a33117f09f9 100644 --- a/lib/classes/component.php +++ b/lib/classes/component.php @@ -172,6 +172,38 @@ class core_component { require($file); return; } + + if (PHPUNIT_TEST) { + // For unit tests we support classes in `\frankenstyle_component\tests\` to be loaded from + // `path/to/frankenstyle/component/tests/classes` directory. + // Note: We do *not* support the legacy `\frankenstyle_component_tests_style_classnames`. + if ($component = self::get_component_from_classname($classname)) { + $pathoptions = [ + '/tests/classes' => "{$component}\\tests\\", + '/tests/behat' => "{$component}\\behat\\", + ]; + foreach ($pathoptions as $path => $testnamespace) { + if (preg_match("#^" . preg_quote($testnamespace) . "#", $classname)) { + $path = self::get_component_directory($component) . $path; + $relativeclassname = str_replace( + $testnamespace, + '', + $classname, + ); + $file = sprintf( + "%s/%s.php", + $path, + str_replace('\\', '/', $relativeclassname), + ); + if (!empty($file) && file_exists($file)) { + require($file); + return; + } + break; + } + } + } + } } /** diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index 0e0bb2d1cb6..1ad2fde17a6 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -529,6 +529,7 @@ class phpunit_util extends testing_util { $template = << @dir@ + @dir@/classes EOF; @@ -621,6 +622,7 @@ class phpunit_util extends testing_util { . + ./classes EOT; diff --git a/lib/tests/component_test.php b/lib/tests/component_test.php index 98ca138454d..894a3e05227 100644 --- a/lib/tests/component_test.php +++ b/lib/tests/component_test.php @@ -914,6 +914,69 @@ final class component_test extends advanced_testcase { ]; } + /** + * Test that the classloader can load from the test namespaces. + */ + public function test_classloader_tests_namespace(): void { + global $CFG; + + $this->resetAfterTest(); + + $getclassfilecontent = function (string $classname, ?string $namespace): string { + if ($namespace) { + $content = " [ + 'classes' => [ + 'example.php' => $getclassfilecontent('example', 'core'), + ], + 'tests' => [ + 'classes' => [ + 'example_classname.php' => $getclassfilecontent('example_classname', \core\tests::class), + ], + 'behat' => [ + 'example_classname.php' => $getclassfilecontent('example_classname', \core\behat::class), + ], + ], + ], + ]); + + // Note: This is pretty hacky, but it's the only way to test the classloader. + // We have to override the dirroot and libdir, and then reset the plugintypes property. + $CFG->dirroot = $vfileroot->url(); + $CFG->libdir = $vfileroot->url() . '/lib'; + (new ReflectionProperty('core_component', 'plugintypes'))->setValue(null, null); + + // Existing classes do not break. + $this->assertTrue( + class_exists(\core\example::class), + ); + + // Test and behat classes work. + $this->assertTrue( + class_exists(\core\tests\example_classname::class), + ); + $this->assertTrue( + class_exists(\core\behat\example_classname::class), + ); + + // Non-existent classes do not do anything. + $this->assertFalse( + class_exists(\core\tests\example_classname_not_found::class), + ); + } + + public function tearDown(): void { + $plugintypes = new ReflectionProperty('core_component', 'plugintypes'); + $plugintypes->setValue(null, null); + } + /** * Test the PSR classloader. * diff --git a/phpunit.xml.dist b/phpunit.xml.dist index fd7faeecc08..de77041fcf2 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -35,54 +35,74 @@ lib/phpunit/tests + lib/phpunit/tests/classes lib/testing/tests + lib/testing/tests/classes lib/ddl/tests + lib/ddl/tests/classes lib/dml/tests + lib/dml/tests/classes lib/tests + lib/tests/classes lib/external/tests + lib/external/tests/classes favourites/tests + favourites/tests/classes lib/form/tests + lib/form/tests/classes lib/filestorage/tests lib/filebrowser/tests files/tests + lib/filestorage/tests/classes + lib/filebrowser/tests/classes + files/tests/classes filter/tests + filter/tests/classes admin/roles/tests + admin/roles/tests/classes cohort/tests + cohort/tests/classes lib/grade/tests grade/tests grade/grading/tests grade/import/csv/tests + lib/grade/tests/classes + grade/tests/classes + grade/grading/tests/classes + grade/import/csv/tests/classes analytics/tests + analytics/tests/classes availability/tests + availability/tests/classes backup/controller/tests @@ -90,135 +110,185 @@ backup/moodle2/tests backup/tests backup/util + backup/controller/tests/classes + backup/converter/moodle1/tests/classes + backup/moodle2/tests/classes + backup/tests/classes + backup/util/classes badges/tests + badges/tests/classes blog/tests + blog/tests/classes customfield/tests + customfield/tests/classes iplookup/tests + iplookup/tests/classes course/tests + course/tests/classes course/format/tests + course/format/tests/classes privacy/tests + privacy/tests/classes question/engine/tests question/tests question/type/tests question/engine/upgrade/tests + question/engine/tests/classes + question/tests/classes + question/type/tests/classes + question/engine/upgrade/tests/classes cache/tests + cache/tests/classes calendar/tests + calendar/tests/classes enrol/tests + enrol/tests/classes group/tests + group/tests/classes message/tests + message/tests/classes notes/tests + notes/tests/classes tag/tests + tag/tests/classes rating/tests + rating/tests/classes repository/tests + repository/tests/classes lib/userkey/tests + lib/userkey/tests/classes user/tests + user/tests/classes webservice/tests + webservice/tests/classes mnet/tests + mnet/tests/classes completion/tests + completion/tests/classes comment/tests + comment/tests/classes search/tests + search/tests/classes competency/tests + competency/tests/classes my/tests + my/tests/classes auth/tests + auth/tests/classes blocks/tests + blocks/tests/classes login/tests + login/tests/classes plagiarism/tests + plagiarism/tests/classes portfolio/tests + portfolio/tests/classes lib/editor/tests + lib/editor/tests/classes rss/tests + rss/tests/classes lib/table/tests + lib/table/tests/classes h5p/tests + h5p/tests/classes lib/xapi/tests + lib/xapi/tests/classes contentbank/tests + contentbank/tests/classes payment/tests + payment/tests/classes reportbuilder/tests + reportbuilder/tests/classes admin/presets/tests + admin/presets/tests/classes admin/tests + admin/tests/classes communication/tests + communication/tests/classes