intellij-platform-gradle-plugin icon indicating copy to clipboard operation
intellij-platform-gradle-plugin copied to clipboard

In tests, class `GoFileType` is unavailable at runtime with 2024.3eap

Open jansorg opened this issue 1 year ago • 17 comments

What happened?

This test compiles, but does not run: https://github.com/jansorg/2024.3-godep-demo/blob/main/src/test/kotlin/dev/ja/godepdemo/godepdemo/GoFileTypeActionTest.kt

GoFileType is available at compile time, but is not available at runtime.

With 2024.2, the test is passing. It's not passing with 2024.3 eap. Adding a dependency on intellij.go.frontback does not work, because the module doesn't exist (apparently, although there's a intellij.go.frontback.xml).

With 2.1.0 and 2024.3 eap it's currently not possible to develop Go-aware code.

Relevant log output or stack trace

No response

Steps to reproduce

  1. Clone https://github.com/jansorg/2024.3-godep-demo.git
  2. Run tests

Gradle IntelliJ Plugin version

2.1.0

Gradle version

8.10.2

Operating System

Linux

Link to build, i.e. failing GitHub Action job

No response

jansorg avatar Oct 15 '24 09:10 jansorg

It's also broken for PhpFileType

jansorg avatar Oct 15 '24 09:10 jansorg

Add at least one bundled plugin dependency, I think just anyone might work, without it things do not work as expected.

AlexanderBartash avatar Oct 15 '24 12:10 AlexanderBartash

Ok, not just anyone, but com.intellij.java will make it work.

AlexanderBartash avatar Oct 15 '24 12:10 AlexanderBartash

I think the issue is that when you have no bundled plugins & modules and add just a 3rd party plugin, when you start an IDE everything seems to work because the Gradle plugin does not attempt to disable non-included bundled plugins, because they are bundled. But, when you run tests somehow it does that. You can see that in the log file I mentioned.

On the left is just Go in tests (not loaded), on the right is go +

bundledPlugin("com.intellij.platform.images")
bundledPlugin("com.intellij.copyright")
bundledPlugin("com.intellij.java")

On the bottom is runIde task logs image

Probably the proper workaround for now is look for a log like:

2024-10-15 15:11:41,840 [    316]   INFO - #c.i.i.p.PluginManager - Module intellij.platform.images.backend.svg is not enabled because dependency com.intellij.platform.images is not available
Module intellij.platform.images.copyright is not enabled because dependency com.intellij.platform.images is not available
Module intellij.kotlin.onboarding-promoter is not enabled because dependency kotlin.features-trainer is not available
Module intellij.profiler.ultimate is not enabled because dependency com.intellij.java is not available

And add what it wants.

This reminds me of https://youtrack.jetbrains.com/issue/IDEA-337162

AlexanderBartash avatar Oct 15 '24 12:10 AlexanderBartash

It may be actually caused just by com.intellij.modules.json dependency. Go added that in 243: image Also adding just bundledPlugin("com.intellij.modules.json") fixed the build.

AlexanderBartash avatar Oct 15 '24 15:10 AlexanderBartash

Oh, my. Thank you, that would explain it! I really wish transitive (and available) dependencies would be added automatically...

jansorg avatar Oct 15 '24 16:10 jansorg

Yeah, adding them is a bit tricky. A bit more on what is going on.

In 243 Go plugin has only these dependencies (if its modules are not taken into account):

<plugin id="com.intellij.modules.lang"/>
<plugin id="com.intellij.modules.ultimate"/>
<plugin id="com.intellij.modules.go-capable"/>
<module name="intellij.platform.vcs.impl"/>

JSON here probably was part of the default IDE capabilities, i.e. always available on the classpath. So it was not defined as a dependency.

Then JSON was moved into a bundled plugin. Which may seem irrelevant, since it is bundled, so why should it matter. So in 243 JSON was added to Go plugin dependencies:

<plugin id="com.intellij.modules.lang"/>
<plugin id="com.intellij.modules.ultimate"/>
<plugin id="com.intellij.modules.go-capable"/>
<plugin id="com.intellij.modules.json"/>
<module name="intellij.platform.vcs.impl"/>

It is important to note, that all these dependencies in the list (except JSON in 243) also look like a part of the default IDE capabilities, i.e. always available on the classpath.

It seems that when the test task runs, it behaves differently from the runIde task:

  • runIde by default loads all bundled plugins
  • test task starts the IDE in a headless mode with all plugins disabled, including bundled plugins.

Most likely because of this line: https://github.com/JetBrains/intellij-platform-gradle-plugin/blob/33af9bdd1d0d8b996b911781ab23013dd14997f4/src/main/kotlin/org/jetbrains/intellij/platform/gradle/tasks/TestIdeTask.kt#L107

If a dependency on a bundled plugin is not defined, it won't be on test classpath so can not be loaded, so it is not.

Which means that in 243 the test task does not load JSON by default, unless a dependency is declared on it. Go probably was not loaded simply because it has a non-optional dependency on JSON.

As far as I know, this plugin does look at dependencies of the 3rd party plugins and does not add them automatically to the classpath. E.g. here after it downloads a 3rd party plugin zip it creates an IvyModule for it, and in that module it does not define dependencies = {list of dependencies from 3rd party plugin.xml}. It creates an IvyModule simply becaues that is how this plugin integrates jars, zips, files with Gradle. It does not mean that we pull it from a real Ivy repo.

https://github.com/JetBrains/intellij-platform-gradle-plugin/blob/bf8fc7fa180ef9c797d18e3b66b3f743532449a3/src/main/kotlin/org/jetbrains/intellij/platform/gradle/shim/PluginArtifactoryShim.kt#L71

Again, it does not matter (not visible) during runIde because it loads everything. But it matters in tests.

To fix this, either:

  • Everyone should declare dependencies on all transitive dependencies of all 3rd party plugins added as a dependency.
  • This plugin should add those dependencies automatically once plugin zip is downloaded, which is a bit tricky because it is a zip, and it is unzipped using a transform, which happens kind of by itself when the dependencies get resolved. So if we want to generate more dependencies when we download random zips from the marketplace, we should do it before the other configuration with dependencies on "bundledPlugin" & "bundledModule" is resolved because it can not be modified after.

AlexanderBartash avatar Oct 15 '24 19:10 AlexanderBartash

Actually, it's almost the same as #1791. The only difference is that here the plugin which is not loaded automatically is com.intellij.modules.json instead of intellij.charts

And from my POV such required dependencies should be loaded automatically (at least if we are talking about bundled plugins) like common transitive dependencies of your external plugins

@hsz WDYT?

Undin avatar Oct 16 '24 12:10 Undin

@Undin For bundled plugins, dependencies are added:

https://github.com/JetBrains/intellij-platform-gradle-plugin/blob/33af9bdd1d0d8b996b911781ab23013dd14997f4/src/main/kotlin/org/jetbrains/intellij/platform/gradle/extensions/IntelliJPlatformDependenciesHelper.kt#L773

Even transitive (notice the recursion): https://github.com/JetBrains/intellij-platform-gradle-plugin/blob/33af9bdd1d0d8b996b911781ab23013dd14997f4/src/main/kotlin/org/jetbrains/intellij/platform/gradle/extensions/IntelliJPlatformDependenciesHelper.kt#L839

AlexanderBartash avatar Oct 16 '24 12:10 AlexanderBartash

@AlexanderBartash But, unfortunately, it doesn't work even with database plugin where all plugins (database plugin itself and all its dependencies) are bundled. But I think it should be discussed in #1791 since these issues might only look similar but have different reasons

Undin avatar Oct 16 '24 13:10 Undin

@Undin Yes, maybe. Try to open Gradle tool window and go to dependencies. You will notice that bundled plugins could have many transitive dependencies.

image

AlexanderBartash avatar Oct 16 '24 13:10 AlexanderBartash

Anyway, it's quite unexpected that converting bundled IDE module into bundled plugin (i.e. pure internal thing) may break tests. And it may take quite a lot of time to understand the reason if you haven't faced with similar things before

Undin avatar Oct 16 '24 13:10 Undin

@AlexanderBartash Sorry, but I don't understand what you want to say. I understand that bundled plugins have dependencies, that it's possible to find them either with Gradle tool window or inside ivy xml files located in .intellijPlatform/localPlatformArtifacts dir. But all these facts don't help, unfortunately. At least, from the users' point of view. And I don't think that it's ok to force users to dive into dependencies of bundled plugins and how intellij platform gradle plugin works. It should just work. Currently, it doesn't work in this way ⇒ it should be fixed

Undin avatar Oct 16 '24 14:10 Undin

@Undin I just updated my https://github.com/JetBrains/intellij-platform-gradle-plugin/issues/1798#issuecomment-2416922554 above, with a more proper example. In the database plugin you can see transitive deps, why grid.impl does not have any deps there is a mystery to me.

AlexanderBartash avatar Oct 16 '24 14:10 AlexanderBartash

@AlexanderBartash Sorry, but I don't understand what you want to say. I understand that bundled plugins have dependencies, that it's possible to find them either with Gradle tool window or inside ivy xml files located in .intellijPlatform/localPlatformArtifacts dir. But all these facts don't help, unfortunately. At least, from the users' point of view. And I don't think that it's ok to force users to dive into dependencies of bundled plugins and how intellij platform gradle plugin works. It should just work. Currently, it doesn't work in this way ⇒ it should be fixed

Those dependencies in Gradle tool window or in Ivy files are exactly the transitive dependencies created by this plugin. Links to the code provided in https://github.com/JetBrains/intellij-platform-gradle-plugin/issues/1798#issuecomment-2416746369

Those dependencies should be available on the classpath. But again, something might be wrong with grid.impl. What idk yet.

AlexanderBartash avatar Oct 16 '24 14:10 AlexanderBartash

@Undin Just for clarification, because it was confusing me too. Those ivy files do not mean that plugins are downloaded from a real ivy repo. Those files are created by this plugin in a dir, then the dir is simply registered in Gradle as a local Ivy repo, so that Gradle can understand where to find the jars by looking at Ivy files.

If we assume that this plugin has a bug leading to a missing transitive dependency, it will be missing in the Ivy files too, because the plugin creates them. Also it will be missing in that Gradle tool window, because that is probably the same as ./gradlew dependencies

That is simply the way this plugin adds dependencies to Gradle today.

AlexanderBartash avatar Oct 16 '24 14:10 AlexanderBartash

@Undin There is a bug with recursion in https://github.com/JetBrains/intellij-platform-gradle-plugin/blob/33af9bdd1d0d8b996b911781ab23013dd14997f4/src/main/kotlin/org/jetbrains/intellij/platform/gradle/extensions/IntelliJPlatformDependenciesHelper.kt#L839

That is the reason for #1791

AlexanderBartash avatar Oct 16 '24 16:10 AlexanderBartash

@jansorg Thanks for reporting. With the recent changes, your demo project works correctly, and all tests have passed. @AlexanderBartash introduced some changes that now include all dependency plugins correctly in the tests runtime classpath. I have also adjusted how that classpath is prepared. Before, only the bundled modules were included, which prevented bundled plugins from being present, causing #1791. Now, all bundled plugins are also present in the runtime classpath but not in the compilation classpath, so you won't include their classes by accident.

hsz avatar Dec 04 '24 08:12 hsz