Skip to content
This repository has been archived by the owner on Oct 22, 2023. It is now read-only.

Is there a facility to discover new bugs between SpotBugs run? #55

Open
smillies opened this issue Aug 31, 2018 · 15 comments
Open

Is there a facility to discover new bugs between SpotBugs run? #55

smillies opened this issue Aug 31, 2018 · 15 comments

Comments

@smillies
Copy link

In our continuous build with Jenkins, I'd be interested in generating warnings when the bugs between two consecutive SpotBugs runs change. Is the XML or HTML report output stable enough so that I can do that with a simple file comparison? Or can there be basically non-deterministic differences with regard to the order of elements or attributes etc. that make this infeasible?

Even better would be a more sophisticated system that raised an alarm only if new bugs appear, but not if just some old ones got fixed. There occurred to me one really roundabout way to achieve this: Generate an exclude filter file from the previous report and use that as input for the next run. But I'd despair of writing the requisite xsl transformation.

Do you know of any ready-made tool that can do this kind of analysis?

@mkienenb
Copy link

We run something like this from a crontab entry each day using findbugs and diff, so, yes, at least the plain-text output should be deterministic with the arguments below and, yes, you should be able to do a simple file compare.

I can't give you what we have verbatim, but here's the important bits:

cron script:

for i in ${FPATH} ;do
    echo >> ${FLOG}
    echo "==============================================================" >> ${FLOG}
    echo "==== ${i} " >> ${FLOG}
    echo "==============================================================" >> ${FLOG}
    bin/findbugs ${i} >> ${FLOG} 2>/dev/null
done

count=$(grep -v "====" ${FLOG} | grep -c YOUR_SEARCH_TERM_HERE)
if [ ${count} -gt 0 ] ;then
    echo "" > ${FLOG}_diff
    if [ -e "${YLOG}" ] ; then
        echo "==============================================================" >> ${FLOG}_diff
        echo "==== Bug Changes since yesterday " >> ${FLOG}_diff
        echo "==============================================================" >> ${FLOG}_diff
        diff ${YLOG} ${FLOG} >> ${FLOG}_diff
        echo "" >> ${FLOG}_diff
        echo "" >> ${FLOG}_diff
    fi

bin/findbugs:

java -jar lib/findbugs-3.0.1.jar -textui -longBugCodes ${excludeOption} ${path} "

@smillies
Copy link
Author

smillies commented Sep 3, 2018

Thank you. I'll try something along these lines.

In this context, it would be good if SpotBugs (Ant task) were able to generate two reports simultaneously: in this case a text report for automatic diffing, and an html report for human consumption. As a single scan of my codebase takes about 4 minutes, it would be tedious having to run SpotBugs twice in a row.

Is this possible? It seems the output-attribute accepts only one value, but perhaps there's a trick?

@smillies
Copy link
Author

smillies commented Sep 3, 2018

If anyone's interested, here's what I do: I start two parallel tasks in Ant, one giving an HTML report, the other giving a text report which is then treated with diff.

<target name="check.spotbugs">
		<!-- Ensure Ant always uses the task from our current SpotBugs version -->
		<copy file="${spotbugs.home}/lib/spotbugs-ant.jar" tofile="${ant.home}/lib/spotbugs-ant.jar" />
		<!-- Ensure log dir exists, save previous text report, delete previous delta and html report -->
		<mkdir dir="${spotbugs.logdir}"/>
		<move file="${spotbugs.logdir}/spotbugs-check.txt" tofile="${spotbugs.logdir}/spotbugs-check.txt.prev" failonerror="false" />
		<delete file="${spotbugs.logdir}/spotbugs-check-newbugs.txt"/>
		<delete file="${spotbugs.logdir}/spotbugs-check.html"/>
		<!-- A single SpotBugs run on our codebase can take several minutes, so we parallelize the creation of the
			text and html reports. The text report is there for automatic diffing, the html report for human consumption. -->
	    <pathconvert property="spotbugsAuxClasspath" refid="our.classpath" />
		<parallel>
			<sequential>
				<run.spotbugs spotbugsOutput="text" spotbugsOutputFile="${spotbugs.logdir}/spotbugs-check.txt"/>
				<antcall target="listBugChanges"/>
			</sequential>
			<run.spotbugs spotbugsOutput="html" spotbugsOutputFile="${spotbugs.logdir}/spotbugs-check.html"/>
		</parallel>
	</target>
	
	<macrodef name="run.spotbugs">
		<attribute name="spotbugsOutput"/>
		<attribute name="spotbugsOutputFile"/>
		<sequential>
			<spotbugs home="${spotbugs.home}"
						output="@{spotbugsOutput}" 
						stylesheet="fancy.xsl"
						outputFile="@{spotbugsOutputFile}" 
						reportLevel="high" 
						nested="false" 
						excludeFilter="${basedir}/build_spotbugs_filter.xml"> 
				<auxClasspath path="${spotbugsAuxClasspath}" />
				<sourcePath path="${srcdir}" />
				<class location="${destdir}" /> 
			</spotbugs>
		</sequential>
	</macrodef>
	
	<!-- List new bugs by diffing text report against previous file. Requires Cygwin and ant-contrib. -->
	<target name="listBugChanges">
		<if>
			<available file="${spotbugs.logdir}/spotbugs-check.txt.prev"/>
			<then>
				<echo>New bugs:</echo>
				<exec executable="cmd">
					<arg value="/c"/>
					<arg value="diff ${spotbugs.logdir}/spotbugs-check.txt.prev ${spotbugs.logdir}/spotbugs-check.txt | grep '^>' | tee ${spotbugs.logdir}/spotbugs-check-newbugs.txt"/>
				 </exec>
			</then>
			<else>
				<echo>Not listing bug changes because comparison file ${spotbugs.logdir}/spotbugs-check.txt.prev does not exist.</echo>
			</else>
		</if>
	</target>

@mkienenb
Copy link

mkienenb commented Sep 3, 2018

In this context, it would be good if SpotBugs (Ant task) were able to generate two reports simultaneously

Patches welcome!

@uhafner
Copy link

uhafner commented Sep 3, 2018

Since you are already using Jenkins: the warnings plug-in already does that computation for you. It calculates the "difference" of the warnings in the old and new build. Would that work for you?

@smillies
Copy link
Author

smillies commented Sep 3, 2018

Thanks, that sounds interesting. For one thing, it promises fewer false positives than a simple diff if the plugin can detect the same warning in two versions of a class where some unrelated lines were changed. I'll have to experiment with it. There are some difficulties, for example from reading the description I don't understand how to make use of the previous build as a reference build for each new run.

@uhafner
Copy link

uhafner commented Sep 3, 2018

The documentation is currently work in progress, is there a specific section I need to clarify? There are several options to compute the reference build.

Internally it uses FindBugs hashCode to decide if a warning belongs to the same code block. This part of the algorithm is still under development, I will improve it with some new ideas in the next couple of months.

@smillies
Copy link
Author

smillies commented Sep 3, 2018

The documentation section seems to offer the possibility of choosing some fixed build as the baseline. I would want to be able to configure the previous build. That might be difficult what with usually cleaning the build directory first etc. so perhaps being able to specify a comparison file that would be saved at the end of each build to some configurable location would be possible? Just thinking out loud.

Anyway, I was able to get the latest release and build an hpi file with Maven. However, when I tried uploading that to Jenkins 2.121.3 the plugin installation somehow gut stuck. When I restarted Jenkins the plugin was listed as installed, but in the post build step, there was no Findbugs/Spotbugs file parser in the list.

@smillies
Copy link
Author

smillies commented Sep 3, 2018

Actually I see there is also no such parser in the source code. The documentation mentions FindBugs as a supported reporting tool. How do I pull in the correct parser?

@uhafner
Copy link

uhafner commented Sep 3, 2018

You can get the latest version from the CI build.

For freestyle builds you need to use the action "Record analysis results". There you can select FindBugs or SpotBugs as a parser.

@uhafner
Copy link

uhafner commented Sep 3, 2018

There is also a list of supported formats.

@smillies
Copy link
Author

smillies commented Sep 3, 2018

Thanks, got it working now (after struggling a bit with Jenkins, what with it not being able with subst-directories under Windows, that really had me stumped for a while).

Very nice output. A bit hard to navigate sometimes, for example, it is somewhat annoying that the "Show xx entries" control keeps losing its setting. And displaying the overall total of say "23" below a page having 10 entries is kind of confusing. Also, having to use browser back to navigate up after a drill-down feels strange. I also don't get what clicking on the severities in the Overview widget is supposed to do. (What is a "severity", BTW? I am only familiar with "confidence" and "priority" from FindBugs). And why are the colors for "low/normal/high/" (blueish and reddish) in the history chart different from those used elsewhere (yellowish and greenish)?

But all of that is only a bit of nit-picking. I am quite happy with this plugin, thank you for your contribution!

@uhafner
Copy link

uhafner commented Sep 4, 2018

Thanks for your feedback! Beta testing of the new release has not been started yet so there will still be several (small) things to improve. I would be glad if you would report all those issues as it will help to make the new release even better. (Bigger changes make sense to be reported in our issue tracker).

it is somewhat annoying that the "Show xx entries" control keeps losing its setting

Fixed in jenkinsci/warnings-plugin@2897242.

And displaying the overall total of say "23" below a page having 10 entries is kind of confusing.

I don't understand you point here? It should report something like 'Showing 1 to 25 of 118 entries'. Is there a wrong number shown?

Also, having to use browser back to navigate up after a drill-down feels strange.

And suggestions on how to change?

I also don't get what clicking on the severities in the Overview widget is supposed to do. (What is a "severity", BTW? I am only familiar with "confidence" and "priority" from FindBugs).

The warnings plugin classifies warnings by a severity (error or warning with priority high, normal or low). This is independent of the tool that actually is used. So every parser maps tool specific priorities to one of these 4 values. Then every static analysis tool could use the same severity levels in the UI. (In the FindBugs parser you can choose if the priority or the rank of a bug is mapped, see FindBugsParser.java for details).

And why are the colors for "low/normal/high/" (blueish and reddish) in the history chart different from those used elsewhere (yellowish and greenish)?

The color should already match in all screens now.

@smillies
Copy link
Author

smillies commented Sep 4, 2018

Thanks for the explanations. When I find anything else, I'll report it. For now, I have uploaded a screenshot here to show what I meant about the totals:

image 5

@uhafner
Copy link

uhafner commented Sep 4, 2018

Ah, I see. Now I understand :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants