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

Allow specifying EquivalencyOptions in string assertions #2413

Conversation

vbreuss
Copy link
Contributor

@vbreuss vbreuss commented Oct 29, 2023

Enhance the EquivalencyOptions to support the following methods:

  • IgnoringLeadingWhitespace
    Instructs the comparison to ignore leading whitespace when comparing strings.
  • IgnoringTrailingWhitespace
    Instructs the comparison to ignore trailing whitespace when comparing strings.
  • IgnoringCase
    Instructs the comparison to compare strings case-insensitive.

Add an overload for the following string assertions that accept a Func<EquivalencyOptions<string>, EquivalencyOptions<string>> config.

  • BeEquivalentTo
  • ContainEquivalentOf
  • EndWithEquivalentOf
  • MatchEquivalentOf
  • NotBeEquivalentTo
  • NotContainEquivalentOf
  • NotEndWithEquivalentOf
  • NotMatchEquivalentOf
  • NotStartWithEquivalentOf
  • StartWithEquivalentOf

Details are discussed in and fixes #2364.
Fixes #2339
Fixes #2064

IMPORTANT

  • If the PR touches the public API, the changes have been approved in a separate issue with the "api-approved" label.
  • The code complies with the Coding Guidelines for C#.
  • The changes are covered by unit tests which follow the Arrange-Act-Assert syntax and the naming conventions such as is used in these tests.
  • If the PR adds a feature or fixes a bug, please update the release notes with a functional description that explains what the change means to consumers of this library, which are published on the website.
  • If the PR changes the public API the changes needs to be included by running AcceptApiChanges.ps1 or AcceptApiChanges.sh.
  • If the PR affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
    • Please also run ./build.sh --target spellcheck or .\build.ps1 --target spellcheck before pushing and check the good outcome

@github-actions

This comment was marked as outdated.

@vbreuss
Copy link
Contributor Author

vbreuss commented Oct 29, 2023

If this pull request is accepted, it replaces #2372

@coveralls
Copy link

coveralls commented Oct 29, 2023

Pull Request Test Coverage Report for Build 7436672942

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 97.45%

Totals Coverage Status
Change from base Build 7378622478: -0.02%
Covered Lines: 11943
Relevant Lines: 12130

💛 - Coveralls

@vbreuss vbreuss force-pushed the topic/add-equivalencyassertionoptions-to-stringassertions branch from abf2b80 to 9f7b5a7 Compare October 29, 2023 14:17
@vbreuss vbreuss marked this pull request as ready for review October 30, 2023 11:30
@vbreuss
Copy link
Contributor Author

vbreuss commented Oct 30, 2023

@dennisdoomen :
I don't understand why the Qodana Scan fails. Is it a problem with the pipeline or with the code changes? Can I see somewhere, what the actual failures are (the raw build log is not really helpful for me)?

@dennisdoomen
Copy link
Member

I don't understand why the Qodana Scan fails. Is it a problem with the pipeline or with the code changes? Can I see somewhare, what the actual failures are (the raw build log is not really helpful for me)?

Is that because you're branch is behind and needs to rebase?

@vbreuss vbreuss force-pushed the topic/add-equivalencyassertionoptions-to-stringassertions branch from 78df3d1 to 0fd9a21 Compare October 30, 2023 15:17
@vbreuss
Copy link
Contributor Author

vbreuss commented Oct 30, 2023

Is that because you're branch is behind and needs to rebase?

I doubt it, as the build ran and the failure was before the merge of #2416 to develop.
Nonetheless I rebased the branch to get rid of the conflicting files.

@vbreuss vbreuss force-pushed the topic/add-equivalencyassertionoptions-to-stringassertions branch from b29c449 to b9e138c Compare November 4, 2023 15:14
Copy link
Member

@dennisdoomen dennisdoomen left a comment

Choose a reason for hiding this comment

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

That's a pretty impressive PR. It just needs a little cleaning up here and there.

@vbreuss vbreuss force-pushed the topic/add-equivalencyassertionoptions-to-stringassertions branch 2 times, most recently from f3e71aa to dcf73ff Compare November 4, 2023 16:35
@vbreuss vbreuss force-pushed the topic/add-equivalencyassertionoptions-to-stringassertions branch 3 times, most recently from 548562e to 5b84065 Compare November 9, 2023 07:43
Copy link
Member

@dennisdoomen dennisdoomen left a comment

Choose a reason for hiding this comment

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

Nice work

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 9, 2023

@vbreuss Can you try behavior instead of behaviour in string.md and check if that silence the spell check?
And if so, probably you could add the british version to the custom dict. ;)

@vbreuss
Copy link
Contributor Author

vbreuss commented Nov 9, 2023

@vbreuss Can you try behavior instead of behaviour in string.md and check if that silence the spell check? And if so, probably you could add the british version to the custom dict. ;)

Yes, changing it to behavior does indeed fix the spell check error.
image

Sorry, I was not aware, that I need to run it locally! 😃

@IT-VBFK : Where can I find the custom dictionary?

@IT-VBFK
Copy link
Contributor

IT-VBFK commented Nov 9, 2023

in the repository root cSpell.json -> words :)

@dennisdoomen
Copy link
Member

And if so, probably you could add the british version to the custom dict. ;)

It's not completely obvious, but we're using US English as our standard.

@ITaluone
Copy link
Contributor

I think the PR title is litte outdated? Should this go to "Allow specifying EquivalencyOptions in string assertions" instead?

@vbreuss vbreuss changed the title Allow specifying EquivalencyAssertionOptions in string assertions Allow specifying EquivalencyOptions in string assertions Nov 16, 2023
@vbreuss vbreuss force-pushed the topic/add-equivalencyassertionoptions-to-stringassertions branch from 303e73a to 0572cd4 Compare January 4, 2024 07:04
@vbreuss
Copy link
Contributor Author

vbreuss commented Jan 4, 2024

I also requeued the build so that wasn't necessary. Our builds build the (virtual) result of merging your PR into the target branch.

@dennisdoomen :
I saw that you manually triggered the Qodana build, but it failed again, so it seems the automatic GitHub merge was incorrect or incomplete. After the manual merge, the build was fine...

Either way, we never merge develop or master in our feature branches. We always use rebases.

I was not aware of this guideline. The build is now also successful after a rebase 😄
Maybe we could add this information to the DOs and DON'Ts?

@jnyrup jnyrup added the feature label Jan 6, 2024
Copy link
Member

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

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

Just two small things.
very impressive work ❤️

docs/_pages/releases.md Outdated Show resolved Hide resolved
@jnyrup jnyrup merged commit f3ff0ff into fluentassertions:develop Jan 14, 2024
5 checks passed
@vbreuss vbreuss deleted the topic/add-equivalencyassertionoptions-to-stringassertions branch January 15, 2024 07:45
@bart-vmware
Copy link

I don't understand how the linked PR that closes this issue solves my problem. It doesn't fix the next bug, which I pointed out earlier:

public static string RemoveNewLines(this string @this)
{
    return @this.Replace("\n", string.Empty, StringComparison.Ordinal)
        .Replace("\r", string.Empty, StringComparison.Ordinal)
        .Replace(@"\r\n", string.Empty, StringComparison.Ordinal);
}

Note the @ verbatim string modifier, which is equivalent to matching "\\r\\n" instead of "\r\n".

Aside from that, how do these changes affect my use case? In other words, what FA extension method should I be using now to pass my custom string comparer (without also ignoring leading/trailing whitespace, line breaks, casing, etc)?

@vbreuss
Copy link
Contributor Author

vbreuss commented Jan 15, 2024

@bart-vmware :
You can now specify the IEqualityComparer<string> that should be used, e.g. with the mentioned IgnoreLineEndingsComparer in #2339:

subject.Should().BeEquivalentTo(expected, o => o
    .Using(IgnoreLineEndingsComparer.Instance));

I don't understand how the linked PR that closes this issue solves my problem. It doesn't fix the next bug, which I pointed out earlier

Or do you mean another issue?

@bart-vmware
Copy link

@vbreuss Thanks, but that doesn't actually work. Tracked at #2566.

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