Feature/upgrade gradle to 8 #125

l-1squared · 2023-07-28T11:04:32Z

No description provided.

klassm

Please also squash commits / bring them a bit into order.

klassm · 2023-07-30T15:27:37Z

src/main/resources/META-INF/plugin.xml


    <description>Provides support for navigation between JGiven scenario states.</description>
    <change-notes>
        The plugin now supports IntelliJ version 2022.2.
    </change-notes>
    <vendor email="matthias.klass@tngtech.com" url="http://www.tngtech.com">TNG Technology Consulting GmbH</vendor>

-    <idea-version since-build="163.6110.112"/>


Why did you increase this? Do you know use APIs only available since then?

I guess so. The IntelliJ plugin basicly stated in the build that it would increase the number, and I thought, well, then we can also increase it manually. I'd wager that the change in question was the addition to the constructor

klassm · 2023-07-30T15:28:09Z

src/test/java/com/tngtech/jgiven/reference/ScenarioStateUsageProviderIntegrationTest.kt

@@ -1,64 +0,0 @@
-package com.tngtech.jgiven.reference


Can we make this test work again? Are you sure this test never worked?

It failed on master. I can double check if you want to

Or to be more precise, it was not exectuted on master, and when I made it, it failed

Still it would be nice to have the test again.

Agree with you. But as I detailed below, I have no working system to Jump off of, and I am not knowledgeable enough to rewrite this test from scratch.

klassm · 2023-07-30T15:28:32Z

.github/workflows/build.yml

      with:
-        java-version: 11
+        java-version: 17
+        distribution: 'adopt'
    - name: Build with Gradle
      run: ./gradlew buildPlugin test


At least here tests were executed...

./gradlew buildPlugin test

so, I guess we're again talking about the ScenarioUserProviderTest...
I captured a screenshot from master, to show that this particular test is in fact not picked up:

If I annotate with @Test the following happens:

I guess we have a bug somewhere in the software responsible for picking up those tests, because one annotation sufficed to activate all four tests.
I tried this with JVM 11 as the backing machine. (JVM -8 seemed to be already incompatible). In any case, the issue with this test is that the LibraryTestUtil cannot find the source code location:

My guess is, however, that this is an important part of the test and I have absolutely no idea on how to replicate such a feature. If you can be of any help here, I would greatly appreciate it.

l-1squared · 2023-08-01T04:27:33Z

Please also squash commits / bring them a bit into order.

I would like to ask for some guidance here; I can squash them, but we are doing three somewhat connected updates here so that may not be a good idea. And in my opinion, they are in the order of "update something; try to fix all complaints; update something else". I don't no what kind of order you would prefer.

klassm · 2023-08-01T05:44:20Z

I would like to ask for some guidance here; I can squash them, but we are doing three somewhat connected updates here so that may not be a good idea. And in my opinion, they are in the order of "update something; try to fix all complaints; update something else". I don't no what kind of order you would prefer.

Well there are various infrastructure upgrades, that can be grouped. I guess we don't need three commits updating Gradle, do we?
Same goes for the actual implementation - Remove deprecations, Update since field, Update intellij plugin somewhat belong together, don't they? So this is rather not about what the commit is doing than about what the intention is.

l-1squared · 2023-08-02T04:44:02Z

I would like to ask for some guidance here; I can squash them, but we are doing three somewhat connected updates here so that may not be a good idea. And in my opinion, they are in the order of "update something; try to fix all complaints; update something else". I don't no what kind of order you would prefer.

Well there are various infrastructure upgrades, that can be grouped. I guess we don't need three commits updating Gradle, do we? Same goes for the actual implementation - Remove deprecations, Update since field, Update intellij plugin somewhat belong together, don't they? So this is rather not about what the commit is doing than about what the intention is.

The updates are not quite independent though. For instance, the newest intellij Plugin talks about Java 17 requirements and I think the old kotlin version (or was it the old intelliJ plugin) was incompatible with Gradle-8. So I cannot reorder or squash commits partially.

I can, if you wish, squash everything into one commit, and name it "Grand build system refurbishment"

According to the docs, the supplier is for a screen reader so I figured that giving the supplier the text value would be helpful. Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

apparently these tests weren't executed on master but for a reason I don't understand, they are picked up with a newer gradle version. Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

…anyway Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

required by IntelliJ anyway. Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Signed-off-by: Christian Oertel <christian.oertel@tngtech.com> Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

According to my (limited) understanding, without adding JGiven as a module the tests depending on it will not function. Therefore, continuing without a library is nonsensical. Furthermore if JGiven is supposed to be present it must be present as a jar (because tha's what we publish), therefore there is no use in trying maybe add Jgiven as a collection of class files (this should only be the case if we were in the same project) Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

l-1squared requested review from klassm and fudler July 28, 2023 11:04

l-1squared force-pushed the feature/upgrade_gradle_to_8 branch 2 times, most recently from f7df931 to 91f91b2 Compare July 28, 2023 12:14

klassm requested changes Jul 30, 2023

View reviewed changes

fudler force-pushed the feature/upgrade_gradle_to_8 branch 2 times, most recently from 4e0bcce to 2d09f77 Compare August 18, 2023 14:45

l-1squared and others added 15 commits August 25, 2023 08:25

Respond to deprecation in IntelliJ Constructor

3b60972

According to the docs, the supplier is for a screen reader so I figured that giving the supplier the text value would be helpful. Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Delete test that never worked

e47b5a4

apparently these tests weren't executed on master but for a reason I don't understand, they are picked up with a newer gradle version. Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Upgrade gradle to version 7.

ff74448

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Remove deprecations

8d66a3b

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Disable "no searchable options warning"

9f84e3c

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Update intellij plugin to be compatible with gradle 8

4ac1e2b

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Update the jvm plugin to 1.9.0

58e0e95

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Update gradle to 8.2.1

9d4a7a5

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Update "since" field in plugin.xml to the version that intellij sets …

18c12fc

…anyway Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Update github actions

3a548e2

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Update IntelliJ Plugin to version 1.15

305e2c6

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Update Target Java Version to 17

c47e2a2

required by IntelliJ anyway. Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Restore failing testclasses

81d4ee1

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

Fix failed tests

c85fef7

Signed-off-by: Christian Oertel <christian.oertel@tngtech.com> Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

l-1squared force-pushed the feature/upgrade_gradle_to_8 branch from 2d09f77 to fff9ef7 Compare August 25, 2023 06:25

fudler requested a review from klassm September 1, 2023 13:05

klassm merged commit 0c0c687 into master Sep 15, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/upgrade gradle to 8 #125

Feature/upgrade gradle to 8 #125

l-1squared commented Jul 28, 2023

klassm left a comment

klassm Jul 30, 2023

l-1squared Aug 1, 2023

klassm Jul 30, 2023

l-1squared Aug 1, 2023

l-1squared Aug 1, 2023

klassm Aug 1, 2023

l-1squared Aug 2, 2023

klassm Jul 30, 2023

l-1squared Aug 1, 2023

klassm Aug 1, 2023

l-1squared Aug 2, 2023

l-1squared commented Aug 1, 2023

klassm commented Aug 1, 2023

l-1squared commented Aug 2, 2023

Feature/upgrade gradle to 8 #125

Feature/upgrade gradle to 8 #125

Conversation

l-1squared commented Jul 28, 2023

klassm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l-1squared commented Aug 1, 2023

klassm commented Aug 1, 2023

l-1squared commented Aug 2, 2023