search

spotbugs / spotbugs-maven-plugin

Maven Mojo Plug-In to generate reports based on the SpotBugs Analyzer
https://spotbugs.github.io/spotbugs-maven-plugin/
Apache License 2.0
69 stars 51 forks source link

[WIP] Issues/63 support maven toolchains plugin #64

Closed exabrial closed 5 years ago

hazendaz commented 5 years ago

@KengoTODA I'm ok with the fact there are no unit tests. There are none currently at all so that doesn't break with the current state and adding them would sort of mean far more on unit tests than the change would imply. The code otherwise looks good and only enables under certain circumstances which I feel are rather limited in real world use. As the large IT set of tests do not break on this, we should be good.

However, @exabrial what might be useful is to write a IT test that covers an example usage of this. Those all run using maven invoker so you should be able to setup one using your expected configuration. I'll hang on this a while before merging in case you can get that together soon. Thanks.

exabrial commented 5 years ago

How would one assert which jdk is used, especially since antbuilder.java is a static method? (normally i'd mock out that dependency and check to see if the jvm flag is passed)

hazendaz commented 5 years ago

@exabrial Look at the IT tests and see if you can create one there that runs with toolchains. I did go ahead and release 3.1.5 so others can use that but need to hold on this until concerns raised are addressed :)

exabrial commented 5 years ago

Attempting an IT... I'm curious what's going to happen on CI

exabrial commented 5 years ago

btw I'm in another city at the moment, sorry for the slow progress. I don't have much time to work on this but I'll eventually get it working

exabrial commented 5 years ago

@KengoTODA Hey could this build possibly be failing because the lack of a toolchains.xml on the build machine? How can we add this file?

KengoTODA commented 5 years ago

How about using -s option? It lets specify the path of settings.xml. Then you can commit settings.xml as a part of test case.

exabrial commented 5 years ago

@KengoTODA Just so you're absolutely informed what's going on, it's missing a toolchains.xml file, NOT the not settings.xml file you quoted above. The two files have different XSDs.

exabrial commented 5 years ago

The toolchains.xml is supposed to be a machine configuration of where the JVMs are physically located on disk. Including those in a commit means that everyone has to set up their machines in the exact same way. https://maven.apache.org/guides/mini/guide-using-toolchains.html

KengoTODA commented 5 years ago

How about --global-toolchains ?

exabrial commented 5 years ago

I'm going to give up :/ There's not a good way to do Integration Tests without control over the environment. I think the original problem I was facing is less of an issue anyway... (cross compiling a jdk 1.8 on a jdk 1.9 maven vm)

hazendaz commented 1 year ago

Tied to #63