From: Chris Jaekl Date: Sat, 9 Jan 2016 13:43:44 +0000 (+0900) Subject: Address some edge cases related to bootstrapping a fresh system. X-Git-Url: https://jaekl.net/gitweb/?a=commitdiff_plain;h=7ef6c36cd147216c5354082b11aa33cf6b5c6f49;p=cfb.git Address some edge cases related to bootstrapping a fresh system. Improve diagnostics when command line is not quite correct. --- diff --git a/prod/cfb.properties b/prod/cfb.properties index e9876ea..c7059d6 100644 --- a/prod/cfb.properties +++ b/prod/cfb.properties @@ -12,8 +12,11 @@ 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 +invoke.with.help.for.help=Use the command-line option "--help" for help. +must.specify.fbp.file=No FindBugs Project (.fbp) file was specified. Unable to continue. new.bugs=New Bugs new.version=New Version +no.earlier.run=(none) num.bugs={0} num.bugs.old={0} (common to both versions) old.bugs=Old Bugs diff --git a/prod/net/jaekl/cfb/CFB.java b/prod/net/jaekl/cfb/CFB.java index c6c995d..eeb2a47 100644 --- a/prod/net/jaekl/cfb/CFB.java +++ b/prod/net/jaekl/cfb/CFB.java @@ -94,6 +94,7 @@ public class CFB { opt.addOption("d", "dbname", true, "DB name"); opt.addOption(null, "drop-tables", false, "Remove database schema (drop all data)"); opt.addOption("f", "fbp", true, "FindBugsProject file"); + opt.addOption(null, "help", false, "Display help message and exit"); opt.addOption("h", "host", true, "DB hostname"); opt.addOption("j", "project", true, "proJect name"); opt.addOption("n", "number", true, "Build number (version)"); @@ -110,6 +111,7 @@ public class CFB { try { CommandLine line = new GnuParser().parse(opt, args); + if (line.hasOption("c")) { m_configFile = new File(line.getOptionValue("c")); } @@ -122,6 +124,10 @@ public class CFB { if (line.hasOption("h")) { m_config.setDbHost(line.getOptionValue("h")); } + if (line.hasOption("help")) { + usage(pw, opt); + return false; + } if (line.hasOption("j")) { m_projName = line.getOptionValue("j"); } @@ -147,9 +153,31 @@ public class CFB { return false; } + // Check for required parameters + if (m_removeSchema) { + // No other parameters required + return true; + } + if (null == m_fbp) { + pw.println(trans(CfbBundle.MUST_SPECIFY_FBP_FILE)); + pw.println(trans(CfbBundle.INVOKE_WITH_HELP_FOR_HELP)); + return false; + } + if (null == m_projName) { + m_projName = m_fbp.getName(); + if (m_projName.endsWith(".fbp")) { + m_projName = m_projName.substring(0, m_projName.length() - 4); + } + } + return true; } + // Note that this leverages commons-cli's HelpFormatter to + // generate the usage message. It will always be in English. + // If we want to localize that, we'd need to recode this, + // and also translate the parameter descriptions in + // createOptions(). void usage(PrintWriter pw, Options opt) { HelpFormatter help = new HelpFormatter(); help.printHelp(pw, 80, getClass().getName(), "", opt, 0, 0, "", true); @@ -159,11 +187,6 @@ public class CFB { return getBundle(m_locale).get(key, params); } - String getenv(String varName) { - // This is a separate function so that we can override it at unit test time - return System.getenv(varName); - } - String getProperty(String propName) { // This is a separate function so that we can override it at unit test time return System.getProperty(propName); @@ -174,7 +197,7 @@ public class CFB { } void initArgs() { - String findBugsDir = getenv("FINDBUGS_HOME"); + String findBugsDir = Env.get("FINDBUGS_HOME"); if (null != findBugsDir) { m_fbDir = new File(findBugsDir); } diff --git a/prod/net/jaekl/cfb/CfbBundle.java b/prod/net/jaekl/cfb/CfbBundle.java index 3914b6d..43f0905 100644 --- a/prod/net/jaekl/cfb/CfbBundle.java +++ b/prod/net/jaekl/cfb/CfbBundle.java @@ -27,8 +27,11 @@ public class CfbBundle { 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 INVOKE_WITH_HELP_FOR_HELP = "invoke.with.help.for.help"; + public static final String MUST_SPECIFY_FBP_FILE = "must.specify.fbp.file"; public static final String NEW_BUGS = "new.bugs"; public static final String NEW_VERSION = "new.version"; + public static final String NO_EARLIER_RUN = "no.earlier.run"; public static final String NUM_BUGS = "num.bugs"; public static final String NUM_BUGS_OLD = "num.bugs.old"; public static final String OLD_BUGS = "old.bugs"; diff --git a/prod/net/jaekl/cfb/analyze/Notifier.java b/prod/net/jaekl/cfb/analyze/Notifier.java index bf92fc1..7fdcfd3 100644 --- a/prod/net/jaekl/cfb/analyze/Notifier.java +++ b/prod/net/jaekl/cfb/analyze/Notifier.java @@ -6,6 +6,7 @@ import java.util.ArrayList; import net.jaekl.cfb.CfbBundle; import net.jaekl.cfb.Config; +import net.jaekl.cfb.store.Run; import net.jaekl.qd.util.MailException; import net.jaekl.qd.util.MimePart; import net.jaekl.qd.util.SendMail; @@ -27,10 +28,31 @@ public class Notifier { sendEmail(pw, report); } } + + // --- end of public interface --- - void sendEmail(PrintWriter pw, HtmlReport report) { - SendMail sendMail = new SendMail(); + String constructSubject(HtmlReport report) { + String earlier; + Run earlierRun = report.getDelta().getEarlier(); + if (null == earlierRun) { + earlier = m_bundle.get(CfbBundle.NO_EARLIER_RUN); + } + else { + earlier = report.getDelta().getEarlier().constructVersionText(m_bundle); + } + String later = report.getDelta().getLater().constructVersionText(m_bundle); + + String subject = m_bundle.get(CfbBundle.CFB_MAIL_SUBJECT, earlier, later); + return subject; + } + + SendMail createSendMail() { + return new SendMail(); + } + + void sendEmail(PrintWriter pw, HtmlReport report) { + SendMail sendMail = createSendMail(); sendMail.setSmtpHost(m_config.getMailSmtpHost()); ArrayList recipients = m_config.getNotify(); @@ -41,11 +63,8 @@ public class Notifier { PrintWriter mailWriter = null; try { - String earlier = report.getDelta().getEarlier().constructVersionText(m_bundle); - String later = report.getDelta().getLater().constructVersionText(m_bundle); - + sendMail.setSubject(constructSubject(report)); sendMail.setFrom(m_config.getMailFrom()); - sendMail.setSubject(m_bundle.get(CfbBundle.CFB_MAIL_SUBJECT, earlier, later)); for (String recipient : recipients) { sendMail.addTo(recipient); diff --git a/prod/net/jaekl/cfb/store/DbStore.java b/prod/net/jaekl/cfb/store/DbStore.java index 1ab2022..bbf2a72 100644 --- a/prod/net/jaekl/cfb/store/DbStore.java +++ b/prod/net/jaekl/cfb/store/DbStore.java @@ -50,7 +50,6 @@ public class DbStore { } assert (null != analysis.getProjectName()); - assert (null != analysis.getBuildNumber()); // ---------------------------------- // Add a run record for this analysis diff --git a/test/net/jaekl/cfb/CFBTest.java b/test/net/jaekl/cfb/CFBTest.java index 6d44667..1e70fb2 100644 --- a/test/net/jaekl/cfb/CFBTest.java +++ b/test/net/jaekl/cfb/CFBTest.java @@ -1,6 +1,7 @@ package net.jaekl.cfb; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -107,6 +108,53 @@ public class CFBTest { m_cfb = new CFBMock(Locale.getDefault(), m_driver); } + @Test + public void testParseArgs_noParams() throws IOException + { + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + String[] args = {}; + + boolean result = m_cfb.parseArgs(pw, args); + pw.close(); + sw.close(); + + assertFalse(result); + assertEquals("[must.specify.fbp.file]\n[invoke.with.help.for.help]\n", sw.toString()); + } + + @Test + public void testParseArgs_dropTables() throws IOException + { + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + String[] args = { "--drop-tables" }; + + boolean result = m_cfb.parseArgs(pw, args); + pw.close(); + sw.close(); + + assertTrue(result); + assertEquals("", sw.toString()); + } + + @Test + public void testParseArgs_invalidParam() throws IOException + { + StringWriter sw = new StringWriter(); + PrintWriter pw = new PrintWriter(sw); + String[] args = {"--xyzzyaoeuidhtnsl"}; + + boolean result = m_cfb.parseArgs(pw, args); + pw.close(); + sw.close(); + String actual = sw.toString(); + + assertFalse(result); + assertTrue(actual.contains("usage")); + assertTrue(actual.contains("--help")); + } + @Test public void testStoreAndReport_noPrior() throws IOException, SQLException, SAXException, TypeMismatchException { diff --git a/test/net/jaekl/cfb/analyze/NotifierMock.java b/test/net/jaekl/cfb/analyze/NotifierMock.java new file mode 100644 index 0000000..8198d32 --- /dev/null +++ b/test/net/jaekl/cfb/analyze/NotifierMock.java @@ -0,0 +1,57 @@ +package net.jaekl.cfb.analyze; + +import net.jaekl.cfb.CfbBundle; +import net.jaekl.cfb.Config; +import net.jaekl.qd.util.SendMail; +import net.jaekl.qd.util.SendMailMock; +import net.jaekl.qd.util.SmtpConversationMock; + +public class NotifierMock extends Notifier { + private final String TYPICAL_INIT = "220 BBN-UNIX.ARPA Simple Mail Transfer Service Ready"; + private final String[][] TYPICAL = { + { + "HELO ", + "250 BBN-UNIX.ARPA" + }, + { + "MAIL FROM:", + "250 OK" + }, + { + "RCPT TO:", + "250 OK" + }, + { + "DATA", + "354 Start mail input; end with ." + }, + { + ".\r\n", + "250 OK" + }, + { + "QUIT", + "221 BBN-UNIX.ARPA Service closing transmission channel" + } + }; + + private SendMailMock mock_sendMail; + private SmtpConversationMock mock_conversat; + + public NotifierMock(CfbBundle bundle, Config config) { + super(bundle, config); + + mock_sendMail = null; + } + + @Override + SendMail createSendMail() { + mock_conversat = new SmtpConversationMock(TYPICAL_INIT, TYPICAL); + mock_sendMail = new SendMailMock(mock_conversat); + return mock_sendMail; + } + + public SendMailMock mock_getSendMail() { + return mock_sendMail; + } +} diff --git a/test/net/jaekl/cfb/analyze/NotifierTest.java b/test/net/jaekl/cfb/analyze/NotifierTest.java new file mode 100644 index 0000000..e6cee95 --- /dev/null +++ b/test/net/jaekl/cfb/analyze/NotifierTest.java @@ -0,0 +1,78 @@ +package net.jaekl.cfb.analyze; + +import static org.junit.Assert.assertTrue; + +import java.io.IOException; + +import net.jaekl.cfb.CfbBundleMock; +import net.jaekl.cfb.Config; + +import org.junit.Before; +import org.junit.Test; + +public class NotifierTest { + private CfbBundleMock m_bundle; + private Config m_config; + private MessageMapMock m_msgMap; + private NotifierMock m_notifier; + + @Before + public void setUp() { + m_bundle = new CfbBundleMock(); + m_config = new Config(); + m_msgMap = new MessageMapMock(); + m_notifier = new NotifierMock(m_bundle, m_config); + } + + @Test + public void testConstructSubject_noPrior() throws IOException { + String[][] data = { + { + "ProjectNameGoesHere", + "0.7.42-SNAPSHOT", + "[cfb.mail.subject][[no.earlier.run]][[version.num][", + "][0.7.42-SNAPSHOT]]" + }, + { + "ProjectNameGoesHere", + null, + "[cfb.mail.subject][[no.earlier.run]][[analyzed.at][", + "]]" + }, + { + null, + "0.7.42-SNAPSHOT", + "[cfb.mail.subject][[no.earlier.run]][[version.num][", + "][0.7.42-SNAPSHOT]]" + }, + { + null, + null, + "[cfb.mail.subject][[no.earlier.run]][[analyzed.at][", + "]]" + } + }; + + for (String[] datum : data) { + String projectName = datum[0]; + String buildNumber = datum[1]; + String expectedPart1 = datum[2]; + String expectedPart2 = datum[3]; + + Analysis first = null; + Analysis second = new Analysis(projectName, buildNumber); + + Delta delta = new Delta(first, second); + HtmlReport htmlReport = new HtmlReport(m_bundle, m_msgMap.getColl(), delta); + + String actual = m_notifier.constructSubject(htmlReport); + boolean pass = (actual.startsWith(expectedPart1)) && (actual.endsWith(expectedPart2)); + if (!pass) { + System.out.println("ProjectName:\n" + projectName + "\nVersion:\n" + buildNumber + + "\nExpected:\n" + expectedPart1 + "{date}" + expectedPart2 + "\nActual:\n" + actual); + } + assertTrue(pass); + } + } + +}