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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

vegantriathlete’s picture

Issue tags: +dcco2017

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mayurjadhav’s picture

Issue tags: +DrupalMumbaiCodeSprint
vegantriathlete’s picture

Issue tags: -dcco2017

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

SherFengChong’s picture

Assigned: Unassigned » SherFengChong

I will work on this

RoSk0’s picture

Assigned: SherFengChong » Unassigned
Issue tags: -DrupalMumbaiCodeSprint

Sher is not going to work on this anymore.

mohit1604’s picture

Assigned: Unassigned » mohit1604
mohit1604’s picture

Status: Active » Needs review
FileSize
53.73 KB

Providing required patch , Please review it :)

Status: Needs review » Needs work

The last submitted patch, 11: 2887161-11-D8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mohit1604’s picture

Working on fixing test ;)

mohit1604’s picture

Status: Needs work » Needs review
FileSize
53.73 KB
mohit1604’s picture

Assigned: mohit1604 » Unassigned
Status: Needs review » Needs work
mohit1604’s picture

Status: Needs work » Needs review
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I went through all the changes and every single of them look like the left value is the expected value.

catch’s picture

Status: Reviewed & tested by the community » Fixed

This doesn't apply to 8.5.x, so I've committed it only to 8.6.x.

  • catch committed b211213 on 8.6.x
    Issue #2887161 by Mohit Malik, joelpittet: Ensure argument order is...
mohit1604’s picture

Status: Fixed » Needs review
FileSize
51.42 KB

@catch , Thanks for committing to 8.6.x , this patch is for 8.5.x :)

apaderno’s picture

Version: 8.6.x-dev » 8.5.x-dev
catch’s picture

Status: Needs review » Reviewed & tested by the community

OK this is worth trying to keep in sync, so moving back to RTBC.

Wim Leers’s picture

❤️

dawehner’s picture

Too 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.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed b79a827 and pushed to 8.5.x. Thanks!

  • catch committed b79a827 on 8.5.x
    Issue #2887161 by Mohit Malik, joelpittet: Ensure argument order is...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.