Skip to content
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

Rewrite more Groovy to Java #1470

Open
marcingrzejszczak opened this issue Aug 13, 2020 · 30 comments · Fixed by #1471
Open

Rewrite more Groovy to Java #1470

marcingrzejszczak opened this issue Aug 13, 2020 · 30 comments · Fixed by #1471

Comments

@marcingrzejszczak
Copy link
Contributor

marcingrzejszczak commented Aug 13, 2020

The rationale

We're looking for help in rewriting all possible production code from Groovy to Java. The Spock tests will remain written in Groovy, however ideally we'd prefer not to have Groovy in any library production code except for the DSL as such.

It's been an ongoing process for years now. The main reason is the compatibility. Gradle comes with a bundled version of groovy and we have a Gradle plugin. Also we depend on groovy version in spring boot. That's all problematic.

It's discouraging for some part of the community to write a fix or a new feature in Groovy rather than in Java.

IDE support for groovy is getting worse every year now. Intellij IDEA introduces new bugs and other IDEs either don't work at all or are even worse.

What do we want to rewrite?

I'd suggest going through all the modules outside of the spring-cloud-contract-spec modules and picking production class after class and rewriting it to java from groovy, running the tests and if the tests work make a commit.

As i said we want to leave the Groovy DSL (groovy-spec module) and the spock tests.

How to start?

Pick a class / package / module, notify the others in a comment and start converting the classes! Once you're done file a pull request and link it to this issue.

Checking if code works

Before you file a pull request you can run

$ ./mvnw clean install -Pintegration,docs

That way you'll also run the standalone samples that simulate end to end tests. It will also build the documentation and check if the docs aren't broken.

Please note that if you rewrite a class that contains such tags as

// tag::foo[]
// end::foo[]

that means that most likely that the code within those comments is used in the documentation.

@armsargis
Copy link

Hi, Groovy DSL should be removed also?

@santfirax
Copy link
Contributor

Hello, @marcingrzejszczak I would love to help in this, could it be possible that you provide some guidance for starters?

@marcingrzejszczak
Copy link
Contributor Author

Groovy DSL remains.

I'd suggest going through all the modules outside of the spring cloud contract spec modules and picking production class after class and rewriting it to java from groovy, running the tests and if the tests work make a commit.

As i said we want to leave the groovy spec and the spock tests.

@saket88
Copy link

saket88 commented Aug 14, 2020

I want to contribute

@marcingrzejszczak
Copy link
Contributor Author

Be my guest. Pick a class or a package and create a pull request where you migrate it to java!

@stessy
Copy link
Contributor

stessy commented Aug 14, 2020

I can help as well.

I'm currently rewriting the module spring-cloud-contract-converters.

@marcingrzejszczak
Copy link
Contributor Author

Fantastic!

@shanman190
Copy link
Contributor

I'm still trying to work through the Gradle plugin pieces also.

@marcingrzejszczak
Copy link
Contributor Author

@ankurongit You can pick a module / class that has not been worked on and put a note here that you're working on it and then link a pull request.

@shanman190 fabulous! I wanted to port whole groovy code from the plugin to java!

@zhmaeff
Copy link
Contributor

zhmaeff commented Aug 16, 2020

@marcingrzejszczak I'd like to work on spring-cloud-contract-verifier module.

@marcingrzejszczak
Copy link
Contributor Author

To everyone willing to help on this issue!

If you haven't already, pick a class / package / module, notify the others in a comment and start converting the classes! Once you're done file a pull request and link it to this issue. If you've picked a module / class / package, I'll add an 👍 emoji to notify that it's a good idea. In any other case I'll leave a comment.

Good luck and thank you so much for your help! :)

BTW I've updated the issue's description. If you think anything else could be put there that could be helpful to others, leave a comment and I'll update it.

@stessy
Copy link
Contributor

stessy commented Aug 16, 2020

Hi @marcingrzejszczak ,

I cannot run a full clean install. I have multiple modules for which the tests fail. In fact 2 modules so far (spring-cloud-contract-verifier and spring-cloud-contract-stub-runner).
On the other hand the spring-cloud-contrat-converters, the one I rewrite, build without any problems.
Don't really know if the rewriting (WireMockToDslConverter) could impact other modules tests.
Here is the link to the rewritten class: stessy@eb79ae8

So, what can I do ?

@marcingrzejszczak
Copy link
Contributor Author

marcingrzejszczak commented Aug 16, 2020

@stessy you need to provide some sort of a stacktrace or sth why the build is failing. It builds fine in Jenkins and CircleCI. If I were you I'd debug if the changes have any impact.

@stessy
Copy link
Contributor

stessy commented Aug 16, 2020

@marcingrzejszczak I think the problem is related to the Temp folder in Windows. For some tests I have an AccessDeniedException when trying to delete recursively a git repo. And some tests fail as well with the AetherStubDownloader. :-(
I'm gonna try on my mac.

@marcingrzejszczak
Copy link
Contributor Author

To everyone willing to help on this issue!

If you haven't already, pick a class / package / module, notify the others in a comment and start converting the classes! Once you're done file a pull request and link it to this issue. If you've picked a module / class / package, I'll add an 👍 emoji to notify that it's a good idea. In any other case I'll leave a comment.

Good luck and thank you so much for your help! :)

BTW I've updated the issue's description. If you think anything else could be put there that could be helpful to others, leave a comment and I'll update it.

@stessy
Copy link
Contributor

stessy commented Aug 18, 2020

@marcingrzejszczak The build fails due to security checks failures. Can you tell me what are the problems ?

@marcingrzejszczak marcingrzejszczak linked a pull request Aug 18, 2020 that will close this issue
marcingrzejszczak pushed a commit that referenced this issue Aug 18, 2020
* Rewrite WireMockToDslConverter to Java

* Fixes imports and code formatting

part of gh-1470
marcingrzejszczak pushed a commit that referenced this issue Aug 20, 2020
* Convert groovy to java (gh-1470)

Co-authored-by: Anatolii Zhmaiev <Anatolii_Zhmaiev@epam.com>
@stessy
Copy link
Contributor

stessy commented Aug 23, 2020

Hi,

I have a question.

What's the plan with Closures and GString ?
I'm rewriting the spring cloud contract pact module. But some part of the code refers to Closures outside the module.
I manage to get rid of local closures (inside the pact module) by converting them to Function, but I'm stuck with the ones implemented outside the module ( i.e: ContentUtils in the spring-cloud-contract-verifier module).

Thanks for your help.

@marcingrzejszczak
Copy link
Contributor Author

Yeah... so GString is a valid thing inside a Groovy based contract so we need to support it. As for ContentUtils I think I've already started introducing Functions instead of Closures there. So Closures can be rewritten to Functions IMO.

@stessy
Copy link
Contributor

stessy commented Sep 28, 2020

Hi there,

can you tell me what's going on with checkstyle complaining about missing javadoc on line 30 ? Do I have to document each variable in the enum ?

Thanks for your help.

@marcingrzejszczak
Copy link
Contributor Author

Yeah, a javadoc is necessary over that enum element.

@stessy
Copy link
Contributor

stessy commented Oct 3, 2020

Hi there,

what's the current status of the spring-cloud-contract-verifier module?
Is there someone working on it ? @zhmaeff I see that you started the rewrite. Dunno where you arrived until now.
Can I help for that module ? If yes, which classes/packages not yet rewritten locally ?

@zhmaeff
Copy link
Contributor

zhmaeff commented Oct 3, 2020

Hi @stessy!
You can pick it up) All migrations from my side have been merged

@stessy
Copy link
Contributor

stessy commented Oct 3, 2020

Hi @zhmaeff ,

OK.

Thanks for your prompt reply.

@stessy
Copy link
Contributor

stessy commented Oct 26, 2020

Hi there,

there was a PR to convert SpringCloudContractAssertions from java to groovy: #1110.

Can we now rewrite the groovy class to java? This means the CollectionAssertSpec groovy class has to be removed as well. But the class CollectionAssertTests covers the same tests.

@marcingrzejszczak
Copy link
Contributor Author

@stessy hmm there was a reason why it was written in Groovy, however I don't remember what that reason was ;) Can you check that once you rewrite this from groovy to java all the tests including the standalone ones are still passing? :P

@stessy
Copy link
Contributor

stessy commented Oct 27, 2020

@marcingrzejszczak The tests in CollectionAssertSpec groovy class which test the SpringCloudContractAssertions all fail. That's the reason why I deleted that test class too as the CollectionAssertTests covers the same tests for the java class. Now I can rollback my change and keep the groovy class.

@santfirax
Copy link
Contributor

Hello @marcingrzejszczak I would like to work on BodyExtractor.groovy in spring-cloud-contract-verifier. I also noticed that I need ObjectMapperFactory class in spring-cloud-contract-pact module, so, can I add that dependency into spring-cloud-contract-verifier module ?

@marcingrzejszczak
Copy link
Contributor Author

hey @santfirax , I'd definitely prefer you to copy that class and not rely on the pact module.

@orbfish
Copy link

orbfish commented Nov 15, 2021

This is sad - I was considering using this project until I saw you were reverting it back to Java. "It's discouraging for some part of the community to write a fix or a new feature in Groovy rather than in Java." Speaking as a Java developer who uses Groovy for testing and builds - people are going to have to move to more modern languages at some point!

@shanman190
Copy link
Contributor

shanman190 commented Nov 15, 2021

@orbfish, this is specifically centering around the internal components of Spring Cloud Contract. The available DSLs are being left as they exist today as interfaces for end users to utilize the project.

@marcingrzejszczak's statement was particularly aimed at contributors to Spring Cloud Contract itself. By having a number of pieces that were written in Groovy or Kotlin, it makes it much more difficult for contributions to the Spring Cloud Contract project itself as the contributor very well may have to know how to write, and the idioms therein, in multiple source languages.

There were additionally a lot of bugs that surfaced up via Spring Cloud Contract related specifically to which version of Groovy or Kotlin Spring Cloud Contract provided that then conflicted with what the user had as a dependency. Eg. Groovy 2.5 VS Groovy 3.0 or Kotlin 1.3 VS Kotlin 1.4

The point is that you can absolutely write your contracts in Groovy DSL and Spock as that feature will not be going away anytime soon.

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

Successfully merging a pull request may close this issue.

9 participants