Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Nov 3, 2019

What changes were proposed in this pull request?

The assertEquals method of JUnit Assert requires the first parameter to be the expected value. In this PR, I propose to change the order of parameters when the expected value is passed as the second parameter.

Why are the changes needed?

Wrong order of assert parameters confuses when the assert fails and the parameters have special string representation. For example:

assertEquals(input1.add(input2), new CalendarInterval(5, 5, 367200000000L));
java.lang.AssertionError: 
Expected :interval 5 months 5 days 101 hours
Actual   :interval 5 months 5 days 102 hours

Does this PR introduce any user-facing change?

No

How was this patch tested?

By existing tests.

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113161 has finished for PR 26377 at commit 86c1340.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 3, 2019

jenkins, retest this, please

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, great fix. Looks good pending tests.

@SparkQA
Copy link

SparkQA commented Nov 3, 2019

Test build #113179 has finished for PR 26377 at commit 86c1340.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thank you, @MaxGekk and @srowen .
Merged to master.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29733][TEST] Fix wrong order of parameters passed to assertEquals [SPARK-29733][TESTS] Fix wrong order of parameters passed to assertEquals Nov 3, 2019
@MaxGekk MaxGekk deleted the fix-order-in-assert-equals branch June 5, 2020 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants