Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use latest stable dependencies and update copyright year #702

Closed
wants to merge 4 commits into from

Conversation

bmscomp
Copy link
Contributor

@bmscomp bmscomp commented Nov 24, 2023

This pull request is aiming to keep a fresh and updated version of testng dependency

@bmscomp bmscomp changed the title Upgrade testng dependency to version 7.8.0 Use latest stable dependencies Nov 25, 2023
@bmscomp bmscomp changed the title Use latest stable dependencies Use latest stable dependencies and update copyright year Nov 26, 2023
@@ -96,7 +96,7 @@ public void testWithTwoGoodCDIProvider() throws Exception {
fw.write('\n');
fw.write(DummyCDIProvider.class.getName());
fw.close();
Assert.assertTrue(CDI.current().getClass().equals(DummyCDIProvider.DummyCDI.class));
Assert.assertEquals(DummyCDIProvider.DummyCDI.class, CDI.current().getClass());
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that in TestNG, the order of parameters of assertEquals is opposite to JUnit: first goes the actual value, then the expected value. (Unless TestNG 7 changed their mind, but I find that unlikely.)

Copy link
Contributor Author

@bmscomp bmscomp Nov 27, 2023

Choose a reason for hiding this comment

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

I'll fix that you are right, I just checked the signature of it

  static public void assertEquals(Object actual, Object expected) {
    assertEquals(actual, expected, null);
  }

@Ladicek isn't it the same, shouldn't have a transitive and reflective equals ? I mean both should work if A is equals to B then be must be equals A

Please tell me more about the context

Copy link
Contributor

Choose a reason for hiding this comment

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

@Ladicek isn't it the same, shouldn't have a transitive and reflective equals ? I mean both should work if A is equals to B then be must be equals A

As far as outcome of the test goes, this is true. However, the ordering matters in cases such as error reporting - then it tries to tell you something along the lines of what went wrong and which value was expected and which was observed instead and in that moment, ordering matters.

Copy link
Contributor

Choose a reason for hiding this comment

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

@manovotn is right on the money, this is about the possible error message being correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manovotn @Ladicek The pull request is alreqady updated and the order is restored

Thanks so much for the review, I am learning much

@starksm64
Copy link
Contributor

This is going to be obsoleted by my #710 PR.

@starksm64
Copy link
Contributor

Closing as it has been incorporated by #710 . Thanks for the PR @bmscomp

@starksm64 starksm64 closed this Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants