Download & Extend

Get rid of ugly simpletest regexp.

Project:Drupal core
Version:7.x-dev
Component:simpletest.module
Category:task
Priority:normal
Assigned:Damien Tournoud
Status:closed (duplicate)

Issue Summary

We should add a simpletest_id() in bootstrap.inc, that returns the simpletest id (if available), and thus remove those ugly regexp against HTTP_USER_AGENT.

Comments

#1

Status:active» needs review

This implements simpletest_id() and fix two small things:

- the file directory should use the not prefixed id
- a typo

AttachmentSizeStatusTest resultOperations
329177-simpletest-id.patch10.8 KBIdleFailed: Failed to apply patch.View details | Re-test

#2

c9606571 noted that the installer was broken and that double prefixing could happen in tearDown().

New version up for review.

AttachmentSizeStatusTest resultOperations
329177-simpletest-id.patch11.14 KBIdleFailed: Failed to apply patch.View details | Re-test

#3

Fixed code comments and removed the unused parameter for simpletest_id(), following c9606571 review on IRC.

AttachmentSizeStatusTest resultOperations
329177-simpletest-id.patch11.17 KBIdleFailed: Failed to apply patch.View details | Re-test

#4

Status:needs review» needs work

Some tests fails with this.

Badfully, we will have to make simpletest_id() to work also in the testing site. Back to the drawing board.

#5

The files directory should most definitely use the database prefix for the same reasons the database does. Since tests can run concurrently through shell script and such it needs the prefix.

#6

Status:needs work» needs review

@boombatower: I reverted back the file directory to the old behavior. Note anyway that we now add the main database prefix just to make the "clean environment" feature work correctly, not because of potential conflicts (which should not happen because the prefix is random enough).

Also:

- moved the detection of HTTP_USER_AGENT out of the database layer and in conf_init(), where it makes sense.
- cleanup the simpletest test :)

AttachmentSizeStatusTest resultOperations
32917-simpletest-id.patch10.82 KBIdleFailed: Failed to apply patch.View details | Re-test

#7

Status:needs review» needs work

The last submitted patch failed testing.

#8

#9

Status:needs review» needs work

The last submitted patch failed testing.

#10

Status:needs work» needs review

Rerolled without significant changes.

AttachmentSizeStatusTest resultOperations
32917-simpletest-id.patch10.8 KBIdlePassed: 7017 passes, 0 fails, 0 exceptionsView details | Re-test

#11

Moved the core of the user agent detection to conf_init(), in order to simplify the logic in simpletest_id, which is now a simple get/set function.

AttachmentSizeStatusTest resultOperations
32917-simpletest-id.patch10.82 KBIdlePassed: 7017 passes, 0 fails, 0 exceptionsView details | Re-test

#12

Following chx' review on the IRC:

- renamed inTestingSite() to inChildSite() which is clearer and reworked comments
- remove a redundant new line

As a summary, this patch aims to remove the multiple regexps on HTTP_USER_AGENT or $db_prefix that were all over core, in the benefit of a simpler and clearer simpletest_id() function. It's a building block above which we will be able to implement #195416: Table prefixes should be per database connection, and that will, later, allow us to speed-up simpletest by simply duplicating a pre-installed Drupal instance, instead of rebuilding it at each test run.

AttachmentSizeStatusTest resultOperations
329177-simpletest-id.patch10.89 KBIdlePassed: 7017 passes, 0 fails, 0 exceptionsView details | Re-test

#13

Title:Implement a simpletest_id() function» Get rid of ugly simpletest regexp.

Finally, a simpletest_id() function was not enough, so I added a small static DrupalTestRunInfo class.

AttachmentSizeStatusTest resultOperations
329177-simpletest-id.patch11.23 KBIdleFailed: 6922 passes, 30 fails, 65 exceptionsView details | Re-test

#14

Status:needs review» needs work

The last submitted patch failed testing.

#15

With the patch in #13, $db_prefix is modified before settings.php is included AFAICT. If settings.php sets $db_prefix, the prefix will get overwritten. So I think this should be moved a bit down in the bootstrap process.

#16

@c960657: $db_prefix is set at the bottom of conf_init(), after the settings.php file has been included.

#17

D'oh. Sorry :-(

#18

Status:needs work» closed (duplicate)

Energy at #518404: Lock down DB config based on simpletest UA headers.

nobody click here