From e8190c8189a5270ada70aaa478409db6dbf1efae Mon Sep 17 00:00:00 2001 From: Chris Jaekl Date: Wed, 30 Dec 2015 15:11:50 +0900 Subject: [PATCH] Add unit tests. Make DbStore handle cases where the bug type or category is not present in the message collection more gracefully. --- prod/cfb.properties | 2 + prod/net/jaekl/cfb/CFB.java | 5 + prod/net/jaekl/cfb/CfbBundle.java | 2 + prod/net/jaekl/cfb/store/DbStore.java | 15 +- .../jaekl/cfb/xml/messages/BugCategory.java | 9 + .../jaekl/cfb/xml/messages/BugPattern.java | 8 + .../cfb/xml/messages/MessageCollection.java | 16 +- test/net/jaekl/cfb/store/DbStoreTest.java | 159 +++++++++++++++++- test/net/jaekl/cfb/xml/BugInstanceTest.java | 2 +- test/net/jaekl/cfb/xml/MessagesXmlData.java | 39 ++++- 10 files changed, 245 insertions(+), 12 deletions(-) diff --git a/prod/cfb.properties b/prod/cfb.properties index 7ddb59d..f27ec18 100644 --- a/prod/cfb.properties +++ b/prod/cfb.properties @@ -1,5 +1,7 @@ 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.exec=Got result code {1} when attempting to execute command-line: {0} cannot.send.mail=Attempt to send email to {0} failed: {1} diff --git a/prod/net/jaekl/cfb/CFB.java b/prod/net/jaekl/cfb/CFB.java index 63271ec..639a731 100644 --- a/prod/net/jaekl/cfb/CFB.java +++ b/prod/net/jaekl/cfb/CFB.java @@ -29,6 +29,7 @@ import net.jaekl.cfb.db.TypeMismatchException; 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.qd.xml.XmlParseException; import org.apache.commons.cli.CommandLine; @@ -235,6 +236,10 @@ public class CFB { Notifier notifier = new Notifier(m_bundle, m_config); notifier.sendEmailIfNeeded(pw, report); } + catch (StoreException exc) { + exc.printStackTrace(pw); + return; + } catch (SQLException exc) { reportUnableToConnect(pw, exc); return; diff --git a/prod/net/jaekl/cfb/CfbBundle.java b/prod/net/jaekl/cfb/CfbBundle.java index 0cb7a40..5f5aa9e 100644 --- a/prod/net/jaekl/cfb/CfbBundle.java +++ b/prod/net/jaekl/cfb/CfbBundle.java @@ -13,6 +13,8 @@ import net.jaekl.qd.QDBundleFactory; public class CfbBundle { public static final String ANALYSIS_FAILED = "analysis.failed"; public static final String ANALYZED_AT = "analyzed.at"; + public static final String BUG_CATEGORY_UNKNOWN = "bug.category.unknown"; + 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_SEND_MAIL = "cannot.send.mail"; diff --git a/prod/net/jaekl/cfb/store/DbStore.java b/prod/net/jaekl/cfb/store/DbStore.java index 3860ca5..ed22291 100644 --- a/prod/net/jaekl/cfb/store/DbStore.java +++ b/prod/net/jaekl/cfb/store/DbStore.java @@ -3,7 +3,9 @@ package net.jaekl.cfb.store; import java.sql.Connection; import java.sql.SQLException; import java.util.List; +import java.util.Locale; +import net.jaekl.cfb.CfbBundle; import net.jaekl.cfb.analyze.Analysis; import net.jaekl.cfb.db.CfbSchema; import net.jaekl.cfb.db.Column; @@ -17,6 +19,8 @@ import net.jaekl.cfb.db.driver.DbDriver; import net.jaekl.cfb.xml.BugCollection; import net.jaekl.cfb.xml.BugInstance; import net.jaekl.cfb.xml.LocalVariable; +import net.jaekl.cfb.xml.messages.BugCategory; +import net.jaekl.cfb.xml.messages.BugPattern; import net.jaekl.cfb.xml.messages.MessageCollection; public class DbStore { @@ -42,7 +46,9 @@ public class DbStore { return getAnalysis(priorId); } - public boolean put(Analysis analysis) throws SQLException, TypeMismatchException { + public boolean put(Analysis analysis) throws SQLException, TypeMismatchException, StoreException { + CfbBundle bundle = CfbBundle.getInst(Locale.getDefault()); + if (null == analysis) { return false; } @@ -82,6 +88,13 @@ public class DbStore { Location secondLoc = (locs.size() > 1) ? locs.get(1) : null; Location thirdLoc = (locs.size() > 2) ? locs.get(2) : null; + if (BugPattern.UNKNOWN.getId() == bugId) { + throw new StoreException(bundle.get(CfbBundle.BUG_TYPE_UNKNOWN, ""+bug.getType())); + } + if (BugCategory.UNKNOWN.getId() == categoryId) { + throw new StoreException(bundle.get(CfbBundle.BUG_CATEGORY_UNKNOWN, ""+bug.getCategory())); + } + values[row][0] = foundId; values[row][1] = runId; values[row][2] = bugId; diff --git a/prod/net/jaekl/cfb/xml/messages/BugCategory.java b/prod/net/jaekl/cfb/xml/messages/BugCategory.java index 757aacf..7adb0b5 100644 --- a/prod/net/jaekl/cfb/xml/messages/BugCategory.java +++ b/prod/net/jaekl/cfb/xml/messages/BugCategory.java @@ -15,6 +15,8 @@ public class BugCategory extends ParseResult { static final String TAG = "BugCategory"; static final String[] INTERNAL = { DESCRIPTION, ABBREVIATION, DETAILS }; static final Object[][] EXTERNAL = { }; + + public static final BugCategory UNKNOWN = new BugCategory(-2); String m_category; String m_descr; @@ -25,9 +27,16 @@ public class BugCategory extends ParseResult { public BugCategory() { super(TAG, INTERNAL, EXTERNAL); + m_id = (-1); m_category = m_descr = m_abbrev = m_details = ""; } + BugCategory(long id) + { + this(); + m_id = (-2); + } + public String getCategory() { return m_category; } public String getDescr() { return m_descr; } public String getAbbrev() { return m_abbrev; } diff --git a/prod/net/jaekl/cfb/xml/messages/BugPattern.java b/prod/net/jaekl/cfb/xml/messages/BugPattern.java index e8e381c..1493ac6 100644 --- a/prod/net/jaekl/cfb/xml/messages/BugPattern.java +++ b/prod/net/jaekl/cfb/xml/messages/BugPattern.java @@ -16,6 +16,8 @@ public class BugPattern extends ParseResult { static final String[] INTERNAL = { SHORT, LONG, DETAILS }; static final Object[][] EXTERNAL = { }; + public static final BugPattern UNKNOWN = new BugPattern(-2); + String m_type; String m_short; String m_long; @@ -25,9 +27,15 @@ public class BugPattern extends ParseResult { public BugPattern() { super(TAG, INTERNAL, EXTERNAL); + m_id = (-1); m_type = m_short = m_long = m_details = ""; } + BugPattern(long id) { + this(); + m_id = id; + } + public String getType() { return m_type; } public String getShort() { return m_short; } public String getLong() { return m_long; } diff --git a/prod/net/jaekl/cfb/xml/messages/MessageCollection.java b/prod/net/jaekl/cfb/xml/messages/MessageCollection.java index ba4b97c..560e04e 100644 --- a/prod/net/jaekl/cfb/xml/messages/MessageCollection.java +++ b/prod/net/jaekl/cfb/xml/messages/MessageCollection.java @@ -23,10 +23,22 @@ public class MessageCollection extends ParseResult { } public Collection getCategories() { return m_categories.values(); } - public BugCategory getCategory(String category) { return m_categories.get(category); } + public BugCategory getCategory(String category) { + BugCategory cat = m_categories.get(category); + if (null == cat) { + cat = BugCategory.UNKNOWN; + } + return cat; + } public Collection getPatterns() { return m_patterns.values(); } - public BugPattern getPattern(String type) { return m_patterns.get(type); } + public BugPattern getPattern(String type) { + BugPattern pat = m_patterns.get(type); + if (null == pat) { + pat = BugPattern.UNKNOWN; + } + return pat; + } @Override public void endContents(String uri, String localName, String qName, String chars) diff --git a/test/net/jaekl/cfb/store/DbStoreTest.java b/test/net/jaekl/cfb/store/DbStoreTest.java index c4e2668..f269a9c 100644 --- a/test/net/jaekl/cfb/store/DbStoreTest.java +++ b/test/net/jaekl/cfb/store/DbStoreTest.java @@ -1,11 +1,17 @@ package net.jaekl.cfb.store; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.UnsupportedEncodingException; +import java.nio.charset.Charset; import java.sql.SQLException; import java.util.Date; @@ -15,15 +21,103 @@ import net.jaekl.cfb.db.CfbSchema; import net.jaekl.cfb.db.TypeMismatchException; import net.jaekl.cfb.db.driver.ConnectionMock; import net.jaekl.cfb.db.driver.DbDriverMock; +import net.jaekl.cfb.util.Command; +import net.jaekl.cfb.xml.BugCollection; import net.jaekl.cfb.xml.MessagesXmlData; -import net.jaekl.cfb.xml.messages.MessageCollection; import org.junit.Before; import org.junit.Test; import org.xml.sax.InputSource; import org.xml.sax.SAXException; -public class DbStoreTest { +public class DbStoreTest { + private static final String BUG_COLLECTION_XML = "" + + "" + + "" + + "/data/prog/findbugs-3.0.1/lib/junit.jar" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + ""; + + private static final String UNKNOWN_BUG_CATEGORY_XML = "" + + "" + + "" + + "/data/prog/findbugs-3.0.1/lib/junit.jar" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + ""; + + private static final String UNKNOWN_BUG_PATTERN_XML = "" + + "" + + "" + + "/data/prog/findbugs-3.0.1/lib/junit.jar" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + ""; + private DbStore m_store; @Before @@ -39,8 +133,7 @@ public class DbStoreTest { schema.setMessageMap(msgMap); schema.ensureDbInitialized(con); - MessageCollection msgColl = new MessageCollection(); - m_store = new DbStore(con, driver, msgColl); + m_store = new DbStore(con, driver, msgMap.getColl()); } @Test @@ -60,9 +153,59 @@ public class DbStoreTest { actual = m_store.getPrior(current); assertNull(actual); } + + private BugCollection parseBugCollection(String xml) throws IOException, SAXException + { + Charset utf8 = Charset.forName(Command.UTF_8); + + try ( ByteArrayInputStream bais = new ByteArrayInputStream(xml.getBytes(utf8))) + { + InputSource inputSource = new InputSource(bais); + Analysis analysis = new Analysis(null, null); + analysis.parse(inputSource); + + assertNotNull(analysis); + + BugCollection bugColl = analysis.getBugCollection(); + return bugColl; + } + } + + @Test + public void testPut_null() throws StoreException, SQLException, TypeMismatchException + { + // Adding null should return false, with no exception thrown + boolean result = m_store.put(null); + assertFalse(result); + } + + @Test + public void testPut_withUnknownPatternOrCategory() throws SQLException, TypeMismatchException, IOException, SAXException + { + final String[] data = { UNKNOWN_BUG_PATTERN_XML, UNKNOWN_BUG_CATEGORY_XML}; + String projName = "ProjectName"; + String firstVersion = "1.0.1"; + Date firstStart = new Date(100); + Date firstEnd = new Date(200); + Analysis firstAnalysis = new Analysis(projName, firstVersion); + firstAnalysis.setStart(firstStart); + firstAnalysis.setEnd(firstEnd); + + for (String xml : data) { + firstAnalysis.setBugCollection(parseBugCollection(xml)); + + try { + m_store.put(firstAnalysis); + fail("Should have thrown an exception\n" + xml); + } + catch (StoreException exc) { + // This is the success path + } + } + } @Test - public void testPut_thenGetPrior() throws SQLException, TypeMismatchException { + public void testPut_thenGetPrior() throws SQLException, TypeMismatchException, IOException, SAXException, StoreException { String projName = "ProjectName"; String firstVersion = "1.0.1"; Date firstStart = new Date(100); @@ -70,10 +213,13 @@ public class DbStoreTest { Analysis firstAnalysis = new Analysis(projName, firstVersion); firstAnalysis.setStart(firstStart); firstAnalysis.setEnd(firstEnd); + firstAnalysis.setBugCollection(parseBugCollection(BUG_COLLECTION_XML)); + // Adding a valid Analysis object should return true boolean result = m_store.put(firstAnalysis); assertTrue(result); + // Create a second Analysis object String secondVersion = "1.0.2"; Date secondStart = new Date(2300); Date secondEnd = new Date(2400); @@ -81,6 +227,7 @@ public class DbStoreTest { secondAnalysis.setStart(secondStart); secondAnalysis.setEnd(secondEnd); + // Retrieve the first Analysis object Analysis priorAnalysis = m_store.getPrior(secondAnalysis); assertNotNull(priorAnalysis); assertEquals(firstAnalysis.getProjectName(), priorAnalysis.getProjectName()); diff --git a/test/net/jaekl/cfb/xml/BugInstanceTest.java b/test/net/jaekl/cfb/xml/BugInstanceTest.java index b7e98fb..0c734bc 100644 --- a/test/net/jaekl/cfb/xml/BugInstanceTest.java +++ b/test/net/jaekl/cfb/xml/BugInstanceTest.java @@ -9,7 +9,7 @@ import org.junit.Test; public class BugInstanceTest { @Test - public void testGetPrincpalLocation() { + public void testGetPrincipalLocation() { Location[][][] data = { { diff --git a/test/net/jaekl/cfb/xml/MessagesXmlData.java b/test/net/jaekl/cfb/xml/MessagesXmlData.java index 5b81dca..94e3f09 100644 --- a/test/net/jaekl/cfb/xml/MessagesXmlData.java +++ b/test/net/jaekl/cfb/xml/MessagesXmlData.java @@ -5,7 +5,7 @@ public class MessagesXmlData { "\n" + "\n" - + "\n" + + " \n" + " Correctness\n" + " C\n" + "
Probable bug - an apparent coding mistake\n" @@ -222,6 +222,41 @@ public class MessagesXmlData { + "" + " ]]>" + "
" - + " " + + " " + + " " + + " An increment to a volatile field isn't atomic" + + " Increment of volatile field {2} in {1}" + + "
" + + " This code increments a volatile field. Increments of volatile fields aren't" + + " atomic. If more than one thread is incrementing the field at the same time," + + " increments could be lost." + + "

" + + " ]]>" + + "
" + + " " + + " " + + " Method invokes inefficient Number constructor; use static valueOf instead" + + " {1} invokes inefficient {2} constructor; use {3} instead" + + "
" + + " " + + " Using new Integer(int) is guaranteed to always result in a new object whereas" + + " Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM." + + " Using of cached values avoids object allocation and the code will be faster." + + "

" + + "

" + + " Values between -128 and 127 are guaranteed to have corresponding cached instances" + + " and using valueOf is approximately 3.5 times faster than using constructor." + + " For values outside the constant range the performance of both styles is the same." + + "

" + + "

" + + " Unless the class must be compatible with JVMs predating Java 1.5," + + " use either autoboxing or the valueOf() method when creating instances of" + + " Long, Integer, Short, Character, and Byte." + + "

" + + " ]]>" + + "
" + + " " + ""; } -- 2.30.2