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

Improve build/test infrastructure to support IDEA 2022.2/2022.3 and Java 17 #113

Merged
merged 3 commits into from
Nov 13, 2022

Conversation

unshare
Copy link
Contributor

@unshare unshare commented Oct 25, 2022

  • Use Java 11 for <2022.2 and Java 17 for >=2022.2
  • Upgrade Gradle to 7.5.1 for Java 17 and fixup build script accordingly
  • Upgrade intellij-gradle-plugin for IDEA >=2022.2
  • Remove Kotlin (not actually used, but messes with classpath)
  • Rearrange stuff around JUnit
  • Fixup several build warnings

…ava 17

* Use Java 11 for <2022.2 and Java 17 for >=2022.2
* Upgrade Gradle to 7.5.1 for Java 17 and fixup build script accordingly
* Upgrade `intellij-gradle-plugin` for IDEA >=2022.2
* Remove Kotlin (not actually used, but messes with classpath)
* Rearrange stuff around JUnit
* Fixup several build warnings
@unshare
Copy link
Contributor Author

unshare commented Oct 25, 2022

@filiphr, please take a look.
We've got serious breakages ahead with the 2022.3 release: 25/164 test cases fail.
Sorry to upgrade Gradle build -- I had to do it actually have the tests run.
Gradle 6.9.1 won't run on Java 17 the way intellij-gradle-plugin works.
223.6646.99-EAP-SNAPSHOT is added because it's less b0rken than LATEST on my machine.

@filiphr
Copy link
Member

filiphr commented Nov 12, 2022

Thanks a lot for your work on this @unshare. I'll have a look at the failing tests on 2022.3. I don't think that the failures are that big, it is only some small text formatting changes that have been done in IntelliJ

* Change use of report to non deprecated property
* Do not add tail text for lookups
* Adapt tests to use message from IntelliJ instead of manually crafted
@filiphr
Copy link
Member

filiphr commented Nov 12, 2022

@unshare I checked this out locally and I've fixed some of the problems in the tests.

By the way, what kind of a machine do you have? I have an Intel Mac and I couldn't run the tests. I had to use a snapshot version of the Gradle IntelliJ Plugin to run the tests properly. The build is also green for me when using LATEST-EAP-SNAPSHOT. I think that we can remove 223.6646.99-EAP-SNAPSHOT from the GItHub matrix build

* remove testing against 223.6646.99 as it outlived its purpose
* rename a variable
@unshare
Copy link
Contributor Author

unshare commented Nov 13, 2022

Great! You did manage to get a green checkmark.
As for my machine and my test issues, I predominantly run Windows desktops both at work and at home.
And I tend to agree the issues I experience are Windows-specific.

  1. There's a lot of lamenting on JNA not available for a new feature: using OS-native certificates. It litters logs yet is completely harmless for our use case.
  2. There's a development around file system implementation that is definitely Windows-specific: JetBrains/intellij-community@9793bb4
    It triggers errors like these: java.lang.IllegalAccessError: class com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl (in unnamed module @0x5f4da5c3) cannot access class sun.nio.fs.BasicFileAttributesHolder (in module java.base) because module java.base does not export sun.nio.fs to unnamed module @0x5f4da5c3
    223.6646.99 just happens to be the last available build without it.
    This issue does affect us as we do file access directly or indirectly.
Stack trace for 223 EAP follows, but beware `master` is already pretty much refactored around this
    java.lang.IllegalAccessError: class com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl (in unnamed module @0x5f4da5c3) cannot access class sun.nio.fs.BasicFileAttributesHolder (in module java.base) because module java.base does not export sun.nio.fs to unnamed module @0x5f4da5c3
        at com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl.listWithAttributes(LocalFileSystemImpl.java:285)
        at com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl.lambda$new$0(LocalFileSystemImpl.java:42)
        at com.intellij.openapi.vfs.DiskQueryRelay.accessDiskWithCheckCanceled(DiskQueryRelay.java:47)
        at com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl.listWithCaching(LocalFileSystemImpl.java:249)
        at com.intellij.openapi.vfs.newvfs.persistent.PersistentFSImpl.persistAllChildren(PersistentFSImpl.java:201)
        at com.intellij.openapi.vfs.newvfs.persistent.PersistentFSImpl.listAll(PersistentFSImpl.java:267)
        at com.intellij.openapi.vfs.newvfs.impl.VirtualDirectoryImpl.loadAllChildren(VirtualDirectoryImpl.java:396)
        at com.intellij.openapi.vfs.newvfs.impl.VirtualDirectoryImpl.getChildren(VirtualDirectoryImpl.java:389)
        at com.intellij.testFramework.UsefulTestCase$2.visitFile(UsefulTestCase.java:1136)
        at com.intellij.openapi.vfs.VirtualFileVisitor.visitFileEx(VirtualFileVisitor.java:106)
        at com.intellij.openapi.vfs.VfsUtilCore.visitChildrenRecursively(VfsUtilCore.java:295)
        at com.intellij.testFramework.UsefulTestCase.refreshRecursively(UsefulTestCase.java:1133)
        at com.intellij.testFramework.fixtures.impl.LightTempDirTestFixtureImpl$1.compute(LightTempDirTestFixtureImpl.java:85)
        at com.intellij.testFramework.fixtures.impl.LightTempDirTestFixtureImpl$1.compute(LightTempDirTestFixtureImpl.java:79)
        at com.intellij.openapi.application.impl.ApplicationImpl.lambda$runWriteAction$10(ApplicationImpl.java:964)
        at com.intellij.openapi.application.impl.ApplicationImpl.runWriteActionWithClass(ApplicationImpl.java:943)
        at com.intellij.openapi.application.impl.ApplicationImpl.runWriteAction(ApplicationImpl.java:964)
        at com.intellij.testFramework.fixtures.impl.LightTempDirTestFixtureImpl.copyAll(LightTempDirTestFixtureImpl.java:79)
        at com.intellij.testFramework.fixtures.impl.LightTempDirTestFixtureImpl.copyAll(LightTempDirTestFixtureImpl.java:74)
        at com.intellij.testFramework.fixtures.impl.CodeInsightTestFixtureImpl.copyDirectoryToProject(CodeInsightTestFixtureImpl.java:476)
        at org.mapstruct.intellij.MapstructBaseCompletionTestCase.addDirectoryToProject(MapstructBaseCompletionTestCase.java:52)
        at org.mapstruct.intellij.expression.JavaExpressionInjectionTest.setUp(JavaExpressionInjectionTest.java:172)

I've always suspected IntelliJ does some workarounds against Java modularity (Project Jigsaw), and this one is kind of proof.
The JetBrains code is not broken, just called in a way not originally intended.

Removed the said last known good build from CI as it makes no difference when tested on a Linux-based CI.

@unshare
Copy link
Contributor Author

unshare commented Nov 13, 2022

@filiphr , please also take a look at #112 as it's this minor edit that triggered the whole investigation.
I do rely on IDEA's code inspection for multiple reasons. Hence my tendency to squash false positives.

@filiphr
Copy link
Member

filiphr commented Nov 13, 2022

I also had a lot of JNA not available stuff locally. Using 1.10.0-SNAPSHOT from the gradle-intellij-plugin took all of that away. Seems like the plugin is configuring the build somehow and disables some things for IntelliJ.

Thanks a lot for your work on this. I'll merge it shortly and yes #112 was next on my list to review and merge. Didn't have the time yesterday (I wanted to check it out locally and see what was causing this)

@filiphr filiphr merged commit a052145 into mapstruct:main Nov 13, 2022
@filiphr filiphr added this to the 1.4.0 milestone Nov 13, 2022
@unshare unshare deleted the idea-2022.3 branch November 17, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants