Random strings in SimpleTest should not contain $db_prefix

Please tell us more about your environment (types and versions of your webserver, PHP, and database engine).

Cannot reproduce. Is there something special in your configuration? I have never seen MySQL returning a "Data too long for column" error.

There is not need to append strings to randomly generated strings (unless test uses the suffix _field_name instead of looking for the whole thing). This should obviously still generate really long strings is db prefix is longer. Perhaps we should generate only include the SimpleTest part of the prefix since it is only being added to ensure uniqueness.

Right, which is what I commented on. Better patch.

Took me a bit to track down upload error.

I am going to install Drupal in prefix of drupal_ as you have see what I get, I can't see why this would introduce more errors that testbot doesn't see.

I am going to install Drupal in prefix of drupal_ as you have see what I get, I can't see why this would introduce more errors that testbot doesn't see.

Doesn't seem to create any problems (obviously didn't run all tests).

I get the feeling that is unrelated to this patch and probably always occured for you.

I am not sure how much further to take this, or if it makes sense to. Removing the host prefix from the randomName() method makes sense, but to do so from all table names is not a good idea. It would seem that is the issue you are having. We may just be running into the limits that database set. As such, either the tests themselves can be tweaked...or SimpleTest just won't work on a prefixed host for all test.

Applied the removal of host prefix to randomString() as well.

I just applied this patch:
patch -p0 < 535440-shorten-random_1.patch

Got this nasty error:

An error occurred.
Path: /drupal-cvs/batch?id=3&op=do
Message: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1103 Incorrect table name 'abcdefghijklmnosimpletest90237field_data_t90237namoummj_field_name_2': CREATE TABLE {field_data_t90237namoummj_field_name_2} (
`etid` INT unsigned NOT NULL COMMENT 'The entity type id this data is attached to',
`bundle` VARCHAR(32) NOT NULL DEFAULT '' COMMENT 'The field instance bundle to which this row belongs, used when deleting a field instance',
`deleted` TINYINT NOT NULL DEFAULT 0 COMMENT 'A boolean indicating whether this data item has been deleted',
`entity_id` INT unsigned NOT NULL COMMENT 'The entity id this data is attached to',
`revision_id` INT unsigned DEFAULT NULL COMMENT 'The entity revision id this data is attached to, or NULL if the entity type is not versioned',
`delta` INT unsigned NOT NULL COMMENT 'The sequence number for this data item, used for multi-value fields',
`language` VARCHAR(32) NOT NULL DEFAULT '' COMMENT 'The language for this data item.',
`t90237namoummj_field_name_value` TINYINT DEFAULT NULL,
PRIMARY KEY (`etid`, `entity_id`, `deleted`, `delta`, `language`),
INDEX `t90237namoummj_field_name_value` (`t90237namoummj_field_name_value`)
) ENGINE = InnoDB DEFAULT CHARACTER SET UTF8 COMMENT 'Data storage for field 2 (t90237namoummj_field_name)'; Array
(
)
in db_create_table() (line 2133 of /Applications/MAMP/htdocs/drupal-cvs/includes/database/database.inc).

Which is worse than the previous nasty error I got:

An error occurred.
Path: /drupal-cvs/batch?id=2&op=do
Message: FieldException: Attempt to create a field with a name longer than 32 characters: <em>abcdefghijklmnos628849vp2uf95q_field_name</em> in field_create_field() (line 222 of /Applications/MAMP/htdocs/drupal-cvs/modules/field/field.crud.inc)

and continuing on to the "Please continue to the error page" error page link now just gives me:

Error message No active batch.

I'm very frustrated with simpleTests right now. It's a system with a lot of potential, but...

#667416: Filefield tests fail with db_prefix > 6 characters is a duplicate of this problem.

Why is the $db_prefix added to the random strings in the first place? This was introduced in #295864: SimpleTest: Change randomName() method to randomString() but without any real explanation AFAICT. Previously randomName() had a prefix argument that allowed each caller to get their own prefix, but prefixing all strings in a test with the same prefix seems doesn't seem to have a good purpose, does it?

No, it doesn't. All it does is cause two problems in several places:

a.) The strings are way too big for database tables, and other things (drupal_simpletest19344_field_something_something_drupal_simpletest19344, wtf?)
b.) The strings are longer than the length that the caller asks for. This has actually resulted in a test passing when it should have failed, on a 255 character boundary issue.

Can we please make these prefixes just disappear entirely from the random functions?

This patch strips the prefix from the random strings.

Now that randomName() does not add a prefix, we need to make sure that field names do not start with a digit.

Easier to just have randomName() start the string with a letter.

global $db_prefix is not needed in those functions, either.

Hmm, there is no functional difference between #40 and $41, so why the failures? Perhaps it is because the strings are actually random now, rather than having a predictable prefix.

Yes, there is a peculiar problem now because the tests fail only some of the time when randomName() and randomString() return truly random strings. This would take a lot of trial and error to work out due to the chancy nature of the failures. I would assume the failing tests have design flaws, since randomName() etc should actually return a random string, not a $db_prefix + random string.

/me likes

Hold it! This patch has been documented to make some tests fail non-reliably, and #41 was re-tested before I could document what the fails were. It indicates that some tests in our suite are written under false assumptions that randomName() and randomString() have 1) a certain minimum length and 2) a predictable prefix. Since we are removing that, we need to run tests over and over to determine which ones need tweaking.

Sounds fun.

Perhaps just make a couple of patches the generate strings with small count (min), max, etc. I'm sure you can figure it out ;)

This isn't a show-stopper, since it only affects tests and tests are done in a controlled environment anyway. Still would be nice to work out though.

@carlos8f What for you change dblog.test ? checkplain for limited set of ascii chars? and limit to 4?

The changes to DBLog are actually just a workaround for #718662: DBLog listings truncate messages in the middle of HTML tags (I did not realize this entirely when I wrote the patch in comment #37).

It's a bit tricky: When the username is 8 characters long (with the patch in this issue, randomName() by default returns 8 characters instead of a much longer string), the test looks for Session opened for <em class="placeholder">rtBXvJNp</em> (56 characters). However, the real string in user.module is Session opened for <em class="placeholder">rtBXvJNp</em>. (57 characters - note the trailing period), and that is truncated to Session opened for <em ..., i.e. the test now fails. Without the patch, randomName() returned a longer string that were truncated in both cases.

We should probably wait for that bug to land before proceeding in this issue.

So waiting for someone to re-roll patch and there's enough people to RTBC :)

Here is re-roll except hunk from #718662: DBLog listings truncate messages in the middle of HTML tags

Re-roll with fix for search tests - use explicit "search string" else string could be partially displayed so test will fall randomly...

Still depends on #718662: DBLog listings truncate messages in the middle of HTML tags

#59: 535440_simpletest_random_d7.patch queued for re-testing.

Search comment tests changed to use exactly "Test comment subject" and "Test comment body" because using random{Name|String} could produce names that less then 3 chars so brings random test fails.

Also limited username in dblog tests, suppose this change has more priority then #718662: DBLog listings truncate messages in the middle of HTML tags

Now patch stably performs tests!

Committed to CVS HEAD. Thanks.