It would be nice to be able to write tests for the Drupal installer.

Of course with the testbot we test the installer all the time, but that is one one specific scenario: a browser-based installation with a particular install profile and a particular set of options chosen (e.g., no language imports).

Due to #524728: Refactor install.php to allow Drupal to be installed from the command line we can now install Drupal via an API so it should be possible to write tests which exercise this API and at the same time make sure that various installation options work correctly.

As far as I can tell, this wouldn't even be that hard (famous last words)... the only limitation is that SimpleTest needs to install on an existing site but with a different database prefix, so we may need some small changes to the API to make sure that it allows that (while still preserving security).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein’s picture

Status: Active » Postponed

However, this is currently postponed on the followup patch at #524728: Refactor install.php to allow Drupal to be installed from the command line being committed. This is because SimpleTest runs in a web browser, and the API functions which we need currently live in install.php itself, a file which cannot be included by a web browser since it automatically triggers a browser-based installation to begin.

David_Rothstein’s picture

Status: Postponed » Active

No longer postponed - the followup patch at #524728: Refactor install.php to allow Drupal to be installed from the command line was committed a while ago, and the API functions we need are now available via normal methods, by including includes/install.core.inc

Also, here's an example of an issue where this would help in allowing automated tests to be written for it: #585012: Setting a default theme in install profile causes WHACK errors

jhedstrom’s picture

Seems like one of the first steps would be to allow tests to override which profile is installed. Currently, this is hard-coded into the setUp() function in drupal_web_test_case.php:

    // Include the default profile.
    variable_set('install_profile', 'standard');
    $profile_details = install_profile_info('standard', 'en');

    // Install the modules specified by the default profile.
    module_enable($profile_details['dependencies'], FALSE);

moving that to a separate method, such as setUpProfile() would allow tests to implement their own setUpProfile method, which would allow for testing of different install profiles.

David_Rothstein’s picture

Yes, for the specific case of testing different install profiles, #279515: setting an installation profile for a test doesn't run hook_install_tasks() would do that.

This issue would intend to go further by switching the custom simpletest installation code to use the API, which also would allow different installation options besides the profile to be tested. But the other one is a huge step and luckily already RTBC, whereas I would love to get this one in, but personally speaking don't have much time to work on it at the moment, compared to other priorities :(

sun’s picture

Version: 7.x-dev » 8.x-dev
Category: task » feature
Issue tags: +Testing system
xjm’s picture

It's come up in #1464944: Installation of configuration system fails in some cases that it would be a good idea to have a documented list of scenarios to test once this feature is in place. From that issue:

I've tested this in every way I could think of, via drush si and install.php, with $config_directory_name set in settings.php and without it, with an existing database and without it, with a directory already existing with correct permissions and without them, and with no directory in place.

and

I tried installing Drupal with this patch and a non-writable settings.php file (with database credentials already included in it). The result was that Drupal installed "successfully" but $config_directory_name was still an empty string. So, this needs work.

Gábor Hojtsy’s picture

xjm’s picture

alexpott’s picture

Category: feature » bug
Status: Active » Needs review
Issue tags: +Security
FileSize
2.7 KB
3.39 KB

I would actually go so far as to say this is critical and a bug... since without the ability to test the interactive installer we are shipping with bugs we don't know about...

For example, install Drupal 8 and visit core/install.php again... you'll get the following error:

Fatal error: __clone method called on non-object in /Users/alex/dev/sites/drupal8alt.dev/core/modules/user/user.module on line 722

What's even more fun about this is actually whilst this is what you see on the screen the http status is a 200!!!

In order to enable testing we could move the user agent header check to after bootstrapping Drupal and use drupal_valid_test_ua(). Is the time based check in drupal_valid_test_ua() good enough to protect us?

Patch #1 just contains the change to install.core.inc and adds the new test...
Patch #2 fixes the issue in the user.module in the same way as #1951068: install.php error reporting is broken and shows that we actually have more issues to fix :)

Status: Needs review » Needs work

The last submitted patch, 630446-interactive-installer-test.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
5.16 KB
1.61 KB

Fixed the test:

Cannot access install.php with a "simpletest" user-agent header.	Browser	SimpleTestTest.php	72	Drupal\simpletest\Tests\SimpleTestTest->testInternalBrowser()

To wait for at least 5 seconds so that we can test the time based protection...

Interdiff is based on the 630446-interactive-installer-test-with-user-fix.patch - we should still have a warning generated by the new Drupal\system\Tests\Installer\AlreadyInstalledTest test.

xjm’s picture

xjm’s picture

Status: Needs review » Closed (duplicate)

Actually, closing as a duplicate, since we now have two issues for exactly the same task. I talked to @alexpott about moving his stuff from #11 and #13 to other issues.

David_Rothstein’s picture

Title: Allow SimpleTest to test the Drupal installer » Allow SimpleTest to test the non-interactive installer
Category: bug » task
Status: Closed (duplicate) » Active

This isn't a duplicate, just poorly titled. Partly because when I wrote it I never believed something like #1961938: Test the interactive installer would have ever been possible :)

As mentioned above, this issue is closely related to #1215104: Use the non-interactive installer in WebTestBase::setUp(). Now that that's in, I wonder if this one would be trivial?

David_Rothstein’s picture

Status: Active » Needs review
FileSize
6.14 KB

Seems like it.

This contains a test for the ability to set the site name during installation as an illustration of how testing the non-interactive installer can work. (Note that if I switched this test to using the standard install profile and additionally searched for "Welcome to [site:name]" on the front page, it would probably reveal a bug since right now in Drupal 8 it seems to be hardcoded to say "Welcome to Drupal" - not sure if there's an issue for that.)

It would be nice to find a way to combine this with #1961938: Test the interactive installer if both eventually happen, since maybe there could be an InstallerTestCase and if you extend that you would just have to provide the installation parameters and write your test once, then the test case itself would be smart enough to run it twice (once with the interactive installer and once with the non-interactive one)?

xjm’s picture

Note that if I switched this test to using the standard install profile and additionally searched for "Welcome to [site:name]" on the front page, it would probably reveal a bug since right now in Drupal 8 it seems to be hardcoded to say "Welcome to Drupal" - not sure if there's an issue for that.

Filed #1963268: The frontpage view no results title should be "Welcome to [site:name]" (as opposed to hardcoding "Drupal"). Thanks @alexpott for the heads-up and @David_Rothstein for finding it. :)

alexpott’s picture

Status: Needs review » Needs work

This needs a re-roll... due to changes in core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php

David_Rothstein’s picture

Status: Needs work » Needs review
FileSize
6.13 KB

Looks like it was patch fuzz rather than an actual conflict, but yeah, Git was refusing to apply it.

Here is a new version.

chx’s picture

Status: Needs review » Reviewed & tested by the community

This seems like a slam dunk, zero functional change anywhere in both the test base and the install system itself.

alexpott’s picture

Title: Allow SimpleTest to test the non-interactive installer » Change notice: Allow SimpleTest to test the non-interactive installer
Priority: Major » Critical
Status: Reviewed & tested by the community » Active
Issue tags: -Security +Needs change record

Committed http://drupalcode.org/project/drupal.git/commit/c5d0a8e and pushed to 8.x. Thanks!

chx’s picture

Why does this need a change notice and what should the change notice contain...? I am totally baffled at this.

chx’s picture

Sorry didnt mean to revert tags.

xjm’s picture

@chx: "Added support for testing the non-interactive installer." + example

moshe weitzman’s picture

Er, I would think that most tests would use non-interactive installer and the interactive install would be the exception. Non interactive does less http traffic and thus performs better.

moshe weitzman’s picture

Sigh

moshe weitzman’s picture

Sigh

chx’s picture

Priority: Critical » Major
Status: Active » Fixed
Issue tags: -Needs change record +Security

http://drupal.org/node/1971002 (tags this time are for real)

Status: Fixed » Closed (fixed)

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

alexpott’s picture

Title: Change notice: Allow SimpleTest to test the non-interactive installer » Allow SimpleTest to test the non-interactive installer