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

.Should().BeInAscendingOrder() does not work properly for MSTest on .NET Framework 4.8 with FluentAssertions 6.9.0 #2109

Closed
KarlZ opened this issue Jan 16, 2023 · 3 comments

Comments

@KarlZ
Copy link

KarlZ commented Jan 16, 2023

Description

It seems that the comparison used in ShouldBeInAscendingOrder is different than what C# uses? At first, I ran into issues after an upgrade because the order seemed to be case sensitive now. But when trying to accommodate that (Which is why everything is in caps in my unit test) I still ran into issues. Thus, I got very basic - let C# sort and yet Should().BeInAscendingOrder() still fails.

Reproduction Steps

[TestMethod]
public void ShouldBeInAscendingOrder_Fails()
{
    // Arrange
    var items = new[] { "INJECTANT VISCOSITY", "INJECTANT/OIL MOBILITY RATIO", "INJECTANT-FREE PRODUCTION DURATION" };

    // Act
    var actual = items.OrderBy(i => i);

    // Assert
    actual.Should().BeInAscendingOrder();
}

Expected behavior

The unit test to pass.

Actual behavior

The unit test fails for MS Test project on .NET Framework 4.8 when running FluentAssertions 6.9.
It seems that with that combination FluentAssertions is expecting the "INJECTANT-" to come before the "INJECTANT/" even though the OrderBy clause sorts the opposite.
FluentAssertions 6.8 and earlier this works fine.
xUnit and MS Test projects on .NET 6 work fine with FluentAssertions 6.9.

Regression?

This test will pass in FluentAssertions 6.8.0, but not in 6.9.0.
Is this commit the issue?
28ef506

Known Workarounds

Don't upgrade beyond 6.8.0 for MSTest projects on .NET Framework 4.8.

Configuration

MS Test Project on .NET Framework 4.8 with FluentAssertions 6.9.0.

Other information

No response

@KarlZ
Copy link
Author

KarlZ commented Jan 16, 2023

FluentAssertionsIssue2109.zip
This is a solution with 3 different projects all using FluentAssertions 6.9.0.
xUnit on net6 works
MSTest on net6 works
MSTest on Net Framework 4.8 fails.

@jnyrup
Copy link
Member

jnyrup commented Jan 17, 2023

I think this is a combination of two things:

  • .NET5 changed behavior when comparing strings - docs. This probably explains the difference between net47 and net6
  • Fluent Assertions 6.9.0 changed what default comparer we use for comparing strings from culture aware to ordinal in Order strings with ordinal comparison #2075. Then intention is to behave identical on all TFMs.

A workaround is to pass in StringComparer.CurrentCulture to BeInAscendingOrder to compare the strings like OrderBy by default does.

actual.Should().BeInAscendingOrder(StringComparer.CurrentCulture);
[Fact]
public void Dash_then_space()
{
    var items = new[] { "-", " " };
    items.Should().BeInAscendingOrder();
}

[Fact]
public void Space_then_dash()
{
    var items = new[] { " ", "-" };
    items.Should().BeInAscendingOrder();
}

FluentAssertions 6.8.0 behaves differently on net47 and net6

net47 net6
Space_then_dash fail pass
Dash_then_space pass fail

FluentAssertions 6.9.0 behaves identically on net47 and net6

net47 net6
Space_then_dash pass pass
Dash_then_space fail fail

@KarlZ
Copy link
Author

KarlZ commented Jan 17, 2023

Thanks for the information and the workaround for making the old test work as we migrate toward net6+. I could not find where OrderBy was changed in the documentation. But, it clearly was.
And I prefer that this test work, which it does on net6, but not on net48. I prefer that it work on net6. It should be symmetrical (i.e., OrderBy with no comparer should work with BeInAscendingOrder with no comparer).

[Fact]
public void OrderBy_Symmetrical_To_BeInAscendingOrder()
{
    // Arrange
    var items = new[] { "-", " " };

    // Act
    var actual = items.OrderBy(i => i);

    // Assert
    actual.Should().BeInAscendingOrder();
}

And it's so nice that FluentAssertions makes migration of our MSTests to xUnit so much easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants