(REF) FileTest - Prevent test-interactions from testIsDirWithOpenBasedir

[totten]

Overview

This addresses a test-leakage issue involving testIsDirWithOpenBasedir. This is a follow-up/replacement for #24307. The previous mitigation allowed PR tests to continue, but we still execute the test in the matrix - so the matrix is still vulnerable to the test-leakage (eg https://test.civicrm.org/job/CiviCRM-Core-Matrix/11473/BKPROF=min,CIVIVER=5.53,SUITES=phpunit-crm,label=bknix-tmp/console).

This makes the test safer.

ping @demeritcowboy @seamuslee001

Steps to Reproduce

To see the test-leakage issue, you need to run CRM_Utils_FileTest::testIsDirWithOpenBasedir in combination with some other failing test. To try it out, I've coerced a facsimile of the 11473 failure:

diff --git a/tests/phpunit/CRM/Utils/HttpClientTest.php b/tests/phpunit/CRM/Utils/HttpClientTest.php
index 599bc6d153..a6593eea8e 100644
--- a/tests/phpunit/CRM/Utils/HttpClientTest.php
+++ b/tests/phpunit/CRM/Utils/HttpClientTest.php
@@ -50,6 +50,7 @@ class CRM_Utils_HttpClientTest extends CiviUnitTestCase {
   }

   public function testFetchHttps_valid() {
+    $this->assertEquals('no', 'not');
     $result = $this->client->fetch(self::VALID_HTTPS_URL, $this->tmpFile);
     $this->assertEquals(CRM_Utils_HttpClient::STATUS_OK, $result);
     $this->assertRegExp(self::VALID_HTTPS_REGEX, file_get_contents($this->tmpFile));

And then run these together:

rm /tmp/ft -rf
mkdir /tmp/ft
./tools/scripts/phpunit CRM_AllTests --filter '(testFetchHttps_valid|testIsDirWithOpenBasedir)' --log-junit /tmp/ft/foo.xml --tap

The output from PHPUnit should not be affected by open_basedir issues.

Before

When testIsDirWithOpenBasedir runs, it changes the open_basedir, which cannot be switched back. But phpunit may still need to do work (like writing JUnit files) -- and open_basedir interferes with PHPUnit.

After

When testIsDirWithOpenBasedir runs, it starts a subprocess and changes open_basedir -- but only in the subprocess. The main phpunit process retains its original file-access privileges.

Technical Details

How do we know this refactoring still leaves a useful test? Well, based on the comments in testIsDirWithOpenBasedir(), you would expect is_dir() to behave badly -- and CRM_Utils_File::isDir() to behave nicely. So I edited the test to switch the focus of the test, eg

-    $actual = CRM_Utils_File::isDir($input); /* Should pass */
+    $actual = is_dir($input); /* Should fail */

The updated test still discerns between these two (ie is_dir() fails the test, and CRM_Utils_File::isDir() passes the test).

Comments

I targeted this PR at 5.53 (5.53.beta) because the issue hinders the readability of matrix results on 5.53. After merging this, we'll need to undo the @group ornery that was applied to master (5.54.alpha).

This is not pretty. I'm sure there could be other/prettier approaches. For now I just need some way to get a reliable matrix.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

I think this is ok - interesting approach. I tried to look what php itself does when it runs these tests which is what this set of tests was based on. It looks like they run all of their test files separately like proc_open('php ... testfile.php'), so kind of the same idea.

In terms of flipping the focus, that's ok. The reason that if block was split into if (TRUE) { assert something } else { assert opposite } was just to (try to) visually separate that that is why CRM_Utils_File::isDir exists in the first place. Because in that part of the test they do do slightly different things.