The run-tests.sh script tries to find your php binary by checking $_ENV. This is not safe. If you have variables_order set to "GPCS" in php.ini (which is common), it will be empty. This is what the getenv() function is for. Patch fixes.

Also, it returns the results of the tests with simpletest_script_format_result(), which prints html special characters as if the output is html. Patch also fixes this by capturing the output and running it through htmlspecialchars_decode() so it's less ugly.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

I'd RTBC the first hunk, but I'm not sure about the second one (which requires manual testing for various environments and scenarios [e.g., testbots]).

Damien Tournoud’s picture

Status: Needs review » Needs work

I have been meaning to do the first hunk forever. For some reason, the default PHP production configuration doesn't include E in variables_order (which seems slightly retarded, but that's another issue).

The second one looks like a hack: simpletest_script_format_result() is local to the script runner, so we could likely fix it not to return HTML in some cases (what are those? verbose results?)

msonnabaum’s picture

Title: Make run-test.sh saner » Fix php path detection in run-tests.sh
Status: Needs work » Needs review
FileSize
1018 bytes

Ok, here's just the env fix. The output fix should probably have it's own issue.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

This looks backportable to D7, to me. Is it?

msonnabaum’s picture

Totally backportable.

xjm’s picture

Issue tags: +Needs backport to D7
Dries’s picture

Version: 8.x-dev » 7.x-dev

Committed to 8.x. Moving to 7.x. Thanks!

tim.plunkett’s picture

Version: 7.x-dev » 8.x-dev

This did not get pushed yet, setting back.

tim.plunkett’s picture

msonnabaum’s picture

Same patch should apply fine for D7 with -p2.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
998 bytes

Rerolled.

xjm’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/scripts/run-tests.shundefined
@@ -253,14 +253,14 @@ function simpletest_script_init($server_software) {
-  elseif (!empty($_ENV['_'])) {
+  elseif ($php_env = getenv('_')) {

Does it look to anyone else like the little face is sadder than it used to be?

(RTBC)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

great thanks!

committed and pushed to 7.x.

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