Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Add other compare extensions and tests. #21020

Merged
merged 1 commit into from
Jun 14, 2017
Merged

Conversation

JeremyKuhne
Copy link
Member

Note that xunit bends over backwards when comparing in their asserts. They try to handle nullable, but that should never be an issue as nullable can never satisfy constraints.

@JeremyKuhne JeremyKuhne added this to the 2.1.0 milestone Jun 13, 2017
@JeremyKuhne JeremyKuhne requested a review from danmoseley June 13, 2017 23:36
@JeremyKuhne
Copy link
Member Author

@dotnet-bot test Portable Windows x64 Release Build

java.lang.NullPointerException?

/// </summary>
/// <param name="actual">The value that should be greater than or equal to <paramref name="greaterThanOrEqualTo"/></param>
/// <param name="greaterThanOrEqualTo">The value that <paramref name="actual"/> should be greater than or equal to.</param>
public static void GreaterThanOrEqualTo<T>(T actual, T greaterThanOrEqualTo, string userMessage = null) where T : IComparable
Copy link
Member

@danmoseley danmoseley Jun 14, 2017

Choose a reason for hiding this comment

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

This could be written in just "one" line

        public static void GreaterThanOrEqualTo<T>(T actual, T greaterThanOrEqualTo, string userMessage = null) where T : IComparable
        {
             if ((actual == null && greaterThanOrEqualTo != null) || actual.CompareTo(greaterThanOrEqualTo) < 0)
                throw new XunitException(AddOptionalUserMessage($@"Expected: {actual ?? ""<null>""} to be greater than or equal to {greaterThanOrEqualTo ?? ""<null>""}", userMessage));
        }

Copy link
Member

Choose a reason for hiding this comment

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

same for the other one

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a little bit more than that:

            if ((actual == null && greaterThanOrEqualTo != null) || (actual != null && actual.CompareTo(greaterThanOrEqualTo) < 0))
                throw new XunitException(AddOptionalUserMessage($@"Expected: {actual?.ToString() ?? Null} to be greater than or equal to {greaterThanOrEqualTo?.ToString() ?? Null}", userMessage));

I opted for clearer and more verbose in these cases as this is a test assertion. I'm ok with merging the other one (LessThanOrEqualTo), but I'd rather keep the layout the same to make visually comparing the two methods easier.

@danmoseley danmoseley merged commit 99dc6c9 into dotnet:master Jun 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants