From e6448f6cf67e5a5409f24b531c2443b3bed53b52 Mon Sep 17 00:00:00 2001 From: Chris Jaekl Date: Sat, 9 Jan 2016 18:19:52 +0900 Subject: [PATCH] Clean up error handling to make things slightly friendlier on an initial install. --- cov.sh | 2 + go.sh | 4 ++ prod/cfb.properties | 5 +- prod/net/jaekl/cfb/CFB.java | 31 ++++++++- prod/net/jaekl/cfb/CfbBundle.java | 3 + .../analyze/FBMsgFileNotFoundException.java | 16 +++++ prod/net/jaekl/cfb/analyze/HtmlReport.java | 31 ++++----- prod/net/jaekl/cfb/analyze/MessageMap.java | 4 +- prod/net/jaekl/cfb/store/DbStore.java | 3 + prod/net/jaekl/cfb/util/Env.java | 27 ++++++++ test/net/jaekl/cfb/CFBTest.java | 67 +++++++++++++++++++ test/net/jaekl/cfb/CfbBundleTest.java | 33 +++++++++ test/net/jaekl/cfb/util/EnvMock.java | 29 ++++++++ 13 files changed, 232 insertions(+), 23 deletions(-) create mode 100644 prod/net/jaekl/cfb/analyze/FBMsgFileNotFoundException.java create mode 100644 prod/net/jaekl/cfb/util/Env.java create mode 100644 test/net/jaekl/cfb/CfbBundleTest.java create mode 100644 test/net/jaekl/cfb/util/EnvMock.java diff --git a/cov.sh b/cov.sh index fd41983..e442016 100755 --- a/cov.sh +++ b/cov.sh @@ -6,6 +6,8 @@ set -o errexit ## (set -e) error exit if subcommand returns nonzero CFB_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" INSTR_DIR="${CFB_ROOT}/../instr" +. ${CFB_ROOT}/setcp.sh + ##################### echo Compiling... find "${CFB_ROOT}/prod" -name "*.java" | xargs javac -g -Xlint:deprecation diff --git a/go.sh b/go.sh index a1a2406..07853cc 100755 --- a/go.sh +++ b/go.sh @@ -1,8 +1,12 @@ #!/bin/bash CFB_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +. ${CFB_ROOT}/setcp.sh + echo Compiling... find "${CFB_ROOT}/prod" -name "*.java" | xargs javac -g -classpath ${CFB_ROOT}/prod:${CLASSPATH} -Xlint:deprecation cp -r ${CFB_ROOT}/prod/* ${CFB_ROOT}/bin/ find "${CFB_ROOT}/prod" -name '*.class' -exec rm {} \; + echo Launching... +echo java -ea -Djsse.enableSNIExtension=false net.jaekl.cfb.CFB $1 $2 $3 $4 $5 $6 $7 $8 $9 ${10} ${11} ${12} ${13} ${14} ${15} ${16} ${17} ${18} ${19} ${20} java -ea -Djsse.enableSNIExtension=false net.jaekl.cfb.CFB $1 $2 $3 $4 $5 $6 $7 $8 $9 ${10} ${11} ${12} ${13} ${14} ${15} ${16} ${17} ${18} ${19} ${20} diff --git a/prod/cfb.properties b/prod/cfb.properties index f27ec18..e9876ea 100644 --- a/prod/cfb.properties +++ b/prod/cfb.properties @@ -2,12 +2,15 @@ analysis.failed=Attempt to analyze source code failed. Will now stop. analyzed.at=Analyzed at {0} bug.category.unknown=Internal error: The bug category {0} is unknown. Perhaps the Findbugs versions used to generate the report, and the FindBugs messages.xml, do not match? bug.type.unknown=Internal error: The bug pattern type {0} is unknown. Perhaps the Findbugs versions used to generate the report, and eth FindBugs messages.xml, do not match? -cannot.connect.to.db=Unable to connect to, or to initialize, database {2} on {0}:{1} as user {3}. +cannot.connect.to.db=Unable to connect to, or to initialize, database {2} on {0}:{1} as user "{3}". cannot.exec=Got result code {1} when attempting to execute command-line: {0} +cannot.load.fbmsg.file=Unable to load the FindBugs messages file (messages.xml). cannot.send.mail=Attempt to send email to {0} failed: {1} cfb=Comparative FindBugs cfb.mail.subject=CFB: {0} vs. {1} cfb.report=Comparative FindBugs Report +findbugs.home.is.not.set=The environment variable FINDBUGS_HOME is not set. Please set it to the path where FindBugs is installed. +findbugs.home.is.set.to=The environment variable FINDBUGS_HOME is set to {0}. Please ensure that it points to the path where FindBugs is installed. fixed.bugs=Fixed Bugs new.bugs=New Bugs new.version=New Version diff --git a/prod/net/jaekl/cfb/CFB.java b/prod/net/jaekl/cfb/CFB.java index 35a08c3..c6c995d 100644 --- a/prod/net/jaekl/cfb/CFB.java +++ b/prod/net/jaekl/cfb/CFB.java @@ -21,6 +21,7 @@ import java.util.Locale.Category; import net.jaekl.cfb.analyze.Analysis; import net.jaekl.cfb.analyze.Analyzer; import net.jaekl.cfb.analyze.Delta; +import net.jaekl.cfb.analyze.FBMsgFileNotFoundException; import net.jaekl.cfb.analyze.HtmlReport; import net.jaekl.cfb.analyze.MessageMap; import net.jaekl.cfb.analyze.Notifier; @@ -30,6 +31,7 @@ import net.jaekl.cfb.db.driver.DbDriver; import net.jaekl.cfb.db.driver.PostgresqlDriver; import net.jaekl.cfb.store.DbStore; import net.jaekl.cfb.store.StoreException; +import net.jaekl.cfb.util.Env; import net.jaekl.qd.xml.XmlParseException; import org.apache.commons.cli.CommandLine; @@ -40,6 +42,8 @@ import org.apache.commons.cli.ParseException; import org.xml.sax.SAXException; public class CFB { + public static final String FINDBUGS_HOME = "FINDBUGS_HOME"; // name of the FINDBUGS_HOME environment variable + DbDriver m_driver; CfbSchema m_schema; volatile static CfbBundle m_bundle = null; @@ -151,8 +155,8 @@ public class CFB { help.printHelp(pw, 80, getClass().getName(), "", opt, 0, 0, "", true); } - String trans(String key) { - return getBundle(m_locale).get(key); + String trans(String key, Object... params) { + return getBundle(m_locale).get(key, params); } String getenv(String varName) { @@ -196,7 +200,13 @@ public class CFB { File findBugsDir = getFindBugsDir(); File workDir = new File("."); MessageMap messageMap = new MessageMap(); - messageMap.load(findBugsDir, Locale.getDefault(Category.DISPLAY)); + try { + messageMap.load(findBugsDir, Locale.getDefault(Category.DISPLAY)); + } + catch (FBMsgFileNotFoundException exc) { + reportException(pw, exc); + return; + } if (!ensureDbInitialized(pw, messageMap)) { return; @@ -270,6 +280,21 @@ public class CFB { reportUnableToConnect(pw, exc); } } + + void reportException(PrintWriter pw, FBMsgFileNotFoundException exc) { + exc.printStackTrace(pw); + + pw.println(trans(CfbBundle.CANNOT_LOAD_FBMSG_FILE, exc.getFilename())); + + String fbHome = Env.get(FINDBUGS_HOME); + if (null == fbHome) { + pw.println(trans(CfbBundle.FINDBUGS_HOME_IS_NOT_SET, FINDBUGS_HOME)); + } + else { + pw.println(trans(CfbBundle.FINDBUGS_HOME_IS_SET_TO, FINDBUGS_HOME, fbHome)); + } + + } private void reportUnableToConnect(PrintWriter pw, SQLException exc) { String cannotConnectFormat = trans(CfbBundle.CANNOT_CONNECT); diff --git a/prod/net/jaekl/cfb/CfbBundle.java b/prod/net/jaekl/cfb/CfbBundle.java index b46b290..3914b6d 100644 --- a/prod/net/jaekl/cfb/CfbBundle.java +++ b/prod/net/jaekl/cfb/CfbBundle.java @@ -17,12 +17,15 @@ public class CfbBundle { public static final String BUG_TYPE_UNKNOWN = "bug.type.unknown"; public static final String CANNOT_CONNECT = "cannot.connect.to.db"; public static final String CANNOT_EXEC = "cannot.exec"; + public static final String CANNOT_LOAD_FBMSG_FILE = "cannot.load.fbmsg.file"; public static final String CANNOT_SEND_MAIL = "cannot.send.mail"; public static final String CFB = "cfb"; public static final String CFB_MAIL_SUBJECT = "cfb.mail.subject"; public static final String CFB_REPORT = "cfb.report"; public static final String COMPARING_RUNS = "comparing.runs"; public static final String COMPARING_VERSIONS = "comparing.versions"; + public static final String FINDBUGS_HOME_IS_NOT_SET = "findbugs.home.is.not.set"; + public static final String FINDBUGS_HOME_IS_SET_TO = "findbugs.home.is.set.to"; public static final String FIXED_BUGS = "fixed.bugs"; public static final String NEW_BUGS = "new.bugs"; public static final String NEW_VERSION = "new.version"; diff --git a/prod/net/jaekl/cfb/analyze/FBMsgFileNotFoundException.java b/prod/net/jaekl/cfb/analyze/FBMsgFileNotFoundException.java new file mode 100644 index 0000000..46e8a20 --- /dev/null +++ b/prod/net/jaekl/cfb/analyze/FBMsgFileNotFoundException.java @@ -0,0 +1,16 @@ +package net.jaekl.cfb.analyze; + +import net.jaekl.cfb.CfbException; + +public class FBMsgFileNotFoundException extends CfbException { + private static final long serialVersionUID = 1L; + + private String m_filename; + + public FBMsgFileNotFoundException(String filename) { + super(filename); + m_filename = filename; + } + + public String getFilename() { return m_filename; } +} diff --git a/prod/net/jaekl/cfb/analyze/HtmlReport.java b/prod/net/jaekl/cfb/analyze/HtmlReport.java index 39c0796..166a140 100644 --- a/prod/net/jaekl/cfb/analyze/HtmlReport.java +++ b/prod/net/jaekl/cfb/analyze/HtmlReport.java @@ -86,9 +86,7 @@ public class HtmlReport { } } - pw.write(" "); - pw.write(" " + sb.toString() + ""); - pw.write(" "); + pw.println(" " + sb.toString() + ""); } } @@ -103,21 +101,20 @@ public class HtmlReport { for (BugInstance bug : bugs) { BugPattern pattern = m_msgColl.getPattern(bug.getType()); - pw.write("

"); - pw.write(" "); - pw.write(" "); - pw.write(" "); - pw.write(" "); - pw.write(" "); + pw.println("

"); + pw.println("

" + bug.getCategory() + "" + bug.getType() + "
"); + pw.println(" "); + pw.println(" "); + pw.println(" "); + pw.println(" "); writeBugLocations(pw, bug); - pw.write(" "); - pw.write(" "); - pw.write(" "); - pw.write(" "); - pw.write(" "); - pw.write(" "); - pw.write("
" + bug.getCategory() + "" + bug.getType() + "
" + pattern.getShort() + "
" + pattern.getDetails() + "
"); - pw.write("

"); + pw.println(" " + pattern.getShort() + ""); + pw.println(" "); + pw.println(" " + pattern.getDetails() + ""); + pw.println(" "); + pw.println(" "); + pw.println("

"); + pw.println("
"); } } diff --git a/prod/net/jaekl/cfb/analyze/MessageMap.java b/prod/net/jaekl/cfb/analyze/MessageMap.java index f4936b0..635e79d 100644 --- a/prod/net/jaekl/cfb/analyze/MessageMap.java +++ b/prod/net/jaekl/cfb/analyze/MessageMap.java @@ -94,7 +94,7 @@ public class MessageMap { } // Load the list of bug patterns and categories from the FindBugs messages.xml file. - public void load(File findBugsDir, Locale locale) throws FileNotFoundException, IOException, SAXException + public void load(File findBugsDir, Locale locale) throws FBMsgFileNotFoundException, IOException, SAXException { m_findBugsDir = findBugsDir; @@ -108,7 +108,7 @@ public class MessageMap { } if (! msgXml.canRead()) { - throw new FileNotFoundException(msgXml.getAbsolutePath()); + throw new FBMsgFileNotFoundException(msgXml.getAbsolutePath()); } parse(new InputSource(new FileInputStream(msgXml))); diff --git a/prod/net/jaekl/cfb/store/DbStore.java b/prod/net/jaekl/cfb/store/DbStore.java index 917dc48..1ab2022 100644 --- a/prod/net/jaekl/cfb/store/DbStore.java +++ b/prod/net/jaekl/cfb/store/DbStore.java @@ -49,6 +49,9 @@ public class DbStore { return false; } + assert (null != analysis.getProjectName()); + assert (null != analysis.getBuildNumber()); + // ---------------------------------- // Add a run record for this analysis diff --git a/prod/net/jaekl/cfb/util/Env.java b/prod/net/jaekl/cfb/util/Env.java new file mode 100644 index 0000000..bda8405 --- /dev/null +++ b/prod/net/jaekl/cfb/util/Env.java @@ -0,0 +1,27 @@ +package net.jaekl.cfb.util; + +public class Env { + static volatile Env m_inst = null; + + Env() {} + + private static Env getInstance() + { + Env inst = m_inst; + if (null == inst) { + synchronized(Env.class) { + if (null == m_inst) { + m_inst = new Env(); + } + inst = m_inst; + } + } + return inst; + } + + String getEnv(String variableName) { return System.getenv(variableName); } + + public static String get(String variableName) { + return getInstance().getEnv(variableName); + } +} diff --git a/test/net/jaekl/cfb/CFBTest.java b/test/net/jaekl/cfb/CFBTest.java index de10fe7..08d9c39 100644 --- a/test/net/jaekl/cfb/CFBTest.java +++ b/test/net/jaekl/cfb/CFBTest.java @@ -2,6 +2,7 @@ package net.jaekl.cfb; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -9,12 +10,14 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.OutputStreamWriter; import java.io.PrintWriter; +import java.io.StringWriter; import java.nio.charset.Charset; import java.sql.Connection; import java.sql.SQLException; import java.util.Locale; import net.jaekl.cfb.analyze.Analysis; +import net.jaekl.cfb.analyze.FBMsgFileNotFoundException; import net.jaekl.cfb.analyze.MessageMap; import net.jaekl.cfb.db.CfbSchema; import net.jaekl.cfb.db.Column; @@ -25,6 +28,7 @@ import net.jaekl.cfb.db.Table; import net.jaekl.cfb.db.TypeMismatchException; import net.jaekl.cfb.db.driver.DbDriverMock; import net.jaekl.cfb.util.Command; +import net.jaekl.cfb.util.EnvMock; import net.jaekl.cfb.xml.MessagesXmlData; import org.junit.Before; @@ -133,4 +137,67 @@ public class CFBTest { assertEquals(PROJECT_NAME, row.getValue(1)); assertEquals(VERSION, row.getValue(2)); } + + @Test + public void testTrans() + { + Object[][] data = { + { "hello", new Object[] {}, "[hello]" }, + { "hello", new Object[] {"world"}, "[hello][world]"} + }; + + for (Object[] datum : data) { + String key = (String)datum[0]; + Object[] params = (Object[])datum[1]; + String expected = (String)datum[2]; + String actual = m_cfb.trans(key, params); + assertEquals(expected, actual); + } + } + + @Test + public void testReportException_FBMsgFileNotFoundException() throws IOException + { + String[][] data = { + { + "/home/fred/findbugs3.0.1", "messages.xml", + "[cannot.load.fbmsg.file][messages.xml]\n[findbugs.home.is.set.to][FINDBUGS_HOME][/home/fred/findbugs3.0.1]\n" + }, + { + null, "messages.xml", + "[cannot.load.fbmsg.file][messages.xml]\n[findbugs.home.is.not.set][FINDBUGS_HOME]\n" + }, + { + null, null, "[cannot.load.fbmsg.file][null]\n[findbugs.home.is.not.set][FINDBUGS_HOME]\n" + } + }; + EnvMock envMock = EnvMock.mock_putInstance(); + + for (String[] datum : data) + { + String fbHome = datum[0]; + String filename = datum[1]; + String expected = datum[2]; + + try ( + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + ) + { + envMock.mock_putEnv(CFB.FINDBUGS_HOME, fbHome); + FBMsgFileNotFoundException exc = new FBMsgFileNotFoundException(filename); + m_cfb.reportException(pw, exc); + + pw.close(); + sw.close(); + + String actual = sw.toString(); + boolean pass = actual.endsWith(expected); + if (!pass) { + System.out.println("Expected:\n" + expected + "\nActual:\n" + actual); + } + assertTrue(pass); + } + } + } } diff --git a/test/net/jaekl/cfb/CfbBundleTest.java b/test/net/jaekl/cfb/CfbBundleTest.java new file mode 100644 index 0000000..db6a899 --- /dev/null +++ b/test/net/jaekl/cfb/CfbBundleTest.java @@ -0,0 +1,33 @@ +package net.jaekl.cfb; + +import static org.junit.Assert.*; + +import org.junit.Test; + +public class CfbBundleTest { + + @Test + public void testFallbackGet() { + Object[][] data = { + { "hello", new Object[] {}, "[hello]" }, + { "hello", new Object[] {"world"}, "[hello][world]"}, + { null, new Object[] {}, "[null]" }, + { + "the.quick.brown.fox", + new Object[] {"jumps.over", Integer.valueOf(1), "lazy", "dog"}, + "[the.quick.brown.fox][jumps.over][1][lazy][dog]" + } + }; + + CfbBundleMock bundle = new CfbBundleMock(); + + for (Object[] datum : data) { + String key = (String)datum[0]; + Object[] params = (Object[])datum[1]; + String expected = (String)datum[2]; + String actual = bundle.fallbackGet(key, params); + assertEquals(expected, actual); + } + } + +} diff --git a/test/net/jaekl/cfb/util/EnvMock.java b/test/net/jaekl/cfb/util/EnvMock.java new file mode 100644 index 0000000..cabf42f --- /dev/null +++ b/test/net/jaekl/cfb/util/EnvMock.java @@ -0,0 +1,29 @@ +package net.jaekl.cfb.util; + +import java.util.HashMap; + +public class EnvMock extends Env { + private HashMap m_map; + + EnvMock() { + m_map = new HashMap(); + } + + @Override + String getEnv(String variableName) { + if (m_map.containsKey(variableName)) { + return m_map.get(variableName); + } + return super.getEnv(variableName); + } + + public static EnvMock mock_putInstance() { + EnvMock mock = new EnvMock(); + m_inst = mock; + return mock; + } + + public void mock_putEnv(String variableName, String value) { + m_map.put(variableName, value); + } +} -- 2.30.2