Skip to content

Handle older versions of Gradle #149

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 19, 2024
Merged

Handle older versions of Gradle #149

merged 1 commit into from
Jun 19, 2024

Conversation

Arthurm1
Copy link
Contributor

@Arthurm1 Arthurm1 commented Jun 7, 2024

I had some issues with older versions of Gradle not working properly so I've changed each test in GradleBuildServerPluginTest to take gradle version as a parameter and run the test with that version.

I may have gone overboard in number of versions that are tested. They're all the ones I could see that the various checks in the code test for. The downside is that the plugin tests take a lot longer although it's good to know all versions work.

Adding these tests showed up a number of things that don't work in older versions that I've fixed in this PR...

  1. Retrieving source sets in general has had to change to work for all versions and on non-java languages.
  2. Scala compiler options changed between Gradle versions and it's not compatible so I've created the options directly which has the advantage of not using a bunch of internal classes.
  3. Scala source dirs weren't found on old versions of Gradle
  4. The module dependencies weren't populated on old versions of Gradle
  5. Test tasks weren't found on old versions of Gradle
  6. Source output dirs weren't populated on old versions
  7. We now test versions 2.12 to 8.8 (current)

What is the earliest version that should be supported?

@jdneo
Copy link
Member

jdneo commented Jun 11, 2024

Thank you for this work! Though the CI takes longer but it help us find a lot of regressions so it's worth doing.

Just curious how are those Gradle versions picked? Is there any rule that we can follow in the future?

@Arthurm1
Copy link
Contributor Author

It was a bit painful - I tried a few random versions at first and then tests started failing so I had to read through release notes to find what version caused the API change and kept that version in the test list. The release notes are all here

I think we should just keep the latest version in the list up to date. Currently it's 8.8, next we should change it to 8.9. Whenever the tests fail on a new version because of API change then we should keep that version in the list forever.

@jdneo
Copy link
Member

jdneo commented Jun 11, 2024

I think we should just keep the latest version in the list up to date. Currently it's 8.8, next we should change it to 8.9. Whenever the tests fail on a new version because of API change then we should keep that version in the list forever.

This makes sense!

BTW, do you think it worth using the Gradle TestKit for the cross version test purpose? I haven't used it before but looking at the document looks like it's a recommended way to do so?

@jdneo
Copy link
Member

jdneo commented Jun 12, 2024

do you think it worth using the Gradle TestKit for the cross version test purpose?

BTW, this is just an ask, if the testkit is fit for our scenario, it's not necessary to do so in this PR. We can file it as a engineering debt and address it in the future.

@Arthurm1
Copy link
Contributor Author

@jdneo I'm not sure. It may be easier to debug - at the moment I just add the jvm options and attach to the process

@jdneo jdneo mentioned this pull request Jun 12, 2024
Open
@jdneo
Copy link
Member

jdneo commented Jun 12, 2024

Tracked by #151.

@Arthurm1 Arthurm1 force-pushed the multi_version_tests branch from da34bf4 to 6580779 Compare June 14, 2024 15:51
@Arthurm1 Arthurm1 force-pushed the multi_version_tests branch from 6580779 to 013a0b4 Compare June 14, 2024 15:55
jdneo
jdneo reviewed Jun 15, 2024
View reviewed changes
plugin/src/main/java/com/microsoft/java/bs/gradle/plugin/ScalaLanguageModelBuilder.java
import org.gradle.api.file.SourceDirectorySet;
import org.gradle.api.internal.tasks.scala.MinimalScalaCompileOptions;
Copy link
Member

Choose a reason for hiding this comment

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

👍

@jdneo jdneo added this to the 0.3.0 milestone Jun 15, 2024
@jdneo jdneo added bug Something isn't working engineering Engineering tasks like refactoring, build infra, etc... labels Jun 15, 2024
jdneo
jdneo approved these changes Jun 19, 2024
View reviewed changes
Copy link
Member

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

LGTM

@jdneo jdneo merged commit 7a5e6e7 into microsoft:develop Jun 19, 2024
4 checks passed
@jdneo
Copy link
Member

jdneo commented Jun 19, 2024

Thank you @Arthurm1

@Arthurm1 Arthurm1 deleted the multi_version_tests branch June 22, 2024 19:29
Jiaaming pushed a commit to Jiaaming/build-server-for-gradle that referenced this pull request Jul 1, 2024
@Arthurm1
fix - Handle older versions of Gradle (microsoft#149)
bd22559 
Co-authored-by: Arthur McGibbon <[email protected]>
Jiaaming pushed a commit to Jiaaming/build-server-for-gradle that referenced this pull request Jul 1, 2024
@jdneo
fix - Only send build target count during initialization (microsoft#153)
2ceb4de 
init named pipe

test build
Server named pipe connection

update named pipe stream solution

finish forward message

finish merge and named pipe

fix - Handle older versions of Gradle (microsoft#149)

Co-authored-by: Arthur McGibbon <[email protected]>

fix - Add JDK 22 compatibility support (microsoft#152)

Signed-off-by: Sheng Chen <[email protected]>

fix - Return the root cause of the message (microsoft#156)

feat - Add support for running tests (microsoft#144)

Co-authored-by: Arthur McGibbon <[email protected]>
Co-authored-by: Sheng Chen <[email protected]>

add Launcher by namedPipe  with backwards compatible
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdneo jdneo jdneo approved these changes

Assignees
No one assigned
Labels
bug Something isn't working engineering Engineering tasks like refactoring, build infra, etc...
Projects
None yet
Milestone
0.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Arthurm1 @jdneo