-
-
Notifications
You must be signed in to change notification settings - Fork 541
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
Add scoped value formatters #2676
Add scoped value formatters #2676
Conversation
Qodana for .NETIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
Pull Request Test Coverage Report for Build 9565943493Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9566134362Details
💛 - Coveralls |
1082f19
to
9bd75b8
Compare
Pull Request Test Coverage Report for Build 9593231468Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9593346015Details
💛 - Coveralls |
9bd75b8
to
f929234
Compare
Pull Request Test Coverage Report for Build 9594672423Details
💛 - Coveralls |
1e8834b
to
50b1f2c
Compare
Pull Request Test Coverage Report for Build 9632815207Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9632826420Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice PR. I mostly have minor suggestions that would make the PR even better.
Tests/FluentAssertions.Specs/Execution/AssertionScope.ScopedFormatters.cs
Outdated
Show resolved
Hide resolved
4cfa82b
to
81796df
Compare
81796df
to
6523752
Compare
Pull Request Test Coverage Report for Build 9668772806Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9668821122Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9668876071Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
6523752
to
fe217ea
Compare
Pull Request Test Coverage Report for Build 9698700015Details
💛 - Coveralls |
fe217ea
to
6bf0046
Compare
Pull Request Test Coverage Report for Build 9744452616Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a quick review just by reading the changes.
As this combines formatters on different levels (static and instance), I'd like to open this up in my IDE to have a deeper look.
We have previously had different bugs for assertion scopes,
e.g. #2329, #2318 and #2044 so we should triple check that we not re-introducing race conditions when e.g. modifying custom formatters on a inner scope while also having statically defined formatters.
Possibly related, #2619 is a case where we didn't consider that a functionality would be added to the shared static context and hence must be thread-safe when executing tests in parallel.
Tests/FluentAssertions.Specs/Execution/AssertionScope.ScopedFormatters.cs
Show resolved
Hide resolved
Tests/FluentAssertions.Specs/Execution/AssertionScope.ScopedFormatters.cs
Outdated
Show resolved
Hide resolved
1124aa0
to
7c63e1c
Compare
7c63e1c
to
f9ba153
Compare
Pull Request Test Coverage Report for Build 9789169633Details
💛 - Coveralls |
fda7d9a
to
f27cc39
Compare
Co-authored-by: Dennis Doomen <dennis.doomen@avivasolutions.nl>
Co-authored-by: Dennis Doomen <dennis.doomen@avivasolutions.nl>
Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com>
f27cc39
to
5efc728
Compare
Pull Request Test Coverage Report for Build 9789460304Details
💛 - Coveralls |
Closes: #2478
IMPORTANT
./build.sh --target spellcheck
or.\build.ps1 --target spellcheck
before pushing and check the good outcome