Currently, if a test passes a custom environment into the 'drush' or 'execute' method, we use proc_open with a clean environment to run the test. Unfortunately, this environment is too clean, and may be missing settings needed to make the tests run.

After unsuccessfully trying to find and inject the right environment variables into proc_open, I eventually decided it was, perhaps, more profitable to just save and restore the custom environment settings. Perhaps this is not ideal, but it does work better than the existing code.

CommentFileSizeAuthor
#5 unish-environment-5.patch3.31 KBgreg.1.anderson
#4 unish-environment-4.patch4.34 KBgreg.1.anderson
#1 unish-failed-environment.patch2.77 KBgreg.1.anderson
unish-environment.patch2.49 KBgreg.1.anderson

Comments

greg.1.anderson’s picture

greg.1.anderson commented
StatusFileSize
new unish-failed-environment.patch2.77 KB

For what it's worth, here is my failed experiment. For some reason, this causes extra whitespace to be added to the end of one of the output lines in outputFormatTest. In comparison, the patch in #0 does not cause this to happen.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

#0 fixes the test fail for me. #0 seems RTBC. The only alternative I can think of is to not do any putenv in PHP but set the env only after we shell out. I don't think #0 will leak env changes to subsequent tests, but I'm certain that the code below won't do that.

foreach ($env as $key => $val) {
   $prefix .= "$key=$val;";
}
exec($prefix . $command, $this->_output, $return);

Also, the new COLUMNS env should have a code comment.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Reviewed & tested by the community » Needs work

Oh, yeah, I like #2. I'll roll a new patch.

greg.1.anderson’s picture

greg.1.anderson commented
StatusFileSize
new unish-environment-4.patch4.34 KB

Here's a new patch that works with most of the unit tests, including the output format test and the drush script test; however, it is failing here:

1) completeCase::testComplete
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-@dev aaaaaaaard-: (f) aaaaaaaard-ant
+@dev aaaaaaaard-: (f) aaaaaaaard-zebra

/home/ga/local/drupal/drush/tests/completeTest.php:187
/home/ga/local/drupal/drush/tests/completeTest.php:58

In comparison, this same test does not fail with the patch from #0. Still trying to figure out why that is. It seems that "completetest.drush.inc" is not being included for some reason.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Needs work » Needs review
StatusFileSize
new unish-environment-5.patch3.31 KB

Here's a hybrid technique that works.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Needs review » Fixed

#5 worked, so committed it.

Status: Fixed » Closed (fixed)

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