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.
Problem/Motivation
ShortcutSetsTest::testShortcutSetSwitchNoSetName calls SafeMarkup::set() which is meant to be for internal use only.
Proposed resolution
- Remove the call by refactoring the code.
Remaining tasks
Evaluate whether the string can be refactored to one of the formats outlined in this change record: https://www.drupal.org/node/2311123Identify whether there is existing automated test coverage for the sanitization of the string. If there is, list the test in the issue summary. If there isn't, add an automated test for it.This issue is fixing a test.If the string cannot be refactored, the SafeMarkup::set() usage needs to be thoroughly audited and documented.
Manual testing steps
N/A
User interface changes
N/A
API changes
N/A
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff.txt | 1.06 KB | kgoel |
#12 | 2550963-12.patch | 1.33 KB | kgoel |
#8 | interdiff.txt | 959 bytes | kgoel |
#8 | 2550963-8.patch | 1.33 KB | kgoel |
#6 | interdiff.txt | 1.33 KB | kgoel |
Comments
Comment #2
josephdpurcell CreditAttribution: josephdpurcell commentedComment #3
akalata CreditAttribution: akalata commentedRefactored to use SafeString, since this is an internal-use-only test. The string being passed should not be escaped, and we do not need to add it to a list of safe strings.
Updating issue summary to be specific to the issue.
Comment #4
akalata CreditAttribution: akalata commentedPostponing on #2550961: Remove or document SafeMarkup::set in UserBlocksTest::testUserLoginBlock since the solution should be nearly identical.
Comment #5
kgoel CreditAttribution: kgoel at Forum One commentedPer @xjm - this is something that can be work on.
Comment #6
kgoel CreditAttribution: kgoel at Forum One commentedComment #7
akalata CreditAttribution: akalata commentedLooks like #6 adds a new line in the code where there was not one in the code before?
Comment #8
kgoel CreditAttribution: kgoel at Forum One commented@akalata - thank you! I did add a new line :P
Comment #11
justAChris CreditAttribution: justAChris as a volunteer commentedLikely failing on the extra "s" in assert
Comment #12
kgoel CreditAttribution: kgoel at Forum One commented@justAChris, thank you for review!
Yay, go me :P
Comment #13
akalata CreditAttribution: akalata commentedThanks justachris, I missed that too!
Comment #14
xjmThis is indeed much better -- as much as possible, tests should just test the expected result without adding dependencies on other code.
Committed and pushed to 8.0.x. Thanks!