DrupalWebTestCase does not set required site_mail variable during setUp()
Dave Reid - December 7, 2008 - 00:03
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | simpletest.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Dave Reid |
| Status: | closed |
Description
I was working on test that was using drupalPost on admin/settings/site-information, and for the longest time I couldn't figure out why it wasn't saving. I finally narrowed it down to the fact that the site_mail field was empty (which is a required field). I would think that the required variable site_mail should be set to 'simpletest@example.com' in DrupalWebTestCase::setUp(), since that's a valid, easy-to-remember e-mail address already used in contact.test.
#1
Patch provided for review.
#2
Bah, forgot to set as code needs review. I suck.
#3
A side note on this, the default value for site_mail is ini_get('sendmail_from'), which is empty on my default Ubuntu 8.04-PHP install.
#4
looks good to me. bumping this to critical, as it will break (at least) anything that tries to test admin/settings/site-information.
#5
If you need it, simply set it in the setUp() method of your test. That's clearly not a bug in Simpletest, as we don't have that approach for any other variable.
#6
Hmm...I just think it's wrong that SimpleTest is running all these tests without a valid site_mail variable since my ini_set('sendmail_from') is empty. All the other required site variables have "normal" default strings.
#7
@Damien: not sure i agree with this. i would have thought the organising idea for a test environment would be that it would match a default drupal install + some test modules.
maybe you could explain why its ok to have a test drupal install that doesn't set site_mail?
#8
Meh, a variable_set cant hurt and it is quite rational to presume that parts of Drupal will except site_mail to be set. (it's high time we provide a write-mails-to-files lib as part of simpletest. followup issue)
#9
I agree that this seems sensible to me. Special-casing this in every test that needs it (or the form it's set in) seems like an exercise in frustration. In theory, the installer should not be letting us continue without that value set though. Hm.
Anyway, committed to HEAD.
#10
Automatically closed -- issue fixed for two weeks with no activity.