Skip to content
This repository was archived by the owner on May 27, 2026. It is now read-only.

fix: resolve 4 SonarQube code quality issues#6

Open
sonarqube-agent[bot] wants to merge 1 commit into
developfrom
remediate-develop-20260527-130116-1f2363d9
Open

fix: resolve 4 SonarQube code quality issues#6
sonarqube-agent[bot] wants to merge 1 commit into
developfrom
remediate-develop-20260527-130116-1f2363d9

Conversation

@sonarqube-agent

Copy link
Copy Markdown
Contributor

This PR was automatically created by the Remediation Agent's Scheduled backlog remediation feature.

Removes unnecessary [TestFixture] attributes from test classes that use xUnit-style tests instead of NUnit attributes, corrects the argument order in Assert.AreEqual calls to match the expected-first convention, and removes an unused private field. These changes eliminate false positives in static analysis and improve code quality compliance.

View Project in SonarCloud


Fixed Issues

csharpsquid:S2187 - Add some tests to this class. • BLOCKERView issue

Location: src/Cake.NUnitRetry.Tests/NUnit3DeserializerTests.cs:12

Why is this an issue?

To ensure proper testing, it is important to include test cases in a test class. If a test class does not have any test cases, it can give the wrong impression that the class being tested has been thoroughly tested, when in reality, it has not.

What changed

Removes the [TestFixture] attribute from the NUnit3DeserializerTests class. The static analysis warning reported that a class marked with [TestFixture] must contain test methods annotated with [Test], [TestCase], [TestCaseSource], or [Theory]. Since this class uses xUnit-style tests (not NUnit attributes on methods), removing the [TestFixture] attribute eliminates the false requirement for NUnit-style test methods and resolves the 'Add some tests to this class' warning.

--- a/src/Cake.NUnitRetry.Tests/NUnit3DeserializerTests.cs
+++ b/src/Cake.NUnitRetry.Tests/NUnit3DeserializerTests.cs
@@ -11,1 +10,0 @@ namespace Cake.NUnitRetry.Tests
-    [TestFixture]
csharpsquid:S3415 - Make sure these 2 arguments are in the correct order: expected value, actual value. • MAJORView issue

Location: src/Cake.NUnitRetry.Tests/NUnit3DeserializerTests.cs:53

Why is this an issue?

The standard assertions library methods such as AreEqual and AreSame in MSTest and NUnit, or Equal and Same in XUnit, expect the first argument to be the expected value and the second argument to be the actual value.

What changed

Swaps the argument order in all Assert.AreEqual calls so that the hard-coded expected value comes first and the actual value (from results object) comes second. Previously, the actual value was passed as the first argument and the expected value as the second, which is the reverse of the correct convention. This fixes the 'Make sure these 2 arguments are in the correct order: expected value, actual value' warning for all five assertion statements.

--- a/src/Cake.NUnitRetry.Tests/NUnit3DeserializerTests.cs
+++ b/src/Cake.NUnitRetry.Tests/NUnit3DeserializerTests.cs
@@ -49,5 +48,5 @@ namespace Cake.NUnitRetry.Tests
-                Assert.AreEqual(results.Total, 18, "Wrong total amount");
-                Assert.AreEqual(results.Passed, 12, "Wrong passed amount");
-                Assert.AreEqual(results.Failed, 2, "Wrong failed amount");
-                Assert.AreEqual(results.Inconclusive, 1, "Wrong inconclusive amount");
-                Assert.AreEqual(results.Skipped, 3, "Wrong skipped amount");
+                Assert.AreEqual(18, results.Total, "Wrong total amount");
+                Assert.AreEqual(12, results.Passed, "Wrong passed amount");
+                Assert.AreEqual(2, results.Failed, "Wrong failed amount");
+                Assert.AreEqual(1, results.Inconclusive, "Wrong inconclusive amount");
+                Assert.AreEqual(3, results.Skipped, "Wrong skipped amount");
csharpsquid:S2187 - Add some tests to this class. • BLOCKERView issue

Location: src/Cake.NUnitRetry.Tests/TestListWriterTests.cs:12

Why is this an issue?

To ensure proper testing, it is important to include test cases in a test class. If a test class does not have any test cases, it can give the wrong impression that the class being tested has been thoroughly tested, when in reality, it has not.

What changed

Removes the [TestFixture] attribute from the outer class TestListWriterTests, which was flagged because it had no test methods directly within it. The class itself is a container for nested test fixture classes, so it should not be annotated with [TestFixture].

--- a/src/Cake.NUnitRetry.Tests/TestListWriterTests.cs
+++ b/src/Cake.NUnitRetry.Tests/TestListWriterTests.cs
@@ -11,1 +10,0 @@ namespace Cake.NUnitRetry.Tests
-    [TestFixture]
csharpsquid:S4487 - Remove this unread private field '_environment' or refactor the code to use its value. • CRITICALView issue

Location: src/Cake.NUnitRetry/XmlResultUpdater.cs:13

Why is this an issue?

Private fields which are written but never read are a case of "dead store". Changing the value of such a field is useless and most probably indicates an error in the code.

What changed

Removes the private field '_environment' that was written but never read, directly fixing the dead store code smell where the field was assigned in the constructor but never used anywhere in the class.

--- a/src/Cake.NUnitRetry/XmlResultUpdater.cs
+++ b/src/Cake.NUnitRetry/XmlResultUpdater.cs
@@ -13,1 +12,0 @@ namespace Cake.NUnitRetry
-        private readonly ICakeEnvironment _environment;

Have a suggestion or found an issue? Share your feedback here.


SonarQube Remediation Agent uses AI. Check for mistakes.

Fixed issues:
- AYfoIqPIwRyr12UHVtuJ for csharpsquid:S4487 rule
- AYfoIqOlwRyr12UHVtuC for csharpsquid:S2187 rule
- AYfoIqOlwRyr12UHVtuH for csharpsquid:S3415 rule
- AYfoIqKwwRyr12UHVtuB for csharpsquid:S2187 rule

Generated by SonarQube Agent (task: 141e4bac-387a-4faa-9515-f84e190ac7a9)
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

1 participant