Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Names of generated tables for tests exceeded 64 chars as reported at #13 #710858: Meta issue: fix the remaining PostgreSQL bugs
SQLSTATE[42P07]: Duplicate table: 7 ERROR: relation "simpletest322535field_data_field_s322535u2crryva_field_s322535u" already exists
So this patch changes field_name to field_file for tests
Comment | File | Size | Author |
---|---|---|---|
#12 | 718254_test_field_length_d71.patch | 13.26 KB | andypost |
#8 | 718254_test_field_length_d7.patch | 12.37 KB | andypost |
#7 | 718254_test_field_length_d7.patch | 11.43 KB | andypost |
file_tests_d7.patch | 2.65 KB | andypost | |
Comments
Comment #1
andypostMysql just silently truncates so affected too.
Comment #2
yched CreditAttribution: yched commentedAgreed. Can we use a less common name than 'field_file', though ?
Also, I'm willing to bet there are other field tests where field names use $this->randomName()...
Comment #3
andypost@yched maybe replace with "test_file" ? I walk through other field-related tests and found no consistency in names: field_1 field_2 field_type decimal52 does_not_exists... drupal_strtolower($this->randomName() . '_field_name')
Suppose that strtolower($this->randomName()) is enough for any test!
Should we replace all occurrences?
Comment #4
c960657 CreditAttribution: c960657 commentedWhen #535440: Random strings in SimpleTest should not contain $db_prefix is fixed, the strings returned by $this->randomName() will be about 15 characters shorter.
Comment #5
andypost@c960657 I talk not only table names but index names also.
Suppose all test fields should start with test_ prefix and should not use field_ which reserved for user's namespace
Comment #6
andypostNew patch fixes
- modules/file/tests/file.test
- modules/image/image.test
- modules/field/modules/field_sql_storage/field_sql_storage.test
Also extends test for indexes for pgsql and sqlite in field_sql_storage.test because now we have API to check indexes
This is very helpful in testing #720620: indexExists() for pgsql and sqlite does not prefix tablename and #718016: DatabaseSchema_pgsql::renameTable() needs to rename the indexes
Comment #7
andypostPatch
Comment #8
andypostAnd another one from list.test, I think all field modules covered
I see no reason to use drupal_strtolower() because strings are ascii
Comment #9
andypostThis depends on #722912: db_index_exists() must conform schemaAPI indexExists()
because all implementations of indexExists are different from db_index_exists and abstract schema class
Comment #10
Dries CreditAttribution: Dries commented1. If it depends on #722912: db_index_exists() must conform schemaAPI indexExists(), how come the tests in #8 pass? Would be great if you could provide more context on the dependency.
2.
strtolower($this->randomName())
seems to be a very common pattern. Any reason why randomName() wouldn't always return lower-case strings? We strtolower() the randomName roughly 50% of the time ... could make for a really sweet clean-up IMO.3. Technically, it would be better to write
drupal_strtolower($this->randomName())
instead ofstrtolower($this->randomName())
.Comment #11
andypost@Dries It depends and pass tests because using
Database::getConnection()->schema()->indexExists()
in the changed test so I think to revert this todb_index_exist()
and glad to remove following comment in code once commited #722912: db_index_exists() must conform schemaAPI indexExists().Now about randomName() (used more then 200 times) it's most widely used then randomString() (6 times in all core test). Using of druppal_strlower() against strlower() is useless and much resource expensive because strings are not multi-byte as nature of randomName().
...The farther into the forest, the more wood
After digging into I found Cause - a bad documentation in #295864: SimpleTest: Change randomName() method to randomString()
It was planed that randomName() should be used for machine-readable-places and randomString() for user-input-data but most of test-writers are missed this point so now we need [drupal_]strlower() on result of randomName() also waiting for #535440: Random strings in SimpleTest should not contain $db_prefix
Comment #12
andypostReroll after commit #535440: Random strings in SimpleTest should not contain $db_prefix
Added small changes
- using randomString() for titles
- randomName(8) changed to randomName() because 8 is really default value now
EDIT: now it passes local tests on pgsql db with 2-character prefix 'd_'
Comment #13
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks andy.