fix #18465 and upgrade other junit4 tests to junit5 in mustache files

[thorstenhirsch]

While #18465 is only about the native client, I've also upgrade JUnit4 to JUnit5 in all other *.mustache files. I'm not sure which api_tests should generate disabled tests, but I couldn't figure out a pattern, and when temporarily activating a disabled test it threw an error, so I decided to keep disabled/enabled tests as they were.

mvn clean test runs successful on my employer's Windows 10 machine, but I wouldn't completely trust ./bin/generate-samples.sh ./bin/configs/*.yaml and ./bin/utils/export_docs_generators.sh on this machine, so please check that again in your environment.

Also I don't know what to do about "update samples", so I marked this point with a question mark. Please tell me if I have further todos.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• (partially) Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh ./bin/configs/*.yaml
  ./bin/utils/export_docs_generators.sh
  

  (For Windows users, please run the script in Git BASH)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request. @martin-mfg @wing328

[wing328]

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

[wing328]

thanks for the PR. i did a test with java (apache-httpclient) with petstore spec but got the following errors:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:testCompile (default-testCompile) on project openapi-java-client: Compilation failure: Compilation failure:
[ERROR] C:\Users\wing3\AppData\Local\Temp\test-apache-httpclient\src\test\java\org\openapitools\client\model\CategoryTest.java:[21,28] error: package org.junit.jupiter.api does not exist
[ERROR] C:\Users\wing3\AppData\Local\Temp\test-apache-httpclient\src\test\java\org\openapitools\client\model\CategoryTest.java:[22,28] error: package org.junit.jupiter.api does not exist
[ERROR] C:\Users\wing3\AppData\Local\Temp\test-apache-httpclient\src\test\java\org\openapitools\client\model\CategoryTest.java:[23,28] error: package org.junit.jupiter.api does not exist
[ERROR] C:\Users\wing3\AppData\Local\Temp\test-apache-httpclient\src\test\java\org\openapitools\client\api\StoreApiTest.java:[18,28] error: package org.junit.jupiter.api does not exist
[ERROR] C:\Users\wing3\AppData\Local\Temp\test-apache-httpclient\src\test\java\org\openapitools\client\api\StoreApiTest.java:[19,28] error: package org.junit.jupiter.api does not exist
[ERROR] C:\Users\wing3\AppData\Local\Temp\test-apache-httpclient\src\test\java\org\openapitools\client\api\StoreApiTest.java:[20,28] error: package org.junit.jupiter.api does not exist
[ERROR] C:\Users\wing3\AppData\Local\Temp\test-apache-httpclient\src\test\java\org\openapitools\client\api\StoreApiTest.java:[32,1] error: cannot find symbol
[ERROR]   symbol: class Disabled

can you please take a look when you've time?

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g java -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/test-apache-httpclient --library apache-httpclient

[thorstenhirsch]

Alright... I've fixed it, but I'm afraid this has become hell of a commit. Yesterday I forgot about the pom templates, so I started upgrading the dependencies to junit5 there. But then I also upgraded the complete project to junit5 (at least I think so).

I've generated the petstore with both, the apache-httpclient and the native client. Both seem to be fine now. But there's one problem I couldn't manage to fix: mvn test hangs when processing openapi-generator-online at this point:

b507e7f1-1ca3-4c08-b201-a0ed57a46ea3, /tmp/codegen-tmp4728575640856685992/java-client-bundle.zip
looking for fileId b507e7f1-1ca3-4c08-b201-a0ed57a46ea3
got filename /tmp/codegen-tmp4728575640856685992/java-client-bundle.zip

I guess this change could be the problem:

diff --git a/modules/openapi-generator-online/src/test/java/org/openapitools/codegen/online/api/GenApiControllerTest.java b/modules/openapi-generator-online/src/test/java/org/openapitools/codegen/online/api/GenApiControllerTest.java
index 8df33829c9..4901cdc3ce 100644
--- a/modules/openapi-generator-online/src/test/java/org/openapitools/codegen/online/api/GenApiControllerTest.java
+++ b/modules/openapi-generator-online/src/test/java/org/openapitools/codegen/online/api/GenApiControllerTest.java
@@ -1,13 +1,14 @@
 package org.openapitools.codegen.online.api;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import org.junit.Test;
-import org.junit.runner.RunWith;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.openapitools.codegen.online.model.ResponseCode;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
+import org.springframework.test.context.junit.jupiter.SpringExtension;
 import org.springframework.test.context.junit4.SpringRunner;
 import org.springframework.test.web.servlet.MockMvc;
 
@@ -18,7 +19,7 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
 
-@RunWith(SpringRunner.class)
+@ExtendWith(SpringExtension.class)
 @WebMvcTest(GenApiController.class)
 public class GenApiControllerTest {

This should've been the way to migrate a JUnit4 test based on SpringRunner to JUnit5's SpringExtension. Oh, I forgot to remove the old import of org.springframework.test.context.junit4.SpringRunner, which is now no longer necessary. I'll push another commit removing it in a second... but I guess this is not the reason for why the openapi-generator-online test is stuck.

[wing328]

https://github.com/OpenAPITools/openapi-generator/actions/runs/8816015528/job/24232484564?pr=18470 reported some errors.

please take a look when you've time

[wing328]

at this stage would it be possible to file a PR just to update the java client mustache templates to use junit5?

we can get that merge first and then tackle the remaining junit tests one by one.

(we prefer smaller PRs for easier review and merge)

[thorstenhirsch]

I've made a stupid search&replace yesterday, which replaced way too much. I think now I've fixed it. But there's still a problem in the Scala generator... I know this PR has become huge, but I think it's 98% finished by now.

[thorstenhirsch]

I'm afraid I don't know enough Scala to fix this (probably last) issue. When I'd chosen the wrong Scala version in the dependencies I was getting the same errors as ci/circle currently has (compile errors due to missing symbols). But on my local machine compilation works, all dependencies use Scala 2.12. But the tests still fail due to http rc 301:

[INFO] Running PetApiTest
[ERROR] Tests run: 3, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 0.921 s <<< FAILURE! - in PetApiTest
[ERROR] PetApiTest.PetApi should add and fetch a pet  Time elapsed: 0.003 s  <<< ERROR!
org.openapitools.client.core.ApiError: (301) Unexpected response code. Content : HttpEntity.Strict(text/html,134 bytes total)

[ERROR] PetApiTest.PetApi should update a pet  Time elapsed: 0 s  <<< ERROR!
org.openapitools.client.core.ApiError: (301) Unexpected response code. Content : HttpEntity.Strict(text/html,134 bytes total)

[ERROR] PetApiTest.PetApi should find pets by status  Time elapsed: 0 s  <<< ERROR!
org.openapitools.client.core.ApiError: (301) Unexpected response code. Content : HttpEntity.Strict(text/html,134 bytes total)

Apr 26, 2024 12:44:40 AM org.scalatestplus.junit5.ScalaTestEngine $anonfun$execute$1
INFO: Completed execution of suite class PetApiTest.
Apr 26, 2024 12:44:40 AM org.scalatestplus.junit5.ScalaTestEngine execute
INFO: Completed tests execution.

I can't figure out how to tell Akka's http client to follow the redirect. There's still an open issue for this feature since 2016: akka/akka-http#195

[bijancn]

@thorstenhirsch just a wild guess but why did you change the scala-collection-compat from 2.4.1 to 2.12.0?

[thorstenhirsch]

Because of these warnings:

[WARNING] Scala library detected 2.12.13 doesn't match scala.compat.version : 2.4.1
[WARNING]  Expected all dependencies to require Scala version: 2.4.1
[WARNING]  org.openapitools:scala-akka-petstore-client:1.0.0 requires scala version: 2.12.13
[WARNING] Multiple versions of scala libraries detected!

Well, the warnings are still there, but 2.12.0 is closer to 2.12.13, right? ;-)

[WARNING] Scala library detected 2.12.13 doesn't match scala.compat.version : 2.12.0
[WARNING]  Expected all dependencies to require Scala version: 2.12.0
[WARNING]  org.openapitools:scala-akka-petstore-client:1.0.0 requires scala version: 2.12.13
[WARNING] Multiple versions of scala libraries detected!

Looks like the expectation (2nd line) is always the version of scala-collection-compat, but I can't find a matching version for all components, because...

• with 2.4.1 there's no Scala 2.4.x (anymore?)
• with 2.12.13 there's no scala-collection-compat 2.12.13 (the most recent one is 2.12.0)
• with 2.12.0 there's a new warning popping up:

[WARNING]  com.typesafe.akka:akka-actor_2.12:2.6.12 requires scala version: 2.12.11

So I thought with Scala 2.12.13 and scala-collection-compat 2.12.0 the versions differ the least.

[bijancn]

I'm just wondering why changes to the Scala libraries are necessary as you were only fixing JUnit here which is a Java library? So I would try to avoid touching anything else?

[thorstenhirsch]

So with Scala 2.13 the petstore test compiles, but the test fails. First it failed, because http://petstore.swagger.io redirects to https and the test doesn't follow redirects. And when the test makes use of https directly it fails, because the server blocks https connections from CircleCI? I have no idea why all these problems only arise now after migrating to junit5.

[wing328]

@thorstenhirsch thanks again for the PR.

as this PR now changes 1300+ files, which is not easy to review (the file diff page is taking a long time to load).

what about filing separate PRs for each generator/library (e.g. resteasy, resttemplate) as smaller PRs will get merged easier and quicker?

[thorstenhirsch]

Like so? @wing328

#18615
#18616
#18617
#18618
#18619
#18620

[thorstenhirsch]

@wing328 Now I finally understand why changing samples directly doesn't make sense. May I suggest to add something like the following sentence to the contributing guide in the section #code-generators?

After modifying a generator you should run bin/generate-samples.sh in order to re-generate the samples,
because they always need to match what the generators produce. This is being checked in the CI pipeline.

[bijancn]

I think that is basically in the "PR checklist" but probably can't hurt to repeat 😅. Thank you for all the hard work @thorstenhirsch !