Bugzilla – Full Text Bug Listing
| Summary: | Fail Gerrit build if new Java warnings are introduced | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Lars Vogel <Lars.Vogel> |
| Component: | UI | Assignee: | Alexander Kurtakov <akurtakov> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | akurtakov, daniel_megert, Lars.Vogel, mat.booth, michael.keppler, rolf.theunissen, sravankumarl |
| Version: | 4.10 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| See Also: | https://git.eclipse.org/r/141950 | ||
| Whiteboard: | |||
| Bug Depends on: | 506778, 547470 | ||
| Bug Blocks: | |||
|
Description
Lars Vogel
Generally that is possible, but there is a twist to it. You can have all kinds of warnings be reported and collected in Jenkins using the Warnings plugin (previously available as a whole bunch of plugins for Checkstyle, Findbugs, PMD, ...). See https://github.com/jenkinsci/warnings-plugin/blob/master/doc/Documentation.md for the documentation. And that plugin (as well as many others) allows to configure thresholds to set a build to failed or unstable. Those thresholds can be absolute numbers of issues or new issues. See https://github.com/jenkinsci/warnings-plugin/blob/master/doc/Documentation.md#quality-gate-configuration That is almost what you need. However, in my experience with gerrit based builds in my company, new issues doesn't mean "new issues in this commit", but "new issues since last build of this project". Therefore this delta is not really useful for gerrit builds, as it just tells the difference since another arbitrary gerrit build. Still, we could use this "new issues" reporting on the master build, instead of on the gerrit build, to at least have some more reporting. This would work by marking an otherwise green build as yellow (unstable), if new issues have been found since last build of master. Since the plugin supports git blame, developers can also be identified easily. One last word of warning: I have used the predecessor plugins of the warning plugin for several years and they worked fine, but the new plugin is currently only in beta (and goes through a major rewrite), so there might be some more hickups than normal. @Mat, Can you take look in to this? This will not work at all for the following repositories - eclipse.platform.runtime - eclipse.platform.ui because they specify -nowarn in their root pom.xml. (In reply to Dani Megert from comment #4) > This will not work at all for the following repositories > - eclipse.platform.runtime > - eclipse.platform.ui > because they specify -nowarn in their root pom.xml. Lets try to remove it and see what happens. New Gerrit change created: https://git.eclipse.org/r/141950 (In reply to Lars Vogel from comment #5) > (In reply to Dani Megert from comment #4) > > This will not work at all for the following repositories > > - eclipse.platform.runtime > > - eclipse.platform.ui > > because they specify -nowarn in their root pom.xml. > > Lets try to remove it and see what happens. Well, the proposed change will work, but you insisted in bug 506778 to use project settings. I was always against that. And current status is that it causes bug 546976. (In reply to Dani Megert from comment #7) > (In reply to Lars Vogel from comment #5) > > (In reply to Dani Megert from comment #4) > > > This will not work at all for the following repositories > > > - eclipse.platform.runtime > > > - eclipse.platform.ui > > > because they specify -nowarn in their root pom.xml. > > > > Lets try to remove it and see what happens. > Well, the proposed change will work, but you insisted in bug 506778 to use > project settings. I was always against that. And current status is that it > causes bug 546976. Lars, your proposed change also fixes the comparator bug 546976. I would really like to revert bug 546976. Besides clearing the road for this bug here and fixing the other bug, I am still convinced that it is better that Gerrit reports what also the official build reports, e.g. same warnings and errors. If you are OK with that, I will handle the revert. (In reply to Dani Megert from comment #8) > Lars, your proposed change also fixes the comparator bug 546976. I would > really like to revert bug 546976. Besides clearing the road for this bug > here and fixing the other bug, I am still convinced that it is better that > Gerrit reports what also the official build reports, e.g. same warnings and > errors. If you are OK with that, I will handle the revert. +1 (In reply to Lars Vogel from comment #9) > (In reply to Dani Megert from comment #8) > > Lars, your proposed change also fixes the comparator bug 546976. I would > > really like to revert bug 546976. Besides clearing the road for this bug > > here and fixing the other bug, I am still convinced that it is better that > > Gerrit reports what also the official build reports, e.g. same warnings and > > errors. If you are OK with that, I will handle the revert. > > +1 Thanks Lars, much appreciated! (In reply to Michael Keppler from comment #1) > Generally that is possible, but there is a twist to it. You can have all > kinds of warnings be reported and collected in Jenkins using the Warnings > plugin (previously available as a whole bunch of plugins for Checkstyle, > Findbugs, PMD, ...). See > https://github.com/jenkinsci/warnings-plugin/blob/master/doc/Documentation.md > for the documentation. > > And that plugin (as well as many others) allows to configure thresholds to > set a build to failed or unstable. Those thresholds can be absolute numbers > of issues or new issues. See > https://github.com/jenkinsci/warnings-plugin/blob/master/doc/Documentation.md#quality-gate-configuration > > > That is almost what you need. However, in my experience with gerrit based > builds in my company, new issues doesn't mean "new issues in this commit", > but "new issues since last build of this project". Therefore this delta is > not really useful for gerrit builds, as it just tells the difference since > another arbitrary gerrit build. For us this would work because our policy is to be COMPILE warning free. Could another option be to fail the build based on some console output, i.e. simply fail when we see a compile warning in the console output? (In reply to Dani Megert from comment #11) > Could another option be to fail the build based on some console output, i.e. > simply fail when we see a compile warning in the console output? Doesn't the compiler have a "treat warnings as errors" flag? (In reply to Mat Booth from comment #12) > (In reply to Dani Megert from comment #11) > > Could another option be to fail the build based on some console output, i.e. > > simply fail when we see a compile warning in the console output? > > Doesn't the compiler have a "treat warnings as errors" flag? It's in the works Bug 547470 and Bug 547591 after that we should be able to pass -Dmaven.compiler.failOnWarning for that. SWT repo now has -Dmaven.compiler.failOnWarning=true passed to maven and warning causes build failure as in https://ci.eclipse.org/platform/job/eclipse.platform.swt-Gerrit/167/console. I'm going to resolve this bug as it's up to each repo to now get to clean build and enable the setting. Kudos to the SWT team for having warnings |