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

fix: spotbugs plugin is not loaded from v5.0.15 #930

Merged
merged 11 commits into from
Aug 3, 2023
Merged

Conversation

KengoTODA
Copy link
Member

@KengoTODA KengoTODA commented Aug 1, 2023

I merged #925 to fix a known bug, but it introduced a regression.
We could not find this regression because functional test cases were not triggered during the CI workflow. So this PR contains two kinds of fixes:

  1. Make functionalTest task running during our CI workflow
  2. Set path of spotbugs plugins properly

@KengoTODA KengoTODA self-assigned this Aug 1, 2023
Signed-off-by: Kengo TODA <skypencil@gmail.com>
Signed-off-by: Kengo TODA <skypencil@gmail.com>
@KengoTODA KengoTODA mentioned this pull request Aug 1, 2023
@KengoTODA KengoTODA changed the title build: run functional-test during the CI process fix: spotbugs plugin is not loaded from v5.0.15 Aug 2, 2023
Signed-off-by: Kengo TODA <skypencil@gmail.com>
@@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
gradle: ['7.0', '8.0']
Copy link
Member Author

Choose a reason for hiding this comment

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

Gradle 7.0 does not support FooJay plugin, so need to bump up the minimal version to 7.6.2. This change does NOT mean that our users need to use 7.6.2 or later.

@@ -72,5 +72,6 @@ tasks {
}
}
tasks.check {
dependsOn(tasks.named("functionalTest"))
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is necessary to run functional test cases during the CI workflow.

import spock.lang.Requires
import spock.lang.Specification

import static org.gradle.testkit.runner.TaskOutcome.SUCCESS

@Ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

This functional test is not working, and we have less contribution on Android support (refs #90) so simply ignore this specification for now.

"""
new File(rootDir, "settings.gradle.kts") << """
plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version("0.6.0")
Copy link
Member Author

Choose a reason for hiding this comment

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

We need this setting to use the Gradle Java toolchain with Gradle 8.0+.

"configuration for the external SpotBugs plugins excluding transitive dependencies")
.create(SpotBugsPlugin.PLUGINS_CONFIG_NAME)
.setDescription("configuration for the external SpotBugs plugins")
.setVisible(false)
Copy link
Member Author

Choose a reason for hiding this comment

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

The internal configuration (actualSpotbugsPlugins) is needless because the FindBugs plugin is always distributed with its dependencies so no need to add transient dependencies to spotbugsClasspath. See PluginLoader for detail, it uses Class-Path metadata of jar file to find dependencies.

Signed-off-by: Kengo TODA <skypencil@gmail.com>
@KengoTODA KengoTODA marked this pull request as ready for review August 2, 2023 08:17
@KengoTODA KengoTODA requested a review from a team August 2, 2023 08:17
@sonarcloud
Copy link

sonarcloud bot commented Aug 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@KengoTODA
Copy link
Member Author

I want to release this change to fix a known regression, so let me merge this for now. There is no additional test case nor new behavior then it should be OK I believe.

@KengoTODA KengoTODA merged commit 61309a2 into master Aug 3, 2023
5 checks passed
@KengoTODA KengoTODA deleted the KengoTODA-patch-1 branch August 3, 2023 13:04
@github-actions
Copy link

github-actions bot commented Aug 3, 2023

🎉 This PR is included in version 5.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

KengoTODA added a commit that referenced this pull request Aug 5, 2023
Signed-off-by: Kengo TODA <skypencil@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant