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

AreEqual/AreNotEqual XML documentation is wrong #1551

Closed
cremor opened this issue Jan 12, 2023 · 3 comments · Fixed by #1563
Closed

AreEqual/AreNotEqual XML documentation is wrong #1551

cremor opened this issue Jan 12, 2023 · 3 comments · Fixed by #1563
Assignees
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting Resolution: Fixed State: Approved Type: Docs

Comments

@cremor
Copy link

cremor commented Jan 12, 2023

Many of the AreEqual/AreNotEqual overloads contain this text in their XML documentation:

Different numeric types are treated as unequal even if the logical values are equal. 42L is not equal to 42.

This is wrong.

To see this, run the following tests. Both will have results that do not match the expectation from the XML documentation.

[TestMethod]
public void ShouldFailAccordingToDocumentation()
{
    Assert.AreEqual(42L, 42);
}

[TestMethod]
public void ShouldPassAccordingToDocumentation()
{
    Assert.AreNotEqual(42L, 42);
}

AB#1726907

@ghost ghost added the Needs: Triage 🔍 label Jan 12, 2023
@Evangelink
Copy link
Member

Hi @cremor,

You are absolutely right!

I just had a look and it doesn't seem to be a regression and all versions always worked as-is. I think the first implementation was containing only Assert.AreEqual(object, object) in which case this mention makes sense but as far as I can see all released versions of MSTest since v1 have other overloads like (long, long) which are used in the example you described and in this case, because of the implicit cast, the two values are now long and so are equal.

Given that and no-one ever complained about this logical equality rather than "type" equality, I would simply update the doc.

Is it something you would be willing to contribute or shall we do it on our side?

@Evangelink Evangelink added Help-Wanted The issue is up-for-grabs, and can be claimed by commenting Type: Docs State: Approved Type: Bug and removed Needs: Triage 🔍 Type: Bug Help-Wanted The issue is up-for-grabs, and can be claimed by commenting labels Jan 17, 2023
@cremor
Copy link
Author

cremor commented Jan 17, 2023

Given that and no-one ever complained about this logical equality rather than "type" equality, I would simply update the doc.

👍

Is it something you would be willing to contribute or shall we do it on our side?

I don't think that I'll have time for this in the near future, sorry.

@Evangelink
Copy link
Member

No worries! Thanks for taking the time to reply! @engyebrahim, I am adding that to your backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help-Wanted The issue is up-for-grabs, and can be claimed by commenting Resolution: Fixed State: Approved Type: Docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants