Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem
There are many assertions where the argument order doesn't match the parameters when comparing values with assertSame()
Proposed Solution
Swap the arguments for assertSame()
assertions to match the expectation of their parameters.
Order should be expected first and actual second arguments:
assertSame($expected, $actual
Comment | File | Size | Author |
---|---|---|---|
#20 | 2887161-20-D8.patch | 51.42 KB | mohit1604 |
#14 | 2887161-14-D8.patch | 53.73 KB | mohit1604 |
#11 | 2887161-11-D8.patch | 53.73 KB | mohit1604 |
Comments
Comment #2
joelpittetMost but not all of these swaps will be due to #2756519: Convert assertIdentical() to assertSame() for some uses in kernel tests that already have assertSame()'s parameter order
Comment #3
vegantriathleteComment #5
mayurjadhav CreditAttribution: mayurjadhav at Blisstering Solutions commentedComment #6
vegantriathleteComment #8
SherFengChong CreditAttribution: SherFengChong commentedI will work on this
Comment #9
RoSk0Sher is not going to work on this anymore.
Comment #10
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedComment #11
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedProviding required patch , Please review it :)
Comment #13
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedWorking on fixing test ;)
Comment #14
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedComment #15
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedComment #16
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedComment #17
dawehnerI went through all the changes and every single of them look like the left value is the expected value.
Comment #18
catchThis doesn't apply to 8.5.x, so I've committed it only to 8.6.x.
Comment #20
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commented@catch , Thanks for committing to 8.6.x , this patch is for 8.5.x :)
Comment #21
apadernoComment #22
catchOK this is worth trying to keep in sync, so moving back to RTBC.
Comment #23
Wim Leers❤️
Comment #24
dawehnerToo bad it is not possible to provide some check for this automatically! Thakn you @Mohit Malik for this. This makes failed tests easier to understand.
Comment #25
catchCommitted b79a827 and pushed to 8.5.x. Thanks!