Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.phpunit.result.cache file is all over the place #3587

Closed
constup opened this issue Apr 5, 2019 · 19 comments
Closed

.phpunit.result.cache file is all over the place #3587

constup opened this issue Apr 5, 2019 · 19 comments
Assignees
Labels
feature/test-runner CLI test runner type/bug Something is broken

Comments

@constup
Copy link

constup commented Apr 5, 2019

Q A
PHPUnit version 8,1,0
PHP version 7.2.8
Installation Method PHAR

Cache is enabled by default, which is not a bad thing. However, by running unit tests, .phpunit.result.cache file is being generated in each folder where the test is run. When running tests from the root of the project - that's not a big issue, since one file is generated in the root directory of the project. However, IDEs have integrations for unit tests where you can run a single test method or a class "on click". That translates to having multiple .phpunit.result.cache files all over the directory structure - in each directory where a test is run.

The solution is obvious - have a single location where the cache file is placed by default and prevent cache clutter in project's directory structure.

@sebastianbergmann
Copy link
Owner

It should only be in the root directory (where phpunit.xml is). Right, @epdenouden?

@epdenouden
Copy link
Contributor

epdenouden commented Apr 5, 2019

@constup Thanks for your report. I will look for ways to tighten up the cachefile placement.

@sebastianbergmann Yes, it was designed that way. However I have also seen it ending up in the 'root' of end-to-end test directories and depending on the IDE/wrapper script the mechanism might see a different root. It still happens, though:

Feb 25 16:16 ./tests/unit/Util/TestDox/.phpunit.result.cache
Mar  5 18:54 ./tests/unit/Framework/.phpunit.result.cache
Mar  3 10:39 ./tests/basic/unit/.phpunit.result.cache
Jan 26 17:06 ./tests/_files/.phpunit.result.cache
Apr  2 11:45 ./.phpunit.result.cache

I will think more about this coming days. Basically the cache is to speed up when testing the same collection over and over again, like I do a lot for PHPUnit. When using the ⏯ buttons in PHPStorm for example, caching this result makes no sense. In addition to the IDE having its own "run failed tests again/first" option.

@sebastianbergmann if you have any wishes or suggestions to be included for this let me know.

The solution is obvious - have a single location where the cache file is placed by default and prevent cache clutter in project's directory structure.

This (already) works when there's a single location to see as root. It gets hard when people use custom configurations with -c, --no-configuration or a different location for phpunit.xml.

@epdenouden
Copy link
Contributor

The current implementation is still the MVP, basically:

if (!Filesystem::createDirectory(\dirname($this->cacheFilename))) {

Unless the user sets a fixed path in the config, the .phpunit.result.cache will end up in the directory phpunit is run from. I'll make some improvements. :)

@sebastianbergmann sebastianbergmann added the feature/test-runner CLI test runner label Apr 22, 2019
@sebastianbergmann
Copy link
Owner

To be honest,

IDEs have integrations for unit tests where you can run a single test method or a class "on click". That translates to having multiple .phpunit.result.cache files all over the directory structure - in each directory where a test is run

suggests to me that IDEs are running single tests wrong. This does not mean, though, that we should not make sure that .phpunit.result.cache is always located in the same directory where phpunit.xml is.

@sebastianbergmann
Copy link
Owner

@epdenouden Thanks! I merged this to 7.5 but am stuck merging it from there to 8.1 due to a merge conflict. Can you send a pull request for 8.1?

@epdenouden
Copy link
Contributor

Will do!

@sebastianbergmann
Copy link
Owner

I managed to merge it without conflict from 7.5 to 8.1:

diff --git a/src/Runner/DefaultTestResultCache.php b/src/Runner/DefaultTestResultCache.php
index 1385849c4..0ff80fc55 100644
--- a/src/Runner/DefaultTestResultCache.php
+++ b/src/Runner/DefaultTestResultCache.php
@@ -66,9 +66,14 @@ final class DefaultTestResultCache implements \Serializable, TestResultCache
      */
     private $times = [];
 
-    public function __construct($filename = null)
+    public function __construct($filepath = null)
     {
-        $this->cacheFilename = $filename ?? $_ENV['PHPUNIT_RESULT_CACHE'] ?? self::DEFAULT_RESULT_CACHE_FILENAME;
+        if ($filepath !== null && \is_dir($filepath)) {
+            // cache path provided, use default cache filename in that location
+            $filepath = $filepath . \DIRECTORY_SEPARATOR . self::DEFAULT_RESULT_CACHE_FILENAME;
+        }
+
+        $this->cacheFilename = $filepath ?? $_ENV['PHPUNIT_RESULT_CACHE'] ?? self::DEFAULT_RESULT_CACHE_FILENAME;
     }
 
     /**
diff --git a/src/TextUI/TestRunner.php b/src/TextUI/TestRunner.php
index 165089587..5655b6923 100644
--- a/src/TextUI/TestRunner.php
+++ b/src/TextUI/TestRunner.php
@@ -159,12 +159,24 @@ public function doRun(Test $suite, array $arguments = [], bool $exit = true): Te
         }
 
         if ($arguments['cacheResult']) {
-            if (isset($arguments['cacheResultFile'])) {
-                $cache = new DefaultTestResultCache($arguments['cacheResultFile']);
-            } else {
-                $cache = new DefaultTestResultCache;
+            if (!isset($arguments['cacheResultFile'])) {
+                if (isset($arguments['configuration']) && $arguments['configuration'] instanceof Configuration) {
+                    $cacheLocation = $arguments['configuration']->getFilename();
+                } else {
+                    $cacheLocation = $_SERVER['PHP_SELF'];
+                }
+
+                $arguments['cacheResultFile'] = null;
+
+                $cacheResultFile = \realpath($cacheLocation);
+
+                if ($cacheResultFile !== false) {
+                    $arguments['cacheResultFile'] = \dirname($cacheResultFile);
+                }
             }
 
+            $cache = new DefaultTestResultCache($arguments['cacheResultFile']);
+
             $this->addExtension(new ResultCacheExtension($cache));
         }

In 7.5 I did 9c2fcfd because the issue solved there resulted in a TypeError in PHPUnit 8 due to strict interpretation of scalar types.

However, with the changes shown above, running the tests for PHPUnit 8.1 results in the following files getting created:

  • tests/_files/.phpunit.result.cache
  • tests/_files/DataProviderIssue2859/.phpunit.result.cache
  • tests/_files/phpunit-example-extension/.phpunit.result.cache
  • tests/basic/.phpunit.result.cache
  • tests/end-to-end/loggers/_files/.phpunit.result.cache
  • tests/end-to-end/regression/GitHub/1216/.phpunit.result.cache
  • tests/end-to-end/regression/GitHub/1265/.phpunit.result.cache
  • tests/end-to-end/regression/GitHub/1330/.phpunit.result.cache
  • tests/end-to-end/regression/GitHub/322/.phpunit.result.cache
  • tests/end-to-end/regression/GitHub/3379/.phpunit.result.cache

Is this behaviour correct, @epdenouden, and we just need to add these files to .gitignore?

@sebastianbergmann
Copy link
Owner

I came to the conclusion that it is the right thing to do here to ignore the generated files.

@epdenouden
Copy link
Contributor

Is this behaviour correct, @epdenouden, and we just need to add these files to .gitignore?

No it certainly isn't! The "we are running our self-tests" environment variable should prevent this. I will have a deeper look. Self-testing a testing framework properly just hurts my brain every now and then. Apologies for the fix, I should have run more tests on it.

@epdenouden epdenouden reopened this Apr 28, 2019
@epdenouden
Copy link
Contributor

@sebastianbergmann Perhaps I am misinterpreting some comments from yesterday, but I do not see a lot of cache files being generated by 7.5 at least:

image

@sebastianbergmann
Copy link
Owner

You probably do not see them because of acd938f.

@sebastianbergmann
Copy link
Owner

Never mind, you're looking using find and not git status.

@sebastianbergmann
Copy link
Owner

I did not see those files on 7.5 but on 8.1. See at the bottom of #3587 (comment).

@epdenouden
Copy link
Contributor

Just made a fresh branch of 8.2-etcetc and I do not see result caches everywhere, only one in the main directory.

When running from another directory new caches get created, as expected. Here's the end result of running the Basic self-test suite from its own directory in tests/basic/:

image

For now it seems to work as expected. Don't hesitate to reopen the issue if a new scenario pops up!

@frederikbosch
Copy link

When I run a test with PHPUnit from PHPStorm I see them all over when I run a single test (test method). I think this cannot be desired behaviour, right? Using PHPStorm 2019.1 and PHPUnit 8.1.3. Based on the last comments from this discussion I am not sure if there was any action on it.

@sebastianbergmann
Copy link
Owner

Changes were made to remedy what you describe, @frederikbosch, but there is no release yet that has them.

@reinholdfuereder
Copy link

reinholdfuereder commented May 21, 2019

@sebastianbergmann and @epdenouden Will that changes also help for our situation, where the PHPUnit PHAR file is located on a share and execution of single PHPUnit Tests via PhpStorm IDE (without configuration file) will always try to create the '.phpunit.result.cache' next to the shared PHPUnit PHAR file, leading to warning logs like:

Testing started at 15:55 ...
[sftp://myuser@myhost.acme.com:22]:/usr/bin/php /mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar --bootstrap /home/data/myuser/unittests/unit/bootstrap.php --no-configuration TestFramework\\Util\\TraitUtilTest /home/data/myuser/unittests/unit/testframework/test/TestFramework/Util/TraitUtilTest.php --teamcity
PHPUnit 8.1.5 by Sebastian Bergmann and contributors.


PHP Warning:  file_put_contents(/mnt/data/irgendwas/components/phpunit/8.x/.phpunit.result.cache): failed to open stream: Permission denied in phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/DefaultTestResultCache.php on line 107
PHP Stack trace:
PHP   1. {main}() /mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar:0
PHP   2. PHPUnit\TextUI\Command::main() /mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar:619
PHP   3. PHPUnit\TextUI\Command->run() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/TextUI/Command.php:163
PHP   4. PHPUnit\TextUI\TestRunner->doRun() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/TextUI/Command.php:207
PHP   5. PHPUnit\Runner\ResultCacheExtension->executeAfterLastTest() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/TextUI/TestRunner.php:617
PHP   6. PHPUnit\Runner\ResultCacheExtension->flush() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/ResultCacheExtension.php:89
PHP   7. PHPUnit\Runner\DefaultTestResultCache->persist() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/ResultCacheExtension.php:29

PHP   8. PHPUnit\Runner\DefaultTestResultCache->saveToFile() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/DefaultTestResultCache.php:84
PHP   9. file_put_contents() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/DefaultTestResultCache.php:107

Warning: file_put_contents(/mnt/data/irgendwas/components/phpunit/8.x/.phpunit.result.cache): failed to open stream: Permission denied in phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/DefaultTestResultCache.php on line 107

Call Stack:
    0.0672     868992   1. {main}() /mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar:0
    1.6985    9865904   2. PHPUnit\TextUI\Command::main() /mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar:619
    1.6985    9866016   3. PHPUnit\TextUI\Command->run() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/TextUI/Command.php:163
    1.9119   49307264   4. PHPUnit\TextUI\TestRunner->doRun() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/TextUI/Command.php:207
    1.9224   49411600   5. PHPUnit\Runner\ResultCacheExtension->executeAfterLastTest() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/TextUI/TestRunner.php:617
    1.9224   49411600   6. PHPUnit\Runner\ResultCacheExtension->flush() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/ResultCacheExtension.php:89
    1.9224   49411600   7. PHPUnit\Runner\DefaultTestResultCache->persist() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/ResultCacheExtension.php:29
    1.9224   49411600   8. PHPUnit\Runner\DefaultTestResultCache->saveToFile() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/DefaultTestResultCache.php:84
    1.9225   49415744   9. file_put_contents() phar:///mnt/data/irgendwas/components/phpunit/8.x/phpunit.phar/phpunit/Runner/DefaultTestResultCache.php:107



Time: 1.92 seconds, Memory: 56.25 MB

OK (13 tests, 13 assertions)

Process finished with exit code 0

In the course of analysing the problem we stumbled over $_ENV['PHPUNIT_RESULT_CACHE'] (

$this->cacheFilename = $filepath ?? $_ENV['PHPUNIT_RESULT_CACHE'] ?? self::DEFAULT_RESULT_CACHE_FILENAME;
) that we initially had hoped to configure (globally on the 'myhost.acme.com') to set to result cache file directory to user-specific non-shared value: however, since the DefaultTestResultCache c'tor is never called with null (cf.
$cacheLocation = $_SERVER['PHP_SELF'];
), this environment variable cannot be used. Actually I think it is not usable anymore since 795abc1
Should it be usable?

As far as I understand our devs using PhpStorm IDE must configure something to avoid these warnings, and the most convenient approach might be: (1) removing all existing PHPUnit runners and (2) configuring the PHPUnit runner template with the option to disable the PHPUnit result cache file via --do-not-cache-result (that is not yet documented https://phpunit.readthedocs.io/en/8.1/textui.html by the way) or alternatively to specify the location via --cache-result-file?

Of course I would be happy if global configuration via $_ENV['PHPUNIT_RESULT_CACHE'] might be supported (again), so that our devs would not need to do anything...

@reinholdfuereder
Copy link

@sebastianbergmann What do you think of my aforementioned comment #3587 (comment) (I am using latest PHPUnit release 8.1.6 and thus unfortunately have the "fix" from 8.1.4 that may help @frederikbosch, but actually causes my problem...)?

@sebastianbergmann
Copy link
Owner

@reinholdfuereder If your problem is a different/new one then please open a new ticket. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-runner CLI test runner type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

5 participants