diff --git a/rules/S3431/csharp/flow-example.adoc b/rules/S3431/csharp/flow-example.adoc new file mode 100644 index 0000000000..29bacf14aa --- /dev/null +++ b/rules/S3431/csharp/flow-example.adoc @@ -0,0 +1,33 @@ +[source,csharp] +---- +[TestMethod] +[ExpectedException(typeof(InvalidOperationException))] +public void UsingTest() +{ + Console.ForegroundColor = ConsoleColor.Black; + try + { + using var _ = new ConsoleAlert(); + Assert.AreEqual(ConsoleColor.Red, Console.ForegroundColor); + throw new InvalidOperationException(); + } + finally + { + Assert.AreEqual(ConsoleColor.Black, Console.ForegroundColor); // The exception itself is not relevant for the test. + } +} + +public sealed class ConsoleAlert : IDisposable +{ + private readonly ConsoleColor previous; + + public ConsoleAlert() + { + previous = Console.ForegroundColor; + Console.ForegroundColor = ConsoleColor.Red; + } + + public void Dispose() => + Console.ForegroundColor = previous; +} +---- diff --git a/rules/S3431/csharp/how-mstest.adoc b/rules/S3431/csharp/how-mstest.adoc new file mode 100644 index 0000000000..39ee97e6c8 --- /dev/null +++ b/rules/S3431/csharp/how-mstest.adoc @@ -0,0 +1,30 @@ +== How to fix it in MSTest + +Remove the `ExpectedException` attribute in favor of using the https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.testtools.unittesting.assert.throwsexception[Assert.ThrowsException] assertion. + +=== Code examples + +==== Noncompliant code example + +[source,csharp,diff-id=1,diff-type=noncompliant] +---- +[TestMethod] +[ExpectedException(typeof(ArgumentNullException))] // Noncompliant +public void Method_NullParam() +{ + var sut = new MyService(); + sut.Method(null); +} +---- + +==== Compliant solution + +[source,csharp,diff-id=1,diff-type=compliant] +---- +[TestMethod] +public void Method_NullParam() +{ + var sut = new MyService(); + Assert.ThrowsException(() => sut.Method(null)); +} +---- diff --git a/rules/S3431/csharp/how-nunit.adoc b/rules/S3431/csharp/how-nunit.adoc new file mode 100644 index 0000000000..61eb1652a9 --- /dev/null +++ b/rules/S3431/csharp/how-nunit.adoc @@ -0,0 +1,30 @@ +== How to fix it in NUnit + +Remove the `ExpectedException` attribute in favor of using the https://docs.nunit.org/articles/nunit/writing-tests/assertions/classic-assertions/Assert.Throws.html[Assert.Throws] assertion. + +=== Code examples + +==== Noncompliant code example + +[source,csharp,diff-id=2,diff-type=noncompliant] +---- +[Test] +[ExpectedException(typeof(ArgumentNullException))] // Noncompliant +public void Method_NullParam() +{ + var sut = new MyService(); + sut.Method(null); +} +---- + +==== Compliant solution + +[source,csharp,diff-id=2,diff-type=compliant] +---- +[Test] +public void Method_NullParam() +{ + var sut = new MyService(); + Assert.Throws(() => sut.Method(null)); +} +---- diff --git a/rules/S3431/csharp/rule.adoc b/rules/S3431/csharp/rule.adoc index 31e121d06e..177dd645c4 100644 --- a/rules/S3431/csharp/rule.adoc +++ b/rules/S3431/csharp/rule.adoc @@ -1,52 +1,18 @@ +include::../../../shared_content/dotnet/csharp_dictionary.adoc[] +:language: csharp + == Why is this an issue? include::../description.adoc[] -=== Noncompliant code example - -[source,csharp] ----- -[TestMethod] -[ExpectedException(typeof(ArgumentNullException))] // Noncompliant -public void TestNullArg() -{ - //... -} ----- - -=== Compliant solution - -[source,csharp] ----- -[TestMethod] -public void TestNullArg() -{ - bool callFailed = false; - try - { - //... - } - catch (ArgumentNullException) - { - callFailed = true; - } - Assert.IsTrue(callFailed, "Expected call to MyMethod to fail with ArgumentNullException"); -} ----- - -or - -[source,csharp] ----- -[TestMethod] -public void TestNullArg() -{ - Assert.ThrowsException(() => /*...*/); -} ----- - include::../exceptions.adoc[] +include::./how-mstest.adoc[] + +include::./how-nunit.adoc[] + +include::../resources.adoc[] + ifdef::env-github,rspecator-view[] ''' diff --git a/rules/S3431/description.adoc b/rules/S3431/description.adoc index d6edc97065..9cbcf70334 100644 --- a/rules/S3431/description.adoc +++ b/rules/S3431/description.adoc @@ -1,3 +1,3 @@ It should be clear to a casual reader what code a test is testing and what results are expected. Unfortunately, that's not usually the case with the `ExpectedException` attribute since an exception could be thrown from almost any line in the method. -This rule detects MSTest and NUnit `ExpectedException` attribute. \ No newline at end of file +This rule detects MSTest and NUnit `ExpectedException` attribute. diff --git a/rules/S3431/exceptions.adoc b/rules/S3431/exceptions.adoc index 83dd36e152..9b67c42eed 100644 --- a/rules/S3431/exceptions.adoc +++ b/rules/S3431/exceptions.adoc @@ -1,3 +1,8 @@ === Exceptions -This rule ignores one-line test methods, since it is obvious in such methods where the exception is expected to be thrown. \ No newline at end of file +This rule ignores: + +* single-line tests, since it is obvious in such methods where the exception is expected to be thrown +* tests when it tests control flow and assertion are present in either a `{keyword_catch}` or `{keyword_finally}` clause + +include::{language}/flow-example.adoc[] diff --git a/rules/S3431/metadata.json b/rules/S3431/metadata.json index 470c1ce024..bbcb9721de 100644 --- a/rules/S3431/metadata.json +++ b/rules/S3431/metadata.json @@ -28,7 +28,7 @@ "sqKey": "S3431", "scope": "Tests", "defaultQualityProfiles": [ - + "Sonar way" ], "quickfix": "unknown" } diff --git a/rules/S3431/resources.adoc b/rules/S3431/resources.adoc new file mode 100644 index 0000000000..201e9f555c --- /dev/null +++ b/rules/S3431/resources.adoc @@ -0,0 +1,8 @@ +== Resources + +=== Documentation + +* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.testtools.unittesting.assert.throwsexception[Assert.ThrowsException Method] +* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.testtools.unittesting.expectedexceptionattribute[ExpectedExceptionAttribute Class] +* NUnit - https://docs.nunit.org/articles/nunit/writing-tests/assertions/classic-assertions/Assert.Throws.html[Assert.Throws] +* NUnit - https://docs.nunit.org/2.4/exception.html[ExpectedExceptionAttribute] diff --git a/rules/S3431/vbnet/flow-example.adoc b/rules/S3431/vbnet/flow-example.adoc new file mode 100644 index 0000000000..69c9184314 --- /dev/null +++ b/rules/S3431/vbnet/flow-example.adoc @@ -0,0 +1,31 @@ +[source,vbnet] +---- + + +Public Sub UsingTest() + Console.ForegroundColor = ConsoleColor.Black + Try + Using alert As New ConsoleAlert() + Assert.AreEqual(ConsoleColor.Red, Console.ForegroundColor) + Throw New InvalidOperationException() + End Using + Finally + Assert.AreEqual(ConsoleColor.Black, Console.ForegroundColor) ' The exception itself is not relevant for the test. + End Try +End Sub + +Public NotInheritable Class ConsoleAlert + Implements IDisposable + + Private ReadOnly previous As ConsoleColor + + Public Sub New() + previous = Console.ForegroundColor + Console.ForegroundColor = ConsoleColor.Red + End Sub + + Public Sub Dispose() Implements IDisposable.Dispose + Console.ForegroundColor = previous + End Sub +End Class +---- diff --git a/rules/S3431/vbnet/how-mstest.adoc b/rules/S3431/vbnet/how-mstest.adoc new file mode 100644 index 0000000000..86dbcdefa4 --- /dev/null +++ b/rules/S3431/vbnet/how-mstest.adoc @@ -0,0 +1,28 @@ +== How to fix it in MSTest + +Remove the `ExpectedException` attribute in favor of using the https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.testtools.unittesting.assert.throwsexception[Assert.ThrowsException] assertion. + +=== Code examples + +==== Noncompliant code example + +[source,vbnet,diff-id=1,diff-type=noncompliant] +---- + + ' Noncompliant +Public Sub Method_NullParam() + Dim sut As New MyService() + sut.Method(Nothing) +End Sub +---- + +==== Compliant solution + +[source,vbnet,diff-id=1,diff-type=compliant] +---- + +Public Sub Method_NullParam() + Dim sut As New MyService() + Assert.ThrowsException(Of ArgumentNullException)(Sub() sut.Method(Nothing)) +End Sub +---- diff --git a/rules/S3431/vbnet/how-nunit.adoc b/rules/S3431/vbnet/how-nunit.adoc new file mode 100644 index 0000000000..408c65a446 --- /dev/null +++ b/rules/S3431/vbnet/how-nunit.adoc @@ -0,0 +1,28 @@ +== How to fix it in NUnit + +Remove the `ExpectedException` attribute in favor of using the https://docs.nunit.org/articles/nunit/writing-tests/assertions/classic-assertions/Assert.Throws.html[Assert.Throws] assertion. + +=== Code examples + +==== Noncompliant code example + +[source,vbnet,diff-id=2,diff-type=noncompliant] +---- + + ' Noncompliant +Public Sub Method_NullParam() + Dim sut As New MyService() + sut.Method(Nothing) +End Sub +---- + +==== Compliant solution + +[source,vbnet,diff-id=2,diff-type=compliant] +---- + +Public Sub Method_NullParam() + Dim sut As New MyService() + Assert.Throws(Of ArgumentNullException)(Sub() sut.Method(Nothing)) +End Sub +---- diff --git a/rules/S3431/vbnet/rule.adoc b/rules/S3431/vbnet/rule.adoc index 66c58cec7b..cab82d7829 100644 --- a/rules/S3431/vbnet/rule.adoc +++ b/rules/S3431/vbnet/rule.adoc @@ -1,46 +1,18 @@ +include::../../../shared_content/dotnet/vbnet_dictionary.adoc[] +:language: vbnet + == Why is this an issue? include::../description.adoc[] -=== Noncompliant code example - -[source,vbnet] ----- - - ' Noncompliant -Public Sub TestNullArg() - '... -End Sub ----- - -=== Compliant solution - -[source,vbnet] ----- - -Public Sub TestNullArg() - Dim CallFailed As Boolean = False - Try - ' ... - Catch ex As Exception - CallFailed = true - End Try - Assert.IsTrue(CallFailed, "Expected call to MyMethod to fail with ArgumentNullException") -End Sub ----- - -or - -[source,vbnet] ----- - -Public Sub TestNullArg() - Assert.ThrowsException(Of ArgumentNullException)(Sub() ... ) -End Sub ----- - include::../exceptions.adoc[] +include::./how-mstest.adoc[] + +include::./how-nunit.adoc[] + +include::../resources.adoc[] + ifdef::env-github,rspecator-view[] ''' == Comments And Links diff --git a/shared_content/dotnet/csharp_dictionary.adoc b/shared_content/dotnet/csharp_dictionary.adoc index 66ffc5ad40..58c4308db2 100644 --- a/shared_content/dotnet/csharp_dictionary.adoc +++ b/shared_content/dotnet/csharp_dictionary.adoc @@ -1,4 +1,6 @@ :keyword_null: null :keyword_async: async +:keyword_catch: catch +:keyword_finally: finally :concept_method: method -:typeparameter_TResult: \ No newline at end of file +:typeparameter_TResult: diff --git a/shared_content/dotnet/vbnet_dictionary.adoc b/shared_content/dotnet/vbnet_dictionary.adoc index dd2c359c20..4cb060240d 100644 --- a/shared_content/dotnet/vbnet_dictionary.adoc +++ b/shared_content/dotnet/vbnet_dictionary.adoc @@ -1,4 +1,6 @@ :keyword_null: Nothing :keyword_async: Async +:keyword_catch: Catch +:keyword_finally: Finally :concept_method: procedure -:typeparameter_TResult: (Of TResult) \ No newline at end of file +:typeparameter_TResult: (Of TResult)