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

Fixed swaped arguments #255

Merged
merged 5 commits into from
May 19, 2023
Merged

Conversation

Eneuman
Copy link
Contributor

@Eneuman Eneuman commented Apr 25, 2023

This PR fixes some tests that had swaped the actual argument with the expected argument.
It also optimizes a test by using Assert.DoesNotContains instead of multiple checks.

This change removes 4 warnings.

Fixed #254

@Eneuman Eneuman requested review from a team, Tatsinnit and sabbour as code owners April 25, 2023 20:13
@github-actions
Copy link

@Eneuman Thanks for your PR!

My in-depth review of this pull request is as follows:

This pull request looks good overall. It fixes some tests that had swaped the actual argument with the expected argument, and optimizes a test by using Assert.DoesNotContains instead of multiple checks. It also removes 4 warnings.

The changes look good. In the first change, the arguments in the Assert.Equal statement were swaped, so the change is correct. In the second change, the Assert.NotEqual statement was changed to Assert.DoesNotContain, which is more efficient and correct. In the third change, the arguments in the Assert.Equal statement were swaped, so the change is correct.

Overall, this pull request looks good and should be accepted.

Copy link
Collaborator

@hsubramanianaks hsubramanianaks left a comment

Choose a reason for hiding this comment

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

Looks good to me @Eneuman thank you.

@cxznmhdcxz
Copy link
Member

Passed on all 3 platforms.

@cxznmhdcxz cxznmhdcxz merged commit 082f816 into Azure:main May 19, 2023
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.

Some tests have swaped expected value with actual value
3 participants