It is not possible to run run-test.sh in a directory containing a space on non-windows systems. This happens because of a bug in the phpunit test runner that doesn't escape the command. Because of this it only affects running phpunit based tests.

$ pwd
/var/www/localhost/htdocs/d8 1
$ php core/scripts/run-tests.sh --url http://localhost.local/d8\ 1 --color --dburl mysql://... --sqlite ./test.sqlite --verbose --class Drupal\Tests\Core\Ajax\AjaxCommandsTest,Drupal\Tests\simpletest\Unit\SimpletestPhpunitRunCommandTest

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\Core\Ajax\AjaxCommandsTest
  - Drupal\Tests\simpletest\Unit\SimpletestPhpunitRunCommandTest

Test run started:
  Monday, December 14, 2015 - 12:50

Test summary
------------

sh: 1: /var/www/localhost/htdocs/d8: Permission denied
sh: 1: /var/www/localhost/htdocs/d8: Permission denied
....
$ php vendor/bin/phpunit -c core/phpunit.xml.dist core/modules/simpletest/tests/src/Unit/SimpletestPhpunitRunCommandTest.php 
PHPUnit 4.8.11 by Sebastian Bergmann and contributors.

E

Time: 301 ms, Memory: 5.50Mb

There was 1 error:

1) Drupal\Tests\simpletest\Unit\SimpletestPhpunitRunCommandTest::testSimpletestPhpUnitRunCommand
PHPUnit_Framework_Exception: sh: 1: /var/www/localhost/htdocs/d8: Permission denied

FAILURES!
Tests: 1, Assertions: 0, Errors: 1.
CommentFileSizeAuthor
#14 simpletest_with_space.patch926 bytesneclimdul
#2 run_test_sh_doesn_t-2635046-2.patch612 bytesneclimdul

Comments

neclimdul created an issue. See original summary.

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new run_test_sh_doesn_t-2635046-2.patch612 bytes

We don't really care about escaping for security I don't think because we're basically hard coding the path but we need to escape the space. escapeshellcmd() doesn't do this anyways so here's a quick fix.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Active » Needs review
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Looks fine for me.
I was wondering whether there is some php helper function for that, but I could not find any.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/simpletest/simpletest.module
@@ -326,7 +326,7 @@ function simpletest_phpunit_command() {
-    $phpunit_bin = $vendor_dir . '/phpunit/phpunit/phpunit';
+    $phpunit_bin = str_replace(' ', '\\ ', $vendor_dir . '/phpunit/phpunit/phpunit');

I've tested this and wrapping it with escapeshellarg() works so why not do that?

dawehner’s picture

dawehner
Primary language German
commented

OH I see, so this would just put it into quotes which has the same behaviour. Yeah why not!

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work
Issue tags: +run-tests.sh

Based on #5, moving this to NW.

Also adding the tag because you can't search for 'run-tests.sh' on d.o.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

rafalenden’s picture

rafalenden commented

It also applies to Simpletest.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review
StatusFileSize
new simpletest_with_space.patch926 bytes
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

+1 for escaping shell arguments always.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed abc6c6a336 to 8.6.x and ab240fc103 to 8.5.x. Thanks!

  • alexpott committed abc6c6a on 8.6.x 
    Issue #2635046 by neclimdul, dawehner, alexpott: run-test.sh doesn't...

  • alexpott committed ab240fc on 8.5.x 
    Issue #2635046 by neclimdul, dawehner, alexpott: run-test.sh doesn't...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.