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

Address deprecation warnings for tests and add local install gradle action #19

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

robsyme
Copy link

@robsyme robsyme commented Dec 4, 2024

This adds a new make command and corresponding gradle task to install the plugin in the user's local plugins directory. This is to make it a little bit easier to get started with plugin development.

make install-plugin

@@ -78,10 +78,43 @@ dependencies {
modules {
module("commons-logging:commons-logging") { replacedBy("org.slf4j:jcl-over-slf4j") }
}

// Add this line to explicitly declare the test framework launcher
testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
Copy link
Author

Choose a reason for hiding this comment

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

This is an addition to address the warning when running make test. The warning was:

./gradlew test --warning-mode all
Task :plugins:nf-hello:test
The automatic loading of test framework implementation dependencies has been deprecated. This is scheduled to be removed in Gradle 9.0. Declare the desired test framework directly on the test suite or explicitly declare the test framework implementation dependencies on the test's runtime classpath. Consult the upgrading guide for further information: https://docs.gradle.org/8.4/userguide/upgrading_version_8.html#test_framework_implementation_dependencies

}

// use JUnit 5 platform
test {
useJUnitPlatform()
}

// Task to install the plugin to the Nextflow plugins directory
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to put this task in the parent build.gradle, inside the submodules config with the other related tasks?

Copy link
Author

Choose a reason for hiding this comment

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

It would make sense if we can have the task iterate over all the possible plugins in the plugins directory. There is only a single plugin at the moment, but it's very possible to have a single repository that hosts multiple plugins.
I'm no gradle expert, but I'll see what I can do.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Thanks for the suggestion, Tom!

/*
* Task to install all plugins to the Nextflow plugins directory
*/
task installPlugins() {
Copy link
Contributor

@tom-seqera tom-seqera Dec 6, 2024

Choose a reason for hiding this comment

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

Gradle isn't the most intuitive, but in this case these two tasks (installPlugins and installSpecificPlugin) are actually implied by installPlugin.

The command

./gradlew installPlugin

will execute the installPlugin task on all modules which define it, and since we define the installPlugin in the submodules block, it's automatically defined on all the individual plugin modules.

To execute the task only for a specific plugin, we can explicitly state which submodule to run the task on:

./gradlew :plugins:nf-hello:installPlugin

This means we can remove both of these tasks, and update the Makefile to use the above forms.

Copy link
Author

Choose a reason for hiding this comment

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

Ah! I didn't know that! Thanks Tom!!

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines +218 to +247
/*
* Task to install all plugins to the Nextflow plugins directory
*/
task installPlugins() {
description = 'Installs all plugins to the Nextflow plugins directory'
group = 'Plugin Installation'

dependsOn subprojects.installPlugin
}

/*
* Task to install a specific plugin by name
* Usage: ./gradlew installPlugin -Pplugin=nf-hello
*/
task installSpecificPlugin {
description = 'Installs a specific plugin (specify with -Pplugin=plugin-name)'
group = 'Plugin Installation'

doFirst {
if (!project.hasProperty('plugin')) {
throw new GradleException("Please specify a plugin name using -Pplugin=plugin-name")
}
def pluginName = project.property('plugin')
def pluginProject = project.findProject(":plugins:${pluginName}")
if (!pluginProject) {
throw new GradleException("Plugin '${pluginName}' not found")
}
dependsOn pluginProject.installPlugin
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to remove these

Suggested change
/*
* Task to install all plugins to the Nextflow plugins directory
*/
task installPlugins() {
description = 'Installs all plugins to the Nextflow plugins directory'
group = 'Plugin Installation'
dependsOn subprojects.installPlugin
}
/*
* Task to install a specific plugin by name
* Usage: ./gradlew installPlugin -Pplugin=nf-hello
*/
task installSpecificPlugin {
description = 'Installs a specific plugin (specify with -Pplugin=plugin-name)'
group = 'Plugin Installation'
doFirst {
if (!project.hasProperty('plugin')) {
throw new GradleException("Please specify a plugin name using -Pplugin=plugin-name")
}
def pluginName = project.property('plugin')
def pluginProject = project.findProject(":plugins:${pluginName}")
if (!pluginProject) {
throw new GradleException("Plugin '${pluginName}' not found")
}
dependsOn pluginProject.installPlugin
}
}

@tom-seqera
Copy link
Contributor

The settings on this repo require you to both 'sign-off' (git commit --signoff) and cryptographically sign (git commit --gpg-sign) all commits before I can merge.

robsyme and others added 4 commits December 9, 2024 17:52
This fix is to address the deprecation warning that occurs when running tests.

This solution follows the recommended approach from the Gradle documentation and should resolve the deprecation warning while maintaining the same test functionality.

Signed-off-by: Rob Syme <[email protected]>
Co-authored-by: Tom Sellman <[email protected]>
Signed-off-by: Rob Syme <[email protected]>
@robsyme
Copy link
Author

robsyme commented Dec 9, 2024

Signed and delivered 🫡

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.

2 participants