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

Commit

Permalink
Re-work to use existing cobertura results
Browse files Browse the repository at this point in the history 
Also fix a bunch of deprecation and lints.
  • Loading branch information
Aiden Scandella committed Jun 2, 2015
1 parent e59ecfe commit 59b1932
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 188 deletions.
192 changes: 118 additions & 74 deletions phabricator-plugin.iml
View file

Large diffs are not rendered by default.

4 changes: 0 additions & 4 deletions src/main/java/com/uber/jenkins/phabricator/CodeCoverageMetrics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,6 @@ public CodeCoverageMetrics(String sha1, float packagesCoveragePercent, float fil
this.conditionalCoveragePercent = conditionalCoveragePercent;
}

public CodeCoverageMetrics(float packages, float files, float classes, float method, float line, float conditional) {
this(null, packages, files, classes, method, line, conditional);
}

public boolean isValid() {
return lineCoveragePercent != -1;
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public Environment setUp(AbstractBuild build,

String diffID = environment.get(PhabricatorPlugin.DIFFERENTIAL_ID_FIELD);
if (diffID == null || "".equals(diffID)) {
this.addShortText(build, "master");
this.addShortText(build);
this.ignoreBuild(logger, "No differential ID found.");
} else {
LauncherFactory starter = new LauncherFactory(launcher, environment, listener.getLogger(), build.getWorkspace());
Expand All @@ -82,7 +82,7 @@ public Environment setUp(AbstractBuild build,
logger.println("Applying patch for differential");

// Post a silent notification
diff.postComment(diff.getBuildStartedMessage(environment), true);
diff.postComment(diff.getBuildStartedMessage(environment));
diff.setBuildURL(environment);

String baseCommit = "origin/master";
Expand Down Expand Up @@ -138,8 +138,8 @@ public void buildEnvVars(Map<String, String> env) {
};
}

private void addShortText(final AbstractBuild build, final String text) {
build.getActions().add(PhabricatorPostbuildAction.createShortText(text, null));
private void addShortText(final AbstractBuild build) {
build.addAction(PhabricatorPostbuildAction.createShortText("master", null));
}

private Environment ignoreBuild(PrintStream logger, String message) {
Expand Down
77 changes: 10 additions & 67 deletions src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@
import hudson.model.AbstractBuild;
import hudson.model.BuildListener;
import hudson.model.Result;
import hudson.plugins.cobertura.CoberturaCoverageParser;
import hudson.plugins.cobertura.CoberturaPublisher;
import hudson.plugins.cobertura.CoberturaBuildAction;
import hudson.plugins.cobertura.Ratio;
import hudson.plugins.cobertura.targets.CoverageMetric;
import hudson.plugins.cobertura.targets.CoverageResult;
Expand All @@ -52,19 +51,15 @@ public class PhabricatorNotifier extends Notifier {
// Post a comment on success. Useful for lengthy builds.
private final boolean commentOnSuccess;
private final boolean uberallsEnabled;
private final String coberturaReportFile;
private final boolean commentWithConsoleLinkOnFailure;
private final String commentFile;
private final String commentSize;

private final int DEFAULT_COMMENT_SIZE = 1000;

// Fields in config.jelly must match the parameter names in the "DataBoundConstructor"
@DataBoundConstructor
public PhabricatorNotifier(boolean commentOnSuccess, String coberturaReportFile, boolean uberallsEnabled,
public PhabricatorNotifier(boolean commentOnSuccess, boolean uberallsEnabled,
String commentFile, String commentSize, boolean commentWithConsoleLinkOnFailure) {
this.commentOnSuccess = commentOnSuccess;
this.coberturaReportFile = coberturaReportFile;
this.uberallsEnabled = uberallsEnabled;
this.commentFile = commentFile;
this.commentSize = commentSize;
Expand Down Expand Up @@ -97,7 +92,7 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
String diffID = environment.get(PhabricatorPlugin.DIFFERENTIAL_ID_FIELD);
if (CommonUtils.isBlank(diffID)) {
if (needsDecoration) {
build.getActions().add(PhabricatorPostbuildAction.createShortText("master", null));
build.addAction(PhabricatorPostbuildAction.createShortText("master", null));
}
if (uberallsEnabled && coverage != null) {
if (!uberallsConfigured) {
Expand Down Expand Up @@ -276,6 +271,7 @@ private String getRemoteComment(AbstractBuild<?, ?> build, PrintStream logger, S

FilePath source = src[0];

int DEFAULT_COMMENT_SIZE = 1000;
int maxLength = DEFAULT_COMMENT_SIZE;
if (!CommonUtils.isBlank(maxSize)) {
maxLength = parseInt(maxSize, 10);
Expand All @@ -288,59 +284,18 @@ private String getRemoteComment(AbstractBuild<?, ?> build, PrintStream logger, S
return new String(buffer);
}

private CoverageResult getUberallsCoverage(AbstractBuild<?, ?> build, BuildListener listener) throws InterruptedException {
private CoverageResult getUberallsCoverage(AbstractBuild<?, ?> build, BuildListener listener) {
if (!build.getResult().isBetterOrEqualTo(Result.UNSTABLE) || !uberallsEnabled) {
return null;
}

PrintStream logger = listener.getLogger();

logger.println("[uberalls] looking for coverage report in " + coberturaReportFile);

final FilePath[] moduleRoots = build.getModuleRoots();
final boolean multipleModuleRoots =
moduleRoots != null && moduleRoots.length > 1;
final FilePath moduleRoot = multipleModuleRoots ? build.getWorkspace() : build.getModuleRoot();
final File buildCoberturaDir = build.getRootDir();
FilePath buildTarget = new FilePath(buildCoberturaDir);

FilePath[] reports = new FilePath[0];

try {
reports = moduleRoot.act(new CoberturaPublisher.ParseReportCallable(coberturaReportFile));
} catch (IOException e) {
Util.displayIOException(e, listener);
e.printStackTrace(listener.fatalError("Unable to find coverage results"));
}

if (reports.length == 0) {
logger.println("[uberalls] no coverage report found");
}

for (int i = 0; i < reports.length; i++) {
final FilePath targetPath = new FilePath(buildTarget, "uberalls" + (i == 0 ? "" : i) + ".xml");
try {
reports[i].copyTo(targetPath);
} catch (IOException e) {
Util.displayIOException(e, listener);
e.printStackTrace(listener.fatalError("Unable to copy coverage from " + reports[i] + " to " + buildTarget));
build.setResult(Result.FAILURE);
}
}

CoverageResult result = null;
for (File coberturaXmlReport : build.getRootDir().listFiles(new CoberturaReportFilenameFilter())) {
try {
result = CoberturaCoverageParser.parse(coberturaXmlReport, result);
} catch (IOException e) {
Util.displayIOException(e, listener);
e.printStackTrace(listener.fatalError("Unable to parse " + coberturaXmlReport));
}
}
if (result == null) {
logger.println("[uberalls] unable to parse any cobertura results");
CoberturaBuildAction coberturaAction = build.getAction(CoberturaBuildAction.class);
if (coberturaAction == null) {
logger.println("[uberalls] no cobertura results found");
return null;
}
return result;
return coberturaAction.getResult();
}

/**
Expand Down Expand Up @@ -370,11 +325,6 @@ public boolean isUberallsEnabled() {
return uberallsEnabled;
}

@SuppressWarnings("UnusedDeclaration")
public String getCoberturaReportFile () {
return coberturaReportFile;
}

@SuppressWarnings("UnusedDeclaration")
public boolean isCommentWithConsoleLinkOnFailure() {
return commentWithConsoleLinkOnFailure;
Expand All @@ -390,11 +340,4 @@ public String getCommentFile() {
public PhabricatorNotifierDescriptor getDescriptor() {
return (PhabricatorNotifierDescriptor) super.getDescriptor();
}

private static class CoberturaReportFilenameFilter implements FilenameFilter {
public boolean accept(File dir, String name) {
// TODO take this out of an anonymous inner class, create a singleton and use a Regex to match the name
return name.startsWith("uberalls") && name.endsWith(".xml");
}
}
}
5 changes: 0 additions & 5 deletions src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifierDescriptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,9 @@
import hudson.model.AbstractProject;
import hudson.tasks.BuildStepDescriptor;
import hudson.tasks.Publisher;
import hudson.util.FormValidation;
import net.sf.json.JSONObject;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;

import javax.servlet.ServletException;
import java.io.IOException;


/**
* Descriptor for {@link PhabricatorNotifier}. Used as a singleton.
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

import hudson.Plugin;
import hudson.PluginWrapper;
import hudson.model.Hudson;
import jenkins.model.Jenkins;

import java.io.File;

Expand All @@ -38,9 +38,9 @@ public static String getIconPath(String icon) {
if(icon.startsWith("/")) return icon;

// Try plugin images dir, fallback to Hudson images dir
PluginWrapper wrapper = Hudson.getInstance().getPluginManager().getPlugin(PhabricatorPlugin.class);
PluginWrapper wrapper = Jenkins.getInstance().getPluginManager().getPlugin(PhabricatorPlugin.class);

boolean pluginIconExists = (wrapper != null) && new File(wrapper.baseResourceURL.getPath() + "/images/" + icon).exists();
return pluginIconExists ? "/plugin/" + wrapper.getShortName() + "/images/" + icon : Hudson.RESOURCE_PATH + "/images/16x16/" + icon;
return pluginIconExists ? "/plugin/" + wrapper.getShortName() + "/images/" + icon : Jenkins.RESOURCE_PATH + "/images/16x16/" + icon;
}
}
18 changes: 9 additions & 9 deletions src/main/java/com/uber/jenkins/phabricator/PhabricatorPostbuildAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@
public class PhabricatorPostbuildAction implements BuildBadgeAction {
private final String iconPath;
private final String text;
private String color = "#1FBAD6";
private String background = "transparent";
private String border = "0";
private String borderColor = "transparent";
private String link;

private PhabricatorPostbuildAction(String iconPath, String text, String link) {
this.iconPath = iconPath;
private final String color = "#1FBAD6";
private final String background = "transparent";
private final String border = "0";
private final String borderColor = "transparent";
private final String link;

private PhabricatorPostbuildAction(String text, String link) {
this.iconPath = null;
this.text = text;
this.link = link;
}

public static PhabricatorPostbuildAction createShortText(String text, String link) {
return new PhabricatorPostbuildAction(null, text, link);
return new PhabricatorPostbuildAction(text, link);
}

/* Action methods */
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/uber/jenkins/phabricator/PhabricatorPostbuildSummaryAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public PhabricatorPostbuildSummaryAction(String iconPath) {
@Exported public String getIconPath() { return iconPath; }
@Exported public String getText() { return textBuilder.toString(); }

public void appendText(String text, boolean escapeHtml) {
if(escapeHtml) {
public void appendText(String text) {
if(false) {
text = StringEscapeUtils.escapeHtml(text);
}
textBuilder.append(text);
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@

package com.uber.jenkins.phabricator.conduit;

import groovy.json.JsonBuilder;
import hudson.Launcher;
import net.sf.json.JSONNull;
import net.sf.json.JSONObject;
import net.sf.json.groovy.JsonSlurper;

Expand All @@ -35,8 +33,8 @@
import java.util.Map;

public class ArcanistClient {
private String methodName;
private Map<String, String> params;
private final String methodName;
private final Map<String, String> params;

public ArcanistClient(String methodName, Map<String, String> params) {
this.methodName = methodName;
Expand Down
14 changes: 7 additions & 7 deletions src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ public JSONObject postComment(String message, boolean silent, String action) thr
return arc.callConduit(this.launcher.launch(), this.launcher.getStderr());
}

public JSONObject postComment(String message, boolean silent) throws IOException, InterruptedException {
return postComment(message, silent, "none");
public JSONObject postComment(String message) throws IOException, InterruptedException {
return postComment(message, true, "none");
}

/**
Expand Down Expand Up @@ -145,16 +145,16 @@ public String getPhabricatorLink(String phabricatorURL) {

public void decorate(AbstractBuild build, String phabricatorURL) {
// Add a badge next to the build
build.getActions().add(PhabricatorPostbuildAction.createShortText(
build.addAction(PhabricatorPostbuildAction.createShortText(
this.getRevisionID(true),
this.getPhabricatorLink(phabricatorURL)));
// Add some long-form text
this.createSummary(build, "phabricator.png").appendText(this.getSummaryMessage(phabricatorURL), false);
this.createSummary(build).appendText(this.getSummaryMessage(phabricatorURL));
}

private PhabricatorPostbuildSummaryAction createSummary(final AbstractBuild build, final String icon) {
PhabricatorPostbuildSummaryAction action = new PhabricatorPostbuildSummaryAction(icon);
build.getActions().add(action);
private PhabricatorPostbuildSummaryAction createSummary(final AbstractBuild build) {
PhabricatorPostbuildSummaryAction action = new PhabricatorPostbuildSummaryAction("phabricator.png");
build.addAction(action);
return action;
}

Expand Down
14 changes: 8 additions & 6 deletions src/main/java/com/uber/jenkins/phabricator/uberalls/UberallsClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import net.sf.json.JSONNull;
import net.sf.json.JSONObject;
import net.sf.json.groovy.JsonSlurper;
import org.apache.commons.httpclient.HttpClient;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.HttpResponseException;
import org.apache.http.client.fluent.Request;
Expand All @@ -46,7 +48,7 @@ public class UberallsClient {
public static final String CONDITIONAL_COVERAGE_KEY = "conditionalCoverage";

private final EnvVars environment;
private String baseURL;
private final String baseURL;
private final PrintStream logger;

public boolean recordCoverage(String currentSHA, String branch, CodeCoverageMetrics codeCoverageMetrics) {
Expand Down Expand Up @@ -96,7 +98,7 @@ public CodeCoverageMetrics getParentCoverage(Differential differential) {
String sha = differential.getBaseCommit();
String coverageJSON = null;
if (sha != null) {
coverageJSON = getCoverage("sha", sha);
coverageJSON = getCoverage(sha);
}
JsonSlurper jsonParser = new JsonSlurper();
JSON responseJSON = jsonParser.parseText(coverageJSON);
Expand All @@ -120,15 +122,15 @@ public CodeCoverageMetrics getParentCoverage(Differential differential) {
return null;
}

public String getCoverage(String param, String value) {
public String getCoverage(String value) {
URIBuilder builder;
try {
builder = getBuilder()
.setParameter("repository", this.environment.get("GIT_URL"))
.setParameter(param, value);
.setParameter("sha", value);

return Request.Get(builder.build())
.execute().returnContent().asString();
GetMethod instance = new GetMethod(builder.build().toString());
return instance.getResponseBodyAsString();
} catch (URISyntaxException e) {
e.printStackTrace(logger);
} catch (HttpResponseException e) {
Expand Down
3 changes: 0 additions & 3 deletions src/main/resources/com/uber/jenkins/phabricator/PhabricatorNotifier/config.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
description="Post a differential comment on successful builds">
<f:checkbox />
</f:entry>
<f:entry title="Uberalls Cobertura XML" field="coberturaReportFile">
<f:textbox default="coverage.xml" />
</f:entry>
<f:entry title="Enable Uberalls" field="uberallsEnabled"
description="Enable code coverage reporting">
<f:checkbox default="true" />
Expand Down

0 comments on commit 59b1932

Please sign in to comment.