Removed JUnit5 dependency for ClientTpcConfigTest #26265

dr-divago · 2024-02-25T17:56:26Z

Removed JUnit5 dependency for ClientTpcConfigTest

Fixes #26193

Checklist:

Labels (Team:, Type:, Source:, Module:) and Milestone set
Add Add to Release Notes label if changes should be mentioned in release notes or Not Release Notes content if changes are not relevant for release notes
Request reviewers if possible
New public APIs have @Nonnull/@Nullable annotations
New public APIs have @since tags in Javadoc
Send backports/forwardports if fix needs to be applied to past/future releases

…5) using org.junit.Assert.assertTrue (JUnit4);

devOpsHazelcast · 2024-02-25T17:56:29Z

Can one of the admins verify this patch?

devOpsHazelcast · 2024-02-25T17:56:29Z

Can one of the admins verify this patch?

devOpsHazelcast · 2024-02-25T17:56:29Z

Can one of the admins verify this patch?

devOpsHazelcast · 2024-02-25T17:56:30Z

All committers have signed the CLA.

devOpsHazelcast · 2024-02-25T17:57:48Z

Internal PR hazelcast/hazelcast-mono#903
Internal message only. Nothing to see here, move along

JackPGreen · 2024-02-25T22:59:48Z

@dr-divago thanks for your contribution!

It's great you've addressed these issues - had you considered amending the test referenced in #26193 to see if further examples could be caught going forwards?

dr-divago · 2024-02-26T07:03:35Z

@JackPGreen Do you mean to check all the references of JUnit5 in the codebase? Or fixing com.hazelcast.NoMixedJUnitAnnotationsInOurTestSourcesTest.noJUnitMixing()?

JackPGreen · 2024-02-26T07:27:05Z

@JackPGreen Do you mean to check all the references of JUnit5 in the codebase? Or fixing com.hazelcast.NoMixedJUnitAnnotationsInOurTestSourcesTest.noJUnitMixing()?

I thought fixing the noJUnitMixing test to find other scenarios and ensure this doesn’t happen again.

… and Junit5 checks

dr-divago · 2024-02-29T07:15:01Z

Hi @JackPGreen
I have added chehck for assert in noJUnitMixing and including IT tests in the scanning

JackPGreen · 2024-02-29T09:30:35Z

The job Hazelcast-mono-OS-pr-builder of your PR failed. (Hazelcast internal details: build log, artifacts).
Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log file

--------------------------
---------SUMMARY----------
--------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:checkstyle (default) on project hazelcast-archunit-rules: An error has occurred in Checkstyle report generation. Failed during checkstyle execution: There are 2 errors reported by Checkstyle 9.3 with /home/jenkins/jenkins_slave/workspace/Hazelcast-mono-OS-pr-builder_2/hazelcast/checkstyle/checkstyle.xml ruleset. -> [Help 1]
--------------------------
---------ERRORS-----------
--------------------------
[ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-mono-OS-pr-builder_2/hazelcast/hazelcast-archunit-rules/src/main/java/com/hazelcast/test/archunit/MixTestAnnotationsCondition.java:24:17: Using the '.*' form of import should be avoided - org.junit.*. [AvoidStarImport]
--------------------------
[ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-mono-OS-pr-builder_2/hazelcast/hazelcast-archunit-rules/src/main/java/com/hazelcast/test/archunit/MixTestAnnotationsCondition.java:39:59: Name 'JUNIT_4_ClASS_ANNOTATION' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'. [ConstantName]
--------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:checkstyle (default) on project hazelcast-archunit-rules: An error has occurred in Checkstyle report generation. Failed during checkstyle execution: There are 2 errors reported by Checkstyle 9.3 with /home/jenkins/jenkins_slave/workspace/Hazelcast-mono-OS-pr-builder_2/hazelcast/checkstyle/checkstyle.xml ruleset. -> [Help 1]
--------------------------
[ERROR] 
--------------------------
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
--------------------------
[ERROR] 
--------------------------
[ERROR] For more information about the errors and possible solutions, please read the following articles:
--------------------------
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
--------------------------
[ERROR] 
--------------------------
[ERROR] After correcting the problems, you can resume the build with the command
--------------------------
[ERROR]   mvn  -rf :hazelcast-archunit-rules
--------------------------

… and Junit5 checks

dr-divago · 2024-02-29T20:35:10Z

Hi @JackPGreen
I updated the PR. How to access the build log?

JackPGreen · 2024-03-01T09:53:37Z

Hi @JackPGreen I updated the PR. How to access the build log?

It's an internal tool unfortunately, but all tests have passed :)

JackPGreen · 2024-03-01T09:54:29Z

...st-archunit-rules/src/main/java/com/hazelcast/test/archunit/MixTestAnnotationsCondition.java

+    private boolean hasJUnit5Assertions(JavaClass item) {
+        Set<JavaMethodCall> methodCalls = item.getMethodCallsFromSelf();
+        return methodCalls.stream().anyMatch(call -> call.getTarget().getFullName().contains("org.junit.jupiter.api.Assertions"));
+    }
+    private boolean hasJUnit4Assertions(JavaClass item) {
+        Set<JavaMethodCall> methodCalls = item.getMethodCallsFromSelf();
+        return methodCalls.stream().anyMatch(call -> call.getTarget().getFullName().contains("org.junit.Assert"));
+    }


nit - it's possible to centralise the logic into a single method that these can delegate to with different fullName parameters

JackPGreen · 2024-03-01T09:54:50Z

...st-archunit-rules/src/main/java/com/hazelcast/test/archunit/MixTestAnnotationsCondition.java

+    private boolean hasAnyJUnit4Annotations(JavaClass item) {
+        boolean hasJUnit4Annotation = item.getMethods().stream()
+                .anyMatch(method -> JUNIT_4_ANNOTATION_CLASSES.stream()
+                        .anyMatch(method::isAnnotatedWith));
+
+        boolean hasJUnit4ClassAnnotation = item.getAnnotations().stream()
+                .anyMatch(annotation -> JUNIT_4_CLASS_ANNOTATION.stream()
+                        .anyMatch(aa -> annotation.getRawType().isAssignableFrom(aa)));
+
+        return hasJUnit4Annotation || hasJUnit4ClassAnnotation;
    }


JackPGreen · 2024-03-01T09:55:20Z

...st-archunit-rules/src/main/java/com/hazelcast/test/archunit/MixTestAnnotationsCondition.java

+    }
+    private boolean hasAnyJUnit4Annotations(JavaClass item) {


Suggested change

}

private boolean hasAnyJUnit4Annotations(JavaClass item) {

}

private boolean hasAnyJUnit4Annotations(JavaClass item) {

Add some space between methods

JackPGreen · 2024-03-01T09:55:39Z

hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/SqlPrunedMemberLeftTest.java

Thanks for finding and fixing these!

JackPGreen · 2024-03-01T09:55:59Z

hazelcast/src/test/java/com/hazelcast/client/config/ClientTpcConfigTest.java

+
+

Suggested change

I don't think we need these extra blank lines?

dr-divago · 2024-03-06T08:36:30Z

Hi @JackPGreen is something missing in the PR?

JackPGreen · 2024-03-06T08:47:14Z

Hi @JackPGreen is something missing in the PR?

No, just waiting for a second reviewer.

devOpsHazelcast · 2024-03-28T16:27:22Z

This pull request has been closed because it was already closed as https://github.com/hazelcast/hazelcast-mono/pull/903

dr-divago · 2024-03-28T17:44:18Z

@JackPGreen why the PR was closed?

JackPGreen · 2024-03-28T17:48:16Z

@JackPGreen why the PR was closed?

It’s open again now, apologies :)

TomaszGaweda · 2024-04-08T18:26:48Z

hazelcast/src/test/java/com/hazelcast/internal/util/StringUtilTest.java

@@ -20,8 +20,8 @@
 import com.hazelcast.test.HazelcastTestSupport;
 import com.hazelcast.test.annotation.ParallelJVMTest;
 import com.hazelcast.test.annotation.QuickTest;
+import org.junit.Test;


In this form it won't work:

Junit 4 test must be public

We use also ParameterizedTest in this class

You are right, I miss it. Do you suggest rewrite the test with Parameterized runner?

Removed dependency org.junit.jupiter.api.Assertions.assertTrue (JUnit…

9437a3d

…5) using org.junit.Assert.assertTrue (JUnit4);

hz-devops-test added the Source: Community PR or issue was opened by a community user label Feb 25, 2024

Fix MixTestAnnotationsCondition to include IT tests and extend JUnit4…

a29a5dc

… and Junit5 checks

Fix MixTestAnnotationsCondition to include IT tests and extend JUnit4…

35f511f

… and Junit5 checks

JackPGreen suggested changes Mar 1, 2024

View reviewed changes

Addressing PR comments

125ad21

JackPGreen approved these changes Mar 1, 2024

View reviewed changes

devOpsHazelcast closed this Mar 28, 2024

JackPGreen reopened this Mar 28, 2024

TomaszGaweda suggested changes Apr 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed JUnit5 dependency for ClientTpcConfigTest #26265

Removed JUnit5 dependency for ClientTpcConfigTest #26265

dr-divago commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024 •

edited

devOpsHazelcast commented Feb 25, 2024

JackPGreen commented Feb 25, 2024

dr-divago commented Feb 26, 2024

JackPGreen commented Feb 26, 2024

dr-divago commented Feb 29, 2024

JackPGreen commented Feb 29, 2024

dr-divago commented Feb 29, 2024

JackPGreen commented Mar 1, 2024

JackPGreen Mar 1, 2024

JackPGreen Mar 1, 2024

JackPGreen Mar 1, 2024

JackPGreen Mar 1, 2024

JackPGreen Mar 1, 2024

dr-divago commented Mar 6, 2024

JackPGreen commented Mar 6, 2024

devOpsHazelcast commented Mar 28, 2024

dr-divago commented Mar 28, 2024

JackPGreen commented Mar 28, 2024

TomaszGaweda Apr 8, 2024

dr-divago Apr 10, 2024

Removed JUnit5 dependency for ClientTpcConfigTest #26265

Are you sure you want to change the base?

Removed JUnit5 dependency for ClientTpcConfigTest #26265

Conversation

dr-divago commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024

devOpsHazelcast commented Feb 25, 2024 • edited

devOpsHazelcast commented Feb 25, 2024

JackPGreen commented Feb 25, 2024

dr-divago commented Feb 26, 2024

JackPGreen commented Feb 26, 2024

dr-divago commented Feb 29, 2024

JackPGreen commented Feb 29, 2024

dr-divago commented Feb 29, 2024

JackPGreen commented Mar 1, 2024

JackPGreen Mar 1, 2024

Choose a reason for hiding this comment

JackPGreen Mar 1, 2024

Choose a reason for hiding this comment

JackPGreen Mar 1, 2024

Choose a reason for hiding this comment

JackPGreen Mar 1, 2024

Choose a reason for hiding this comment

JackPGreen Mar 1, 2024

Choose a reason for hiding this comment

dr-divago commented Mar 6, 2024

JackPGreen commented Mar 6, 2024

devOpsHazelcast commented Mar 28, 2024

dr-divago commented Mar 28, 2024

JackPGreen commented Mar 28, 2024

TomaszGaweda Apr 8, 2024

Choose a reason for hiding this comment

dr-divago Apr 10, 2024

Choose a reason for hiding this comment

devOpsHazelcast commented Feb 25, 2024 •

edited