Skip to content

Implement Worker API for VerifyPluginTask #1948

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

alajemba-vik
Copy link

@alajemba-vik alajemba-vik commented May 31, 2025

Pull Request Details

Updated VerifyPluginTask to delegate task action to VerifyPluginWorkAction to leverage Worker API

Description

classes VerifyPluginWorkAction and VerifyPluginWorkParameters were added to implement Worker API functionality. The VerifyPluginTask was updated to extend DefaultTask instead of JavaExec because it is no longer directly delegating work to the intellijPluginVerifier but allowing the WorkAction class to do so instead.
Only the necessary information for each submitted work action is defined in the VerifyPluginWorkParameters

Motivation and Context

Improve the performance of the VerifyPluginTask

How Has This Been Tested

Compared the outputs of the VerifyPluginTask before and after the Worker API implementation
Compared the Build Scan results of running the verifyPlugin task before integrating Worker API and after the integration

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have included my change in the CHANGELOG.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@alajemba-vik
Copy link
Author

These changes will result in a race condition if there are several ides we are trying to verify. The submitted work actions end up deleting the same verification reports directory.
Potential solutions I can think of for now:

  • Allow each work action to accept a different verification reports directory
  • Delete the verification reports directory before you submit the work action
    @hsz

@alajemba-vik
Copy link
Author

alajemba-vik commented Jun 2, 2025

Additionally, the ides to use in plugin verification is resolved before task execution. Waiting for all ides to download before going on to execute the task action is one of the major delays.
Considering resolving the ides during task execution? That way, we don't have to wait for all the ides to download before we start verifying.

@hsz
Copy link
Member

hsz commented Jun 5, 2025

These changes will result in a race condition if there are several ides we are trying to verify. The submitted work actions end up deleting the same verification reports directory. Potential solutions I can think of for now:

  • Allow each work action to accept a different verification reports directory
  • Delete the verification reports directory before you submit the work action
    @hsz

Different report directories should do the job – each report should be stored within a directory named by the IDE we use for testing.

@hsz
Copy link
Member

hsz commented Jun 5, 2025

Additionally, the ides to use in plugin verification is resolved before task execution. Waiting for all ides to download before going on to execute the task action is one of the major delays. Considering resolving the ides during task execution? That way, we don't have to wait for all the ides to download before we start verifying.

Are you sure that IDEs are resolved before the task execution? I'd rather assume that the ides collection is resolved when we start looping over it. Or maybe not, and it's calculated (therefore resolved) yet in the configuration phase.

You can try moving the following part right into the task's action:

ides = project.files(project.provider {
with(intellijPluginVerifierIdesConfiguration.incoming.dependencies) {
if (size > 5) {
val ideList = joinToString(", ") { "${it.group}:${it.name}:${it.version}" }
log.warn("The ${Tasks.VERIFY_PLUGIN} task is about to resolve $size IDEs: $ideList")
}
flatMap {
project.configurations.detachedConfiguration(it).apply {
attributes { attribute(Attributes.extracted, true) }
}.resolve()
}
}
})

This will surely violate the configuration cache, but our task is already untracked, so nothing should fail, I believe.

Copy link
Member

@hsz hsz left a comment

Choose a reason for hiding this comment

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

Please remember about the apiCheck and apiDump tasks.

@hsz hsz added the enhancement label Jun 5, 2025
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