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.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | node-test-cleanup-1786142-8.patch | 1.98 KB | dcam |
| #5 | 1786124-5.patch | 2.12 KB | xjm |
| #5 | interdiff.txt | 765 bytes | xjm |
| node-access-pager-cleanup.patch | 2.08 KB | xjm |
Comments
Comment #1
chx commentedquoting 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().
Comment #2
damienmckennaA 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.
Comment #3
xjmActually, that's incorrect. "Lookup" is a noun. "Look up" is a verb. :)
Comment #4
damienmckennaPer irc, there's a second occurrence of "lookup" :)
Comment #5
xjmI get it now. Here's that fixed as well. Thanks @DamienMcKenna!
Also tagging for backport for parity.
Comment #6
damienmckennaSetting it back to RTBC based because chx liked it and my own review.
Comment #7
catchYep. Committed/pushed to 8.x.
Comment #8
dcam commentedBackported #5 to D7.
Comment #9
xjmLooks good, thanks @dcam!
Comment #10
webchickCommitted and pushed to 7.x, too. Thanks!