Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem
PHP Warning: fopen(/phpunit-1103.xml): failed to open stream: Permission denied
in /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/phpunit/phpunit/PHPUnit/Util/Printer.php on line 105
PHP Warning: Invalid argument supplied for foreach()
in /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh on line 658
Cause
simpletest.module:
function simpletest_phpunit_xml_filepath($test_id) {
return drupal_realpath('public://simpletest') . '/phpunit-' . $test_id . '.xml';
}
Remaining tasks
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
File a followup issue | Novice |
Comment | File | Size | Author |
---|---|---|---|
#18 | drupal8.run-tests-stream.18.patch | 1.75 KB | sun |
#3 | drupal8.run-tests-stream.3.patch | 1.75 KB | sun |
#2 | drupal8.run-tests-stream.2.patch | 1.74 KB | sun |
#1 | drupal8.run-tests-stream.1.patch | 487 bytes | sun |
Comments
Comment #1
sunComment #2
sunThis fixes the issue for me locally.
How to reproduce/test:
Comment #3
sunOne more tweak - adding
$recursive = TRUE
tomkdir('public://simpletest')
Comment #4
sunComment #7
John_B CreditAttribution: John_B commentedI reproduced the warning on a clean git copy of 8.x-alpha11 (without an installed site), tested the patch, which works, and read the code.
In the command to reproduce in #2, db name and path should be same for --sqlite and --dburl (?). Also I cannot get run-tests.sh to work without quotation marks round the class name - maybe there should be a new issue to document this?
Marking as RTBC. I am pretty new to this. Still it seems uncontroversial and the code is straightforward.
Comment #8
catchIs there an issue for
?
From the issue title that's what I expected the patch to do.
Comment #9
ParisLiakos CreditAttribution: ParisLiakos commentedi guess we can commit this as a stopgag and continue then here to actually remove the dependency?
Comment #10
sunAlright, so here's the fundamental problem space + root cause:
The test runner needs a filesystem path to write to. Period.
Whether that is for PHPUnit XML result files, or for a --sqlite database, or for the --xml junit output, or anything else, doesn't matter. We need a writable filesystem path.
In HEAD, we're working around that problem by installing an entire Drupal site for the test runner. In turn,
public://simpletest/
is available to the test runner, which means that it has a dedicated place to write to.If there is no Drupal site for the test runner, then there is no "public files directory" (at all).
Along the same lines,
simpletest_requirements()
creates the (writable)/sites/simpletest
base directory that holds the test site directories for each executed Simpletest test.If Simpletest module is not installed, then that directory does not exist. The only reason the testbot did not complain is because the testbot not only operates a root user, but additionally performs a blatant
chmod -rf 0777 ./*
prior to executing run-tests.sh, which in turn means that the test runner has write access everywhere.Whenever I'm using the new "headless" --sqlite + --dburl parameters on my local box, I'm specifying a writable path for all the things that need to be written by the test runner; e.g.:
However, (1) that doesn't address
/sites/simpletest
which happens to exist on my disk already, and (2) I'm running on Windows, so I generally face no file permission issues (much rather the opposite; a malformed test run would be able to delete tons of other filesystem data).The existing patch here essentially retains
public://simpletest/
as-is; i.e., for "headless" test runs the default directories of a Drupal site are assumed, even though there is no Drupal site. If you'd start from a completely clean slate, that would translate into:Whichever way we go, we'll need at least these two writable filesystem locations. Since code relies on
public://simpletest
anyway already, we could just simply make the public files directory of the (assumed) default site that required writable directory.The
sites/simpletest
directory is required in any case, because Drupal needs to detect that before Drupal is bootstrapped in any way.FWIW, this problem space is not limited to the test runner + Simpletest. PHPUnit tests should have a dedicated filesystem location to be able to write to, too. Not only for test results, but also for dynamically generated test data - which is very common for PHPUnit tests normally; our PHPUnit tests in core just weren't able to do that yet, because they have no filesystem directory to write to.
Comment #11
catchJust to be clear on #8, I'm fine committing this, just prefer follow-up issues to @todos. A @todo without an issue is either an empty apology or wishful thinking.
Comment #12
xjmSo to be clear, the only thing blocking this issue is to file the followup issue based on the @todo and #10?
Sounds like a straightforward novice task.
Comment #13
RainbowArrayFiled a follow-up issue here. Feel free to rewrite the issue summary if my understanding of the problem is incorrect. #2318797: Remove dependency on public://simplestest from Simpletest
Comment #14
RainbowArrayPatch in #3 no longer applies. Needs reroll.
Comment #15
RainbowArrayThis patch modifies simpletest_script_bootstrap, which seems to no longer exist. Haven't been able to find an equivalent where the code additions in this patch might apply.
Comment #16
xjm@mdrummond, maybe some potential leads:
Comment #17
RainbowArraysimpletest_script_bootstrap was removed in #2016629: Refactor bootstrap to better utilize the kernel. Will look for answers there.
Comment #18
sunRe-rolled against HEAD, additionally taking #10 into account.
Comment #19
moshe weitzman CreditAttribution: moshe weitzman commentedThat will work for now.
Comment #24
sunComment #26
alexpottCommitted 234382b and pushed to 8.0.x. Thanks!
Comment #27
sun