Posted by xjm on September 16, 2012 at 6:16pm
8 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | node.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D7 |
Issue Summary
While I was doing some work on a node access issue, I noticed that the node access pager test we added in #681760: Try to improve performance and eliminate duplicates caused by node_access table joins could use a little cleanup for easier debugging. The attached patch clarifies some assertion messages and comments, and removes assertion messages that hide the more specific message provided by the base class method.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| node-access-pager-cleanup.patch | 2.08 KB | Idle | PASSED: [[SimpleTest]]: [MySQL] 41,307 pass(es). | View details |
Comments
#1
quoting xjm from IRC: "the logic is that assertText() and assertRaw() provide their own assertion messages that are much more specific" and that's why those get messages removed the others just the t().
#2
A minor thing: the first occurrence of "Lookup" was replaced with "Look up" but the second was left as-is, they should both either be left as-is or changed to the same values; fyi "lookup" is a valid word so I'd question the need to change it at all.
Beyond that, the patch works fine and does exactly what it says it does - the messages from the base class are displayed instead of the previously overridden values.
#3
Actually, that's incorrect. "Lookup" is a noun. "Look up" is a verb. :)
#4
Per irc, there's a second occurrence of "lookup" :)
#5
I get it now. Here's that fixed as well. Thanks @DamienMcKenna!
Also tagging for backport for parity.
#6
Setting it back to RTBC based because chx liked it and my own review.
#7
Yep. Committed/pushed to 8.x.
#8
Backported #5 to D7.
#9
Looks good, thanks @dcam!
#10
Committed and pushed to 7.x, too. Thanks!
#11
Automatically closed -- issue fixed for 2 weeks with no activity.