Download & Extend
Issues

Fix php path detection in run-tests.sh

Posted by msonnabaum on March 10, 2012 at 9:01pm
Project:Drupal core
Version:7.x-dev
Component:simpletest.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)
Issue tags:needs backport to D7

Issue Summary

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.

AttachmentSizeStatusTest resultOperations
un-f-runtests-script.patch1.39 KBIdlePASSED: [[SimpleTest]]: [MySQL] 35,631 pass(es).View details
Login or register to post comments

Comments

#1

Posted by sun on March 14, 2012 at 1:24am

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]).

#2

Posted by Damien Tournoud on March 14, 2012 at 7:20am
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?)

#3

Posted by msonnabaum on April 21, 2012 at 10:22pm
Title:Make run-test.sh saner» Fix php path detection in run-tests.sh
Status:needs work» needs review

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

AttachmentSizeStatusTest resultOperations
un-f-runtests-script-3.patch1018 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 35,371 pass(es).View details

#4

Posted by Damien Tournoud on April 21, 2012 at 10:40pm
Status:needs review» reviewed & tested by the community

#5

Posted by webchick on April 23, 2012 at 2:25am

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

#6

Posted by msonnabaum on April 23, 2012 at 3:17am

Totally backportable.

#7

Posted by xjm on April 26, 2012 at 10:01pm

#8

Posted by Dries on April 29, 2012 at 4:22pm
Version:8.x-dev» 7.x-dev

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

#9

Posted by tim.plunkett on April 29, 2012 at 4:46pm
Version:7.x-dev» 8.x-dev

This did not get pushed yet, setting back.

#10

Posted by tim.plunkett on April 30, 2012 at 3:57am
Version:8.x-dev» 7.x-dev

http://drupalcode.org/project/drupal.git/commit/4b6355f

#11

Posted by msonnabaum on April 30, 2012 at 5:03am

Same patch should apply fine for D7 with -p2.

#12

Posted by tim.plunkett on April 30, 2012 at 3:02pm
Status:reviewed & tested by the community» needs review

Rerolled.

AttachmentSizeStatusTest resultOperations
drupal-1477110-12.patch998 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 38,859 pass(es).View details

#13

Posted by xjm on April 30, 2012 at 9:03pm
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)

#14

Posted by webchick on May 1, 2012 at 5:06am
Status:reviewed & tested by the community» fixed

great thanks!

committed and pushed to 7.x.

#15

Posted by System Message on May 15, 2012 at 5:10am
Status:fixed» closed (fixed)

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