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.As #250047: Rework the SimpleTest results interface and clean-up of backend code became a monster patch, here is an extract of the patch that only rework the backend.
Highlights:
- Fix the backtracing logic of
_assert() - Remove the artificial limit on the size of the 'message' field (once again a regression of the DB:TNG patch: we changed that field to be a TEXT in #293500)
- Fixed error handling inside tests
- Rename {simpletest}.caller to {simpletest}.function for consistency
- Improvements to code comments and documentation
Compared to boombatower's patch:
- I used a slightly different backtracing logic, in order to get the good caller name: for an assertion, we know return the name of the function that called the assertion, not the name of the assertion function itself.
- I kept $custom_caller (renamed $caller), because (1) we need it for errorHandler() because we don't want to backtrace in case there is an error in an assertion function, (2) I need it for #304924: Extend drupal_error_handler to manage exceptions.
- I revised several comments and corrected typos.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | simpletest-backend-rework.patch | 15.22 KB | chx |
| #6 | simpletest-backend-rework-6.patch | 15.01 KB | Damien Tournoud |
| #5 | new-backtracing.png | 176.3 KB | Damien Tournoud |
| #5 | simpletest-backend-rework-5.patch | 10.69 KB | Damien Tournoud |
| #4 | simpletest-backend-rework-4.patch | 10.74 KB | Damien Tournoud |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedAfter a discussion with chx on the IRC, I rewrote the backtracing logic a bit and added useful comments.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedAfter new dicussions with chx, I simplified the
getCallerContext()method a moved it to common.inc with the name_drupal_get_last_caller(). This is for future extensibility, as it will be useful in #304924: Extend drupal_error_handler to manage exceptions.Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedHere are results before and after the patch. Notice how file and line numbers were wrong: there were the line and file number of the call to the function, not those of the call to the assertion inside the function.
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedAdds an exception handler for tests. Essentially, this is #293521: Simpletest should catch exceptions but adapted to the context of this patch.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedAgain on chx' suggestion, here is a slight change in the backtracing logic: we now ignore all methods from DrupalWebTestCase, not only the assertions. The result actually looks great (see attached screenshot).
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedHere a new patch that removes two unneeded lines, and adds tests of the new backtracing code to
simpletest.test.Comment #7
chx CreditAttribution: chx commentedI run out of ideas of what to add / remove / fix and I even channeled Dries to DamZ on IRC by asking for a new test. So, I believe this is ready.
Comment #8
boombatower CreditAttribution: boombatower commentedErrrr....can I review this...I worked on the other patch that this stems from and I not really sure I like what is going on. Don't have time at the moment, but should later.
Comment #9
webchickYes, I'd like to hear your thoughts. I do agree with splitting this from the UI changes, however. That other patch is currently in discussion about buttons and placement and descriptions, and this patch looks like it fixes a lot of underlying things that will help us fix other testing system stuff in the meantime.
Comment #10
boombatower CreditAttribution: boombatower commentedThe patch looks good, simpletest.test passes. Just includes the back-end changes from my patch.
Once this goes in I'll work on re-rolling the other patch to reflect this, could be fun with the large overlap and slight differences. :)
Comment #11
boombatower CreditAttribution: boombatower commentedWould probably be a good idea if someone ran all the tests.
Comment #12
maartenvg CreditAttribution: maartenvg commentedI ran all the test and everything passes, 5390 passes to be exact. :)
Comment #13
boombatower CreditAttribution: boombatower commentedGreat, looks like it is ready to go.
Comment #14
webchickI'll be taking a look at this after supper and committing it providing I don't find any remaining issues.
Comment #15
chx CreditAttribution: chx commentedJust PHPdoc and indent fixes. Do not credit me.
Comment #16
webchickCommitted this, with a couple minor PHPDoc tweaks. Thanks!
Now let's get #250047: Rework the SimpleTest results interface and clean-up of backend code in. ;)
Btw, I talked over with chx that _drupal_get_last_caller($backtrace) looks like it could be generalized a bit and re-used in multiple places, if it accepts an index of how far back to parse... _db_query() does (or used to before DBTNG?) a similar thing where it grabs the "true" calling function rather than db_query() for more sensible debug messages.
Can be handled in a follow-up patch, but I'm curious about Damien's thoughts on that.
Comment #17
webchickComment #18
Damien Tournoud CreditAttribution: Damien Tournoud commentedOf course, that's exactly the plan. That's also why it is placed just below
drupal_error_handler()in common.inc :) I already have a mockup of this in #304924: Extend drupal_error_handler to manage exceptions, but it will require a reroll and some more testing.Comment #19
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.