search

microsoft / build-server-for-gradle

An implementation of the Build Server Protocol for Gradle
MIT License
51 stars 8 forks source link

Handle older versions of Gradle #149

Closed Arthurm1 closed 3 months ago

Arthurm1 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

Tracked by https://github.com/microsoft/build-server-for-gradle/issues/151.

jdneo commented 3 months ago

Thank you @Arthurm1