Skip to content

fix: simplify AssertJ File assertions in FileLocationTest#409

Open
sonarqube-agent[bot] wants to merge 1 commit into
masterfrom
remediate-master-20260516-050246-1dc7d94a
Open

fix: simplify AssertJ File assertions in FileLocationTest#409
sonarqube-agent[bot] wants to merge 1 commit into
masterfrom
remediate-master-20260516-050246-1dc7d94a

Conversation

@sonarqube-agent
Copy link
Copy Markdown

Refactored File-related assertions in FileLocationTest to use AssertJ's dedicated File assertion methods instead of extracting properties and comparing with generic assertions. This improves code readability and follows AssertJ best practices by using specialized assertions like exists(), isFile(), and hasName() instead of chaining method calls with isTrue() and isEqualTo().

View Project in SonarCloud


Fixed Issues

java:S5838 - Use assertThat(actual).exists() instead. • MINORView issue

Location: orchestrator-parent:sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java:80

Why is this an issue?

AssertJ contains many assertions methods specific to common types. Both versions will test the same things, but the dedicated one will provide a better error message, simplifying the debugging process.

What changed

This hunk fixes two AssertJ assertion simplification issues at lines 80-81. First, it replaces assertThat(location.getFile().exists()).isTrue() with assertThat(location.getFile()).exists(), using the dedicated File assertion method instead of checking the boolean result. Second, it replaces assertThat(location.getFile().getName()).isEqualTo("foo.txt") with assertThat(location.getFile()).hasName("foo.txt"), using the dedicated hasName() assertion instead of extracting the name and comparing with isEqualTo().

--- a/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
+++ b/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
@@ -80,2 +80,2 @@ public class FileLocationTest {
-    assertThat(location.getFile().exists()).isTrue();
-    assertThat(location.getFile().getName()).isEqualTo("foo.txt");
+    assertThat(location.getFile()).exists();
+    assertThat(location.getFile()).hasName("foo.txt");
java:S5838 - Use assertThat(actual).hasName(expected) instead. • MINORView issue

Location: orchestrator-parent:sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java:81

Why is this an issue?

AssertJ contains many assertions methods specific to common types. Both versions will test the same things, but the dedicated one will provide a better error message, simplifying the debugging process.

What changed

This hunk fixes two AssertJ assertion simplification issues at lines 80-81. First, it replaces assertThat(location.getFile().exists()).isTrue() with assertThat(location.getFile()).exists(), using the dedicated File assertion method instead of checking the boolean result. Second, it replaces assertThat(location.getFile().getName()).isEqualTo("foo.txt") with assertThat(location.getFile()).hasName("foo.txt"), using the dedicated hasName() assertion instead of extracting the name and comparing with isEqualTo().

--- a/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
+++ b/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
@@ -80,2 +80,2 @@ public class FileLocationTest {
-    assertThat(location.getFile().exists()).isTrue();
-    assertThat(location.getFile().getName()).isEqualTo("foo.txt");
+    assertThat(location.getFile()).exists();
+    assertThat(location.getFile()).hasName("foo.txt");
java:S5838 - Use assertThat(actual).isFile() instead. • MINORView issue

Location: orchestrator-parent:sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java:89

Why is this an issue?

AssertJ contains many assertions methods specific to common types. Both versions will test the same things, but the dedicated one will provide a better error message, simplifying the debugging process.

What changed

This hunk fixes three AssertJ assertion simplification issues at lines 89-91. First, it replaces assertThat(location.getFile().isFile()).isTrue() with assertThat(location.getFile()).isFile(), using the dedicated File assertion. Second, it replaces assertThat(location.getFile().exists()).isTrue() with assertThat(location.getFile()).exists(), using the dedicated existence assertion. Third, it replaces assertThat(location.getFile().getName()).isEqualTo("foo.txt") with assertThat(location.getFile()).hasName("foo.txt"), using the dedicated hasName() assertion instead of extracting the name and comparing with isEqualTo().

--- a/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
+++ b/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
@@ -89,3 +89,3 @@ public class FileLocationTest {
-    assertThat(location.getFile().isFile()).isTrue();
-    assertThat(location.getFile().exists()).isTrue();
-    assertThat(location.getFile().getName()).isEqualTo("foo.txt");
+    assertThat(location.getFile()).isFile();
+    assertThat(location.getFile()).exists();
+    assertThat(location.getFile()).hasName("foo.txt");
java:S5838 - Use assertThat(actual).exists() instead. • MINORView issue

Location: orchestrator-parent:sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java:90

Why is this an issue?

AssertJ contains many assertions methods specific to common types. Both versions will test the same things, but the dedicated one will provide a better error message, simplifying the debugging process.

What changed

This hunk fixes three AssertJ assertion simplification issues at lines 89-91. First, it replaces assertThat(location.getFile().isFile()).isTrue() with assertThat(location.getFile()).isFile(), using the dedicated File assertion. Second, it replaces assertThat(location.getFile().exists()).isTrue() with assertThat(location.getFile()).exists(), using the dedicated existence assertion. Third, it replaces assertThat(location.getFile().getName()).isEqualTo("foo.txt") with assertThat(location.getFile()).hasName("foo.txt"), using the dedicated hasName() assertion instead of extracting the name and comparing with isEqualTo().

--- a/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
+++ b/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
@@ -89,3 +89,3 @@ public class FileLocationTest {
-    assertThat(location.getFile().isFile()).isTrue();
-    assertThat(location.getFile().exists()).isTrue();
-    assertThat(location.getFile().getName()).isEqualTo("foo.txt");
+    assertThat(location.getFile()).isFile();
+    assertThat(location.getFile()).exists();
+    assertThat(location.getFile()).hasName("foo.txt");
java:S5838 - Use assertThat(actual).hasName(expected) instead. • MINORView issue

Location: orchestrator-parent:sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java:91

Why is this an issue?

AssertJ contains many assertions methods specific to common types. Both versions will test the same things, but the dedicated one will provide a better error message, simplifying the debugging process.

What changed

This hunk fixes three AssertJ assertion simplification issues at lines 89-91. First, it replaces assertThat(location.getFile().isFile()).isTrue() with assertThat(location.getFile()).isFile(), using the dedicated File assertion. Second, it replaces assertThat(location.getFile().exists()).isTrue() with assertThat(location.getFile()).exists(), using the dedicated existence assertion. Third, it replaces assertThat(location.getFile().getName()).isEqualTo("foo.txt") with assertThat(location.getFile()).hasName("foo.txt"), using the dedicated hasName() assertion instead of extracting the name and comparing with isEqualTo().

--- a/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
+++ b/sonar-orchestrator-locators/src/test/java/com/sonar/orchestrator/locator/FileLocationTest.java
@@ -89,3 +89,3 @@ public class FileLocationTest {
-    assertThat(location.getFile().isFile()).isTrue();
-    assertThat(location.getFile().exists()).isTrue();
-    assertThat(location.getFile().getName()).isEqualTo("foo.txt");
+    assertThat(location.getFile()).isFile();
+    assertThat(location.getFile()).exists();
+    assertThat(location.getFile()).hasName("foo.txt");

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


SonarQube Remediation Agent uses AI. Check for mistakes.

Fixed issues:
- AZl2kM_eTJNPgjOVPf12 for java:S5838 rule
- AZl2kM_eTJNPgjOVPf13 for java:S5838 rule
- AZl2kM_eTJNPgjOVPf14 for java:S5838 rule
- AZl2kM_eTJNPgjOVPf10 for java:S5838 rule
- AZl2kM_eTJNPgjOVPf11 for java:S5838 rule

Generated by SonarQube Agent (task: 06f732a7-85e1-4c17-bb45-7a7e8c04b430)
@sonar-review-alpha
Copy link
Copy Markdown

sonar-review-alpha Bot commented May 16, 2026

Summary

⚠️ The PR description exceeded the analysis limit and was truncated. The review may not reflect all context.

This PR replaces 5 generic AssertJ assertions with dedicated File assertion methods across two test methods in FileLocationTest (lines 80–81 and 89–91). The changes are mechanical refactoring with identical semantics—no test logic is altered. Specifically:

  • .exists().isTrue().exists()
  • .isFile().isTrue().isFile()
  • .getName().isEqualTo(...).hasName(...)

These simplifications leverage AssertJ's File-specific APIs for clearer, more maintainable test code.

What reviewers should know

Verification is straightforward: Each change replaces a property extraction + generic assertion with a single dedicated method. The behavior is identical in all cases—just using the right AssertJ API.

Two test methods affected:

  1. testLocationOfShared() (line 80–81) — 2 assertions updated
  2. testLocationOfSharedWithSystemProperty() (line 89–91) — 3 assertions updated

Note: Line 79 in the second test method uses .isFile().isTrue() instead of the dedicated .isFile() method. This inconsistency is unrelated to the PR scope but may be worth noting for future cleanup.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

Copy link
Copy Markdown

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

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

The PR correctly migrates five assertions to dedicated AssertJ File methods, but one instance of the old pattern is left behind in the first test method — inconsistent with both the stated goal and the fix applied in the second method.

🗣️ Give feedback

@@ -77,18 +77,18 @@ public void ofSharedWithEnv() throws URISyntaxException {

FileLocation location = FileLocation.ofShared("abap/foo.txt");
assertThat(location.getFile().isFile()).isTrue();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This line still uses the old pattern (assertThat(file.isFile()).isTrue()) that this PR is explicitly fixing. The identical assertion on line 89 in the second test method was migrated to assertThat(location.getFile()).isFile(), but this one was skipped. The fix is incomplete and leaves a live S5838 issue behind.

Suggested change
assertThat(location.getFile().isFile()).isTrue();
assertThat(location.getFile()).isFile();

@sonarqube-next
Copy link
Copy Markdown

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant