-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
feature/227 assertionmethod #229
Conversation
Hi @pavel-mikula-sonarsource, can I get you to give this PR a very quick look and see if I am doing it right. |
Codecov Report
@@ Coverage Diff @@
## dev #229 +/- ##
==========================================
+ Coverage 79.18% 79.31% +0.13%
==========================================
Files 109 109
Lines 3046 3046
Branches 396 396
==========================================
+ Hits 2412 2416 +4
+ Misses 504 502 -2
+ Partials 130 128 -2
Continue to review full report at Codecov.
|
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.
LGTM, I left some minor improvement suggestions
@@ -42,6 +43,7 @@ public static void WaitForState(this IRenderedFragmentBase renderedFragment, Fun | |||
/// <param name="assertion">The verification or assertion to perform.</param> | |||
/// <param name="timeout">The maximum time to attempt the verification.</param> | |||
/// <exception cref="WaitForFailedException">Thrown if the timeout has been reached. See the inner exception to see the captured assertion exception.</exception> | |||
[AssertionMethod] |
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.
There's inconsistency in the codebase. This method doesn't need it for purpose of SonarSource analyzers, because it contains Assert
.
And there exist other assertion methods (like DiffAssertExtensions
) that don't need it and don't have it.
CHANGELOG.md
Outdated
@@ -34,7 +34,8 @@ List of changes in existing functionality. | |||
); | |||
``` | |||
By [@egil](https://github.com/egil) in [#203](https://github.com/egil/bUnit/pull/203). | |||
- All test doubles are now in the same namespace, `Bunit.TestDoubles`. So all import statements for `Bunit.TestDoubles.JSInterop` and `Bunit.TestDoubles.Authorization` must be changed to `Bunit.TestDoubles`. By [@egil](https://github.com/egil) in [#203](https://github.com/egil/bUnit/pull/223). | |||
- All test doubles are now in the same namespace, `Bunit.TestDoubles`. So all import statements for `Bunit.TestDoubles.JSInterop` and `Bunit.TestDoubles.Authorization` must be changed to `Bunit.TestDoubles`. By [@egil](https://github.com/egil) in [#223](https://github.com/egil/bUnit/pull/223). | |||
- Marked MarkupMatches and WaitForAssertion as assertion methods to stop SonarQube analyzers complaining about missing assertions in tests. By [@egil](https://github.com/egil) in [#229](https://github.com/egil/bUnit/pull/229). |
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.
To keep it simple and accurate: We have SonarQube
, SonarCloud
, SonarLint
and also standalone NuGet packages that might complain about this.
- Marked MarkupMatches and WaitForAssertion as assertion methods to stop SonarQube analyzers complaining about missing assertions in tests. By [@egil](https://github.com/egil) in [#229](https://github.com/egil/bUnit/pull/229). | |
- Marked MarkupMatches and WaitForAssertion as assertion methods to stop SonarSource analyzers complaining about missing assertions in tests. By [@egil](https://github.com/egil) in [#229](https://github.com/egil/bUnit/pull/229). |
Thanks @pavel-mikula-sonarsource. Appreciate the input. |
Co-authored-by: Pavel Mikula <[email protected]>
src/bunit.core/Extensions/WaitForHelpers/RenderedFragmentWaitForHelperExtensions.cs
Outdated
Show resolved
Hide resolved
src/bunit.core/Extensions/WaitForHelpers/RenderedFragmentWaitForHelperExtensions.cs
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Marked MarkupMatches and WaitForAssertion as assertion methods.
Details in #227.