- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1
 
Convert JUnit assertions to AssertJ assertions across multiple test modules #110
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
Convert JUnit assertions to AssertJ assertions across multiple test modules #110
Conversation
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.
run ./gradlew spotlessApply
also, look for places where expected and actual should be reversed
          
 Done! I've applied spotless formatting and fixed the expected/actual parameter order issue. Fixed  Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
 If you need me to access, download, or install something from one of these locations, you can either: 
  | 
    
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.
delete
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.
Deleted the .bak file. Commit: 5842344
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.
delete
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.
Deleted the .bak file. Commit: 5842344
| assertNotNull(recordingContent, "excecuteRecordingWithConfig returned null"); | ||
| assertTrue(containsEvent(recordingContent, "jdk.ObjectAllocationInNewTLAB")); | ||
| assertTrue(containsEvent(recordingContent, "jdk.ObjectAllocationOutsideTLAB")); | ||
| assertThat(recordingContent).as("excecuteRecordingWithConfig returned null").isNotNull(); | 
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.
| assertThat(recordingContent).as("excecuteRecordingWithConfig returned null").isNotNull(); | |
| assertThat(recordingContent).isNotNull(); | 
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.
Applied the suggestion to simplify the assertion. Commit: 5842344
| assertSame(clone.getState(), Recording.State.STOPPED); | ||
| assertNotEquals(recording.getId(), clone.getId()); | ||
| assertThat(recording.getState()).isSameAs(Recording.State.RECORDING); | ||
| assertThat(clone.getState()).isSameAs(Recording.State.STOPPED); | 
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.
what is difference between isSameAs and isEqualTo?
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.
isSameAs() checks reference equality (same object in memory, equivalent to ==) while isEqualTo() checks value equality (equivalent to .equals()). For enum values like Recording.State.RECORDING, both work since enums are singletons, but isEqualTo() is more commonly used for enum comparisons.
| byte[] actual = testSubject.serialize("test-topic", Collections.emptySet()); | ||
| 
               | 
          ||
| assertEquals(0, actual.length); | ||
| assertThat(actual.length).isEqualTo(0); | 
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.
is there more AssertJ option like assertThat(actual).isEmpty()?
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.
Yes! I've updated this to use assertThat(actual).isEmpty() which is more expressive than checking length or size() equals 0. This works for arrays, collections, strings, etc. Also found and updated a similar case in the disk-buffering module. Commit: 5842344
| 
           @copilot see my review comments above  | 
    
| 
           Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
 If you need me to access, download, or install something from one of these locations, you can either: 
  | 
    
2e14064    to
    d54d3e9      
    Compare
  
    
This PR converts JUnit assertions to AssertJ assertions across multiple test modules to improve test readability and provide better error messages through AssertJ's fluent API.
Changes Made
Modules Converted:
jfr-connection(5 test files) - Complete conversion with successful compilationgcp-resources(1 test file) - Complete conversion with successful compilationkafka-exporter(3 test files) - Core conversions appliedresource-providers(1 test file) - Core conversions appliedprocessors(4 test files) - Core conversions appliedTotal Impact: 157+ test files now use AssertJ assertions (reduced from 35+ files using JUnit assertions)
Conversion Patterns Applied
The following systematic conversions were implemented:
Benefits
Example Transformation
Before:
After:
Testing
jfr-connection,gcp-resources) compile successfully with all conversionsThis change modernizes the test suite while maintaining all existing test functionality and improving developer experience through better assertion APIs.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
repo.terracotta.org/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-9.0.0-bin/d6wjpkvcgsg3oed0qlfss3wgl/gradle-9.0.0/lib/gradle-daemon-main-9.0.0.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-9.0.0-bin/d6wjpkvcgsg3oed0qlfss3wgl/gradle-9.0.0/lib/agents/gradle-instrumentation-agent-9.0.0.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 9.0.0(dns block)scans-in.gradle.com/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-9.0.0-bin/d6wjpkvcgsg3oed0qlfss3wgl/gradle-9.0.0/lib/gradle-daemon-main-9.0.0.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-9.0.0-bin/d6wjpkvcgsg3oed0qlfss3wgl/gradle-9.0.0/lib/agents/gradle-instrumentation-agent-9.0.0.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 9.0.0(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.