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.
After discussion with chx on #drupal, we found that a good majority of Database tests are wrapping their test in try-catch statements that are no longer necessary with the Drupal error handler.
Comment | File | Size | Author |
---|---|---|---|
#2 | 343631-dbtng-test-cleanup-D7.patch | 59.18 KB | Dave Reid |
#1 | 343631-dbtng-test-cleanup-D7.patch | 56.04 KB | Dave Reid |
Comments
Comment #1
Dave ReidInitial pass removing all try-catch statements. Tested locally and worked perfectly on database tests: 616 passes, 0 fails, and 0 exceptions.
Comment #2
Dave ReidSecond pass. Line whitespace changes, try-catch removal, and brings the tests in DatabaseInsertTestCase in line with each other. I feel like with these changes that the tests are running a little bit faster.
Comment #3
Dave ReidIn my completely un-scientific test-running stopwatch performance testing, the patch sped the Database tests up by about 5 seconds (with about a total running time of 220 seconds). :)
Comment #4
Dave Reidchx wanted to know why I've removed the try-catch statements from all the database tests. We are currently testing the error handler in common.test which throws the following errors in system_test.module to make sure they're caught during testing:
It is reasonable that if we're testing the error handler here, we don't need all the try-catch statements in the database tests anymore.
Comment #5
chx CreditAttribution: chx commentedI knew why you were removing them -- I suggested removing them, actually -- I rather asked, why did you not add tests for this but there you are. FYI I also asked Dave how much the speedup is in percentage and it's like 2% so not that big of a deal but still.
Comment #6
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.