Skip to content

Commit

Permalink
MDL-79637 phpunit: Do not reset DB during init of isolated tests
Browse files Browse the repository at this point in the history
During the bootstrap of PHPUnit we ensure that the database has been
reset to its initial state.

We do this by checking the internally-stored DB write count between
runs. If the count is not yet set (null), or it has been increased, we
force a reset.

When running an isolated test the test runner resets the database, it
then sets up a new isolated test environment by writing a new PHPUnit
test case and passing it to a new PHP Process using standard in. As part
of this, the bootstrap is run for that process.

Because we are in a new process, the db write count is fresh and not yet
set. This has been leading to an additional db reset before the isolated
test.

To handle this we want to _not_ perform a reset during the
initialisation for isolated runs. We know that the DB is in a fresh
state before we start the run.

To support this we need to know whether the test is an isolated test
during the bootstrap, which means we cannot use the previous approach to
calculating this.

Instead we look at the PHP_SELF value. PHP sets this to "Standard input
code" when run from stdin, instead of running a file.

There should not be any other legitimate reason to run a PHPUnit
bootstrap via this stdin approach.

Unfortunately this approach is a little bit risky as it depends on the
presence of a specific string, however this string has been in place
since 2016, and there is no legitimate way of calculating this.

I did consider looking at whether the called script included `/vendor/`
and `/phpunit`, but this is also likely a risky approach if someone
calls PHPUnit in an unexpected way.

This approach is itself unit tested so any change to PHP's stdin string
before we deprecate this approach entirely in 12 months time will be
caught.
  • Loading branch information
andrewnicols committed Oct 11, 2023
1 parent 769c670 commit 99a6cd1
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 15 deletions.
14 changes: 14 additions & 0 deletions lib/phpunit/bootstrap.php
Expand Up @@ -205,6 +205,20 @@
$CFG->themerev = 1;
$CFG->jsrev = 1;

(function () {
// Determine if this test is being run with isolation.
// This is tricky because neither PHPUnit, nor PHP provide an official way to work this out.
// PHPUnit does set a value, but not until later on and we need this earlier.
// PHPUnit runs isolated tests by creating a class on the fly and running it through proc_open as standard input.
// There is no other legitimate reason to run PHPUnit this way that I'm aware of.
// When run in this way, PHP sets the value of $_SERVER['PHP_SELF'] to "Standard input code".
// It has done this since 2016, and it is unlikely to change.
define(
'PHPUNIT_ISOLATED_TEST',
$_SERVER['PHP_SELF'] === 'Standard input code',
);
})();

// load test case stub classes and other stuff
require_once("$CFG->dirroot/lib/phpunit/lib.php");

Expand Down
13 changes: 0 additions & 13 deletions lib/phpunit/classes/advanced_testcase.php
Expand Up @@ -60,19 +60,6 @@ final public function __construct($name = null, array $data = array(), $dataName

}

/**
* 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);
}
}

/**
* Runs the bare test sequence.
* @return void
Expand Down
8 changes: 7 additions & 1 deletion lib/phpunit/classes/util.php
Expand Up @@ -320,7 +320,13 @@ public static function reset_all_data($detectchanges = false) {
public static function reset_database() {
global $DB;

if (!is_null(self::$lastdbwrites) and self::$lastdbwrites == $DB->perf_get_writes()) {
if (defined('PHPUNIT_ISOLATED_TEST') && PHPUNIT_ISOLATED_TEST && self::$lastdbwrites === null) {
// This is an isolated test and the lastdbwrites has not yet been initialised.
// Isolated test runs are reset by the test runner before the run starts.
self::$lastdbwrites = $DB->perf_get_writes();
}

if (!is_null(self::$lastdbwrites) && self::$lastdbwrites == $DB->perf_get_writes()) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/setuplib.php
Expand Up @@ -2202,7 +2202,7 @@ function require_phpunit_isolation(): void {
return;
}

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

0 comments on commit 99a6cd1

Please sign in to comment.