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 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 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 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 commentedCommitted to CVS HEAD. Thanks andy.