Skip to content

Commit

Permalink
MDL-77989 testing: Add test file isolation helper
Browse files Browse the repository at this point in the history
When we deprecate the use of a file, we often include tests which ensure
that the legacy behaviour is maintained. There are also legacy uses
in the community where people would like to use the deprecated API for a
period.

The issue that we face is that, if the deprecated file is included once,
then it will be included for all other, unrelated, tests. This means
that other tests may not detect cases where the deprecated file was
included.

We can solve these cases by running the test that performs the inclusion
in a deprecated process. This means that the inclusion is only performed
in that isolated process, and other unrelated tests do not include the
file.

However, we also then need to detect which files which are including the
file and which we do not know about.

This change introduces:
- an override to the TestCase::setInIsolation method to define a
  constant when the test is running in isolation
- a new function that a file can call when it is included, to make sure
  that the test process was isolated, where there is any test.
  • Loading branch information
andrewnicols committed Apr 21, 2023
1 parent f7d7ad7 commit 1a53cbb
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/externallib.php
Expand Up @@ -24,10 +24,16 @@

use core_external\util;

defined('MOODLE_INTERNAL') || die;

// Please note that this file and all of the classes and functions listed below will be deprecated from Moodle 4.6.
// This deprecation is delayed to aid plugin developers when maintaining plugins for multiple Moodle versions.
// See MDL-76583 for further information.

// If including this file for unit testing, it _must_ be run in an isolated process to prevent
// any side effect upon other tests.
require_phpunit_isolation();

class_alias(\core_external\external_api::class, 'external_api');
class_alias(\core_external\restricted_context_exception::class, 'restricted_context_exception');
class_alias(\core_external\external_description::class, 'external_description');
Expand Down
14 changes: 14 additions & 0 deletions lib/phpunit/classes/advanced_testcase.php
Expand Up @@ -57,6 +57,20 @@ final public function __construct($name = null, array $data = array(), $dataName
$this->setBackupGlobals(false);
$this->setBackupStaticAttributes(false);
$this->setPreserveGlobalState(false);

}

/**
* Hook into the setInIsolation method to define an optional constant.
*
* @param bool $inisolation
*/
public function setInIsolation(bool $inisolation): void {
parent::setInIsolation($inisolation);
if ($inisolation) {
// Note: This is safe to do because it will only be set once per test run.
define('PHPUNIT_ISOLATED_TEST', true);
}
}

/**
Expand Down
23 changes: 23 additions & 0 deletions lib/setuplib.php
Expand Up @@ -2183,3 +2183,26 @@ function proxy_log_callback($code) {
error_log($error . format_backtrace($trace, true)); // phpcs:ignore
}
}

/**
* A helper function for deprecated files to use to ensure that, when they are included for unit tests,
* they are run in an isolated process.
*
* @throws \coding_exception The exception thrown when the process is not isolated.
*/
function require_phpunit_isolation(): void {
if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
// Not a test.
return;
}

if (defined('PHPUNIT_ISOLATED_TEST')) {
// Already isolated.
return;
}

throw new \coding_exception(
'When including this file for a unit test, the test must be run in an isolated process. ' .
'See the PHPUnit @runInSeparateProcess and @runTestsInSeparateProcesses annotations.'
);
}
22 changes: 22 additions & 0 deletions lib/tests/setuplib_test.php
Expand Up @@ -520,4 +520,26 @@ public function test_core_uuid_generate() {
$uuid = \core\uuid::generate();
$this->assertTrue(self::is_valid_uuid_v4($uuid), "Invalid v4 UUID: '$uuid'");
}

/**
* Test require_phpunit_isolation in a test which is not isolated.
*
* @covers ::require_phpunit_isolation
*/
public function test_require_phpunit_isolation(): void {
// A unit test which is not isolated will throw a coding_exception when the function is called.
$this->expectException('coding_exception');
require_phpunit_isolation();
}

/**
* Test require_phpunit_isolation in a test which is isolated.
*
* @covers ::require_phpunit_isolation
* @runInSeparateProcess
*/
public function test_require_phpunit_isolation_isolated(): void {
$this->expectNotToPerformAssertions();
require_phpunit_isolation();
}
}

0 comments on commit 1a53cbb

Please sign in to comment.