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

#911 support intellij 2024.2 by updating gradle version #1053

Open
wants to merge 17 commits into
base: intellij-2024.2-support
Choose a base branch
from

Conversation

staicy123
Copy link

@staicy123 staicy123 commented Oct 28, 2024

Fixes #911
Updated intellij platform and kotlin jvm version
Updated source compatability and target compatability in build.gradle and given java version inside gradle.properties file
Changed intellij to intellijPlatform and added it inside dependencies tag
Updated runIde task in build.gradle
Added intellijPlatform tag to add buildSearchableOptions, since build, until build and add the changeNotes
updated platformVersion in gradle.properties

@vaisakhkannan
Copy link
Contributor

vaisakhkannan commented Nov 8, 2024

NOTE : This PR doesn’t include the changes for the runIdeForUiTests task. That's why this PR build is failed. I will open a separate PR for the runIdeForUiTests task to enable UI tests(since I worked mainly on Ui test task issue #885), which will ensure both UI and unit tests pass in that build. I have tested this in my fork branch with these PR changes and the runIdeForUiTests task enabled, and the build succeeded for IntelliJ IDE version 2024.1. However, if we update to version 2024.2(in gradle.properties), we will encounter both unit and UI test failures. So, we'll just stick with IntelliJ IDEA version 2024.1 for now.

Check the build that enabled runIdeForUiTests Task : https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/11741958620
Branch : https://github.com/vaisakhkannan/liberty-tools-intellij/tree/refs/heads/%23885-Enable-runIdeForUiTests-task
( This branch includes Stacy's this PR change )

cc: @TrevCraw

gradle.properties Outdated Show resolved Hide resolved
dessina-devasia
dessina-devasia previously approved these changes Nov 8, 2024
build.gradle Outdated Show resolved Hide resolved
Copy link
Contributor

@TrevCraw TrevCraw left a comment

Choose a reason for hiding this comment

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

The commit history associated with this PR is confusing. There are some duplicate commits that have the exact same changes:

There are also commits that have the same description, but have different changes:

Please take some time to clean up the commit history.

patchPluginXml {
changeNotes = """
<h2> 24.0.9 </h2>
intellijPlatform {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is intellijPlatform in three different spots?

Copy link
Author

Choose a reason for hiding this comment

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

Here in this doc it's explained https://plugins.jetbrains.com/docs/intellij/tools-intellij-platform-gradle-plugin.html#setting-up-intellij-platform to add intellijPlatform in different places as per new updation.
Screenshot 2024-11-11 at 9 04 37 AM

sinceBuild = providers.gradleProperty("pluginSinceBuild")
untilBuild = providers.gradleProperty("pluginUntilBuild")
}
changeNotes = """
Copy link
Contributor

Choose a reason for hiding this comment

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

why did the change notes need to be moved from pathPluginXml to intellijPlatform?

Copy link
Author

Choose a reason for hiding this comment

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

This change was suggested by jetbrains team to move the patchPluginXml.changeNotes to intellijPlatform.pluginConfiguration.changeNotes
https://jetbrains-platform.slack.com/archives/C5U8BM1MK/p1729610706101729?thread_ts=1729576853.003949&cid=C5U8BM1MK

Copy link
Contributor

@TrevCraw TrevCraw Nov 14, 2024

Choose a reason for hiding this comment

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

OK. We will need to confirm that this still works correctly when the plugin is uploaded to the marketplace. Can you please open an issue to upload an early version of the plugin to a private repo/stream in the JetBrains marketplace to check that the plugin info and change notes are displayed properly?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I have created the issue for analysing this part here : #1112

gradle.properties Outdated Show resolved Hide resolved
@@ -97,9 +103,7 @@ dependencies {
testImplementation 'com.intellij.remoterobot:remote-robot:' + remoteRobotVersion
testImplementation 'com.intellij.remoterobot:remote-fixtures:' + remoteRobotVersion
testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.2'
testImplementation('com.jetbrains.intellij.maven:maven-test-framework:241.15989.150') {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this being removed? This will eventually be updated later, but should be left alone for now.

Copy link
Author

Choose a reason for hiding this comment

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

Removed this dependency as jetbrains have introduced new TEST Framework which will automatically include this dependency based on intellij version in our project. This was suggested by jetbrains team here
https://jetbrains-platform.slack.com/archives/C5U8BM1MK/p1730297530842639?thread_ts=1730105939.390899&cid=C5U8BM1MK

Copy link
Contributor

Choose a reason for hiding this comment

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

So we have confirmed that we can run the unit tests with this dependency removed?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, @TrevCraw,

Instead of adding maven-test-framework directly, IntelliJ 2024.2 introduced a way to add this line: testFramework TestFrameworkType.Plugin.Maven.INSTANCE within the intellijPlatform setup under the dependencies tag. This approach ensures that the dependency is downloaded in our project, and the version is determined by the IntelliJ platform version specified.

However, even with this adjustment, unit tests do not work in IntelliJ 2024.2. That said, making this change has been verified to fully support unit tests in IntelliJ 2024.1.

These are the dependencies used in the project, and when removing the direct dependency and instead adding the framework as suggested by JetBrains, maven-test-framework is downloaded correctly.
Screenshot 2024-11-15 at 11 21 57 AM

Thanks,

build.gradle Outdated
version.set(remoteRobotVersion)
}

runIdeForUiTests {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the unit test framework, we should not be removing the runIdeForUiTests or downloadRobotServerPlugin sections. This should be left alone for now.

Copy link
Author

Choose a reason for hiding this comment

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

I removed this part, as @vaisakhkannan will be raising the PR for UI test part including all this. If you think I shouldn't remove and comment it out I can do so.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not remove. Let @vaisakhkannan handle it in his PR.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I will add back the changes.

gradle.properties Outdated Show resolved Hide resolved
Comment on lines +145 to +147
pluginVerifier()
zipSigner()
instrumentationTools()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why these lines needed to be added?

Copy link
Author

@staicy123 staicy123 Nov 11, 2024

Choose a reason for hiding this comment

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

InstrumentationTools()
When we remove this it will throw error during buildPlugin:
Screenshot 2024-11-11 at 4 47 15 PM
=> The code instrumentation process handled with the instrumentCode task, requires extra dependencies to work and properly adjust the Java bytecode. There's the instrumentationTools() dependencies helper introduced to apply all required dependencies using default configuration
https://plugins.jetbrains.com/docs/intellij/tools-intellij-platform-gradle-plugin-dependencies-extension.html#java-runtime
PluginVerifier():
the Plugin Verifier is a tool used to check whether your IntelliJ IDEA plugin is compatible with specific versions of IntelliJ IDEA (or other JetBrains IDEs like PyCharm, WebStorm, etc.). It helps you ensure that your plugin works correctly with different versions of the IDE before releasing it to users, especially when updates to the IDE or your plugin occur.

Why Do You Need It?

When you develop a plugin for an IntelliJ-based IDE, new versions of the IDE might introduce changes that could potentially break the functionality of your plugin. The Plugin Verifier ensures your plugin works with the version(s) of the IDE you intend it to support. It checks for compatibility issues and reports any problems, such as deprecated API usage, missing features, or unsupported changes in the IDE.

Without verifying, your plugin might break when users upgrade to newer versions of the IDE, leading to crashes or broken functionality for them.https://plugins.jetbrains.com/docs/intellij/tools-intellij-platform-gradle-plugin-dependencies-extension.html#target-platforms

I am not sure whether we need zipSigner().
Purpose: Plugin Signing is a security feature introduced by JetBrains starting from 2021.2 to protect users from potentially malicious or tampered plugins in the JetBrains Marketplace (and in IntelliJ-based IDEs like IntelliJ IDEA, PyCharm, WebStorm, etc.).

In simple terms, Plugin Signing ensures that the plugin you install on your IDE has not been tampered with and comes from the original developer who created it.
https://plugins.jetbrains.com/docs/intellij/plugin-signing.html

build.gradle Show resolved Hide resolved
@staicy123 staicy123 force-pushed the #911-Support-Intellij-2024.2-by-updating-gradle-version branch from 39fa3f5 to 9a868b2 Compare November 11, 2024 12:50
Updated intellij platform and kotlin jvm version
Updated source compatability and target compatability in build.gradle and given java version inside gradle.properties file
…ij version and platform

Changed intellij to intellijPlatform and added it inside dependencies tag
Updated runIde task in build.gradle
…ld, until build and add the changeNotes

Added intellijPlatform tag to add buildSearchableOptions, since build, until build and add the changeNotes
Changed intellij to intellijPlatform and added it inside dependencies tag
updated platformVersion
Removed manual dependency added for maven test as jetbrains providing a testframework which will auto download latest maven test version
…dependency=false

defaulted intellij version to 2024.1 i.e PROD version and added kotlin.stdlib.default.dependency=false
to stop including this lib automatically when build is ran
Added an import and commented runIdeForUiTest task as it doesn't support intellij 2024.2 version
Removed since and until build from plugin.xml since already given in gradle.properties
@staicy123 staicy123 force-pushed the #911-Support-Intellij-2024.2-by-updating-gradle-version branch from 9a868b2 to 39b2e89 Compare November 11, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants