Skip to content

Commit

Permalink
Remove assumption that strings are always intern
Browse files Browse the repository at this point in the history 
The pre-existing code assumed that string (qName) passed in
to start and end element was always interned and compared
those strings to constants using ==. Instead rewrite
code using switch statements on the strings.

Also includes small change to the exception handling so that
code analysis can correctly where exceptions are always thrown.
i.e. instead of hiding a throw in an always throwing method,
return the exception and throw it at the call site.
  • Loading branch information
@jonahgraham
jonahgraham committed Aug 14, 2023
1 parent 1e04efa commit 754bd7d
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 52 deletions.
2 changes: 1 addition & 1 deletion testsrunner/org.eclipse.cdt.testsrunner.boost/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.cdt.testsrunner.boost;singleton:=true
Bundle-Version: 7.2.0.qualifier
Bundle-Version: 7.2.100.qualifier
Bundle-Activator: org.eclipse.cdt.testsrunner.internal.boost.BoostTestsRunnerPlugin
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
114 changes: 63 additions & 51 deletions ....testsrunner.boost/src/org/eclipse/cdt/testsrunner/internal/boost/BoostXmlLogHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
package org.eclipse.cdt.testsrunner.internal.boost;

import java.text.MessageFormat;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Stack;

import org.eclipse.cdt.testsrunner.model.ITestItem;
Expand Down Expand Up @@ -56,19 +53,6 @@ public class BoostXmlLogHandler extends DefaultHandler {
private static final String XML_ATTR_MESSAGE_FILE = "file"; //$NON-NLS-1$
private static final String XML_ATTR_MESSAGE_LINE = "line"; //$NON-NLS-1$

/** Maps the string message level representation to the Tests Runner internal enum code. */
private static final Map<String, ITestMessage.Level> STRING_TO_MESSAGE_LEVEL;
static {
Map<String, ITestMessage.Level> aMap = new HashMap<>();
aMap.put(XML_NODE_INFO, ITestMessage.Level.Info);
aMap.put(XML_NODE_MESSAGE, ITestMessage.Level.Message);
aMap.put(XML_NODE_WARNING, ITestMessage.Level.Warning);
aMap.put(XML_NODE_ERROR, ITestMessage.Level.Error);
aMap.put(XML_NODE_FATAL_ERROR, ITestMessage.Level.FatalError);
// NOTE: Exception node is processed separately
STRING_TO_MESSAGE_LEVEL = Collections.unmodifiableMap(aMap);
}

/** The default file name for test message location. */
private static final String DEFAULT_LOCATION_FILE = null;

Expand Down Expand Up @@ -106,12 +90,18 @@ public class BoostXmlLogHandler extends DefaultHandler {
public void startElement(String namespaceURI, String localName, String qName, Attributes attrs)
throws SAXException {

if (qName == null) {
throw createAndLogExceptionForElement(qName);
}

elementDataStack.push(new StringBuilder());
if (qName == XML_NODE_TEST_SUITE) {
switch (qName) {
case XML_NODE_TEST_SUITE:
String testSuiteName = attrs.getValue(XML_ATTR_TEST_SUITE_NAME);
modelUpdater.enterTestSuite(testSuiteName);
break;

} else if (qName == XML_NODE_TEST_CASE) {
case XML_NODE_TEST_CASE:
String testCaseName = attrs.getValue(XML_ATTR_TEST_CASE_NAME);

if (lastTestCaseName.equals(testCaseName)) {
Expand All @@ -124,23 +114,31 @@ public void startElement(String namespaceURI, String localName, String qName, At

modelUpdater.enterTestCase(testCaseName);
testStatus = Status.Passed;

} else if (STRING_TO_MESSAGE_LEVEL.containsKey(qName) || qName == XML_NODE_LAST_CHECKPOINT) {
break;

case XML_NODE_INFO:
case XML_NODE_MESSAGE:
case XML_NODE_WARNING:
case XML_NODE_ERROR:
case XML_NODE_FATAL_ERROR:
case XML_NODE_LAST_CHECKPOINT:
fileName = attrs.getValue(XML_ATTR_MESSAGE_FILE);
String lineNumberStr = attrs.getValue(XML_ATTR_MESSAGE_LINE);
lineNumber = lineNumberStr != null ? Integer.parseInt(lineNumberStr.trim()) : DEFAULT_LOCATION_LINE;
break;

} else if (qName == XML_NODE_EXCEPTION) {
case XML_NODE_EXCEPTION:
fileName = DEFAULT_LOCATION_FILE;
lineNumber = DEFAULT_LOCATION_LINE;
break;

} else if (qName == XML_NODE_TESTING_TIME) {

} else if (qName == XML_NODE_TEST_LOG) {
case XML_NODE_TESTING_TIME:
case XML_NODE_TEST_LOG:
/* just skip, do nothing */
break;

} else {
logAndThrowErrorForElement(qName);
default:
throw createAndLogExceptionForElement(qName);
}
}

Expand All @@ -167,30 +165,53 @@ private void addCurrentMessage(ITestMessage.Level level) {
@Override
public void endElement(String namespaceURI, String localName, String qName) throws SAXException {

if (qName == XML_NODE_TEST_SUITE) {
if (qName == null) {
throw createAndLogExceptionForElement(qName);
}
switch (qName) {
case XML_NODE_TEST_SUITE:
modelUpdater.exitTestSuite();
break;

} else if (qName == XML_NODE_TEST_CASE) {
case XML_NODE_TEST_CASE:
modelUpdater.setTestStatus(testStatus);
modelUpdater.exitTestCase();
break;

} else if (qName == XML_NODE_TESTING_TIME) {
case XML_NODE_TESTING_TIME:
modelUpdater.setTestingTime(Integer.parseInt(elementDataStack.peek().toString().trim()) / 1000);

} else if (STRING_TO_MESSAGE_LEVEL.containsKey(qName)) {
addCurrentMessage(STRING_TO_MESSAGE_LEVEL.get(qName));

} else if (qName == XML_NODE_EXCEPTION) {
break;

case XML_NODE_INFO:
addCurrentMessage(ITestMessage.Level.Info);
break;
case XML_NODE_MESSAGE:
addCurrentMessage(ITestMessage.Level.Message);
break;
case XML_NODE_WARNING:
addCurrentMessage(ITestMessage.Level.Warning);
break;
case XML_NODE_ERROR:
addCurrentMessage(ITestMessage.Level.Error);
break;
case XML_NODE_FATAL_ERROR:
addCurrentMessage(ITestMessage.Level.FatalError);
break;

case XML_NODE_EXCEPTION:
if (fileName != DEFAULT_LOCATION_FILE && !fileName.isEmpty() && lineNumber >= 0) {
elementDataStack.peek().append(BoostTestsRunnerMessages.BoostXmlLogHandler_exception_suffix);
}
addCurrentMessage(ITestMessage.Level.Exception);
break;

} else if (qName == XML_NODE_TEST_LOG || qName == XML_NODE_LAST_CHECKPOINT) {
case XML_NODE_TEST_LOG:
case XML_NODE_LAST_CHECKPOINT:
/* just skip, do nothing */
break;

} else {
logAndThrowErrorForElement(qName);
default:
throw createAndLogExceptionForElement(qName);
}
elementDataStack.pop();
}
Expand All @@ -207,22 +228,13 @@ public void characters(char[] ch, int start, int length) {
* Throws the testing exception for the specified XML tag.
*
* @param tagName XML tag name
* @throws SAXException the exception that will be thrown
*/
private void logAndThrowErrorForElement(String tagName) throws SAXException {
logAndThrowError(MessageFormat.format(BoostTestsRunnerMessages.BoostXmlLogHandler_wrong_tag_name, tagName));
}

/**
* Throws the testing exception with the specified message.
*
* @param message the reason
* @throws SAXException the exception that will be thrown
* @return SAXException the exception that will be thrown
*/
private void logAndThrowError(String message) throws SAXException {
SAXException e = new SAXException(message);
private SAXException createAndLogExceptionForElement(String tagName) {
SAXException e = new SAXException(
MessageFormat.format(BoostTestsRunnerMessages.BoostXmlLogHandler_wrong_tag_name, tagName));
BoostTestsRunnerPlugin.log(e);
throw e;
return e;
}

@Override
Expand Down

0 comments on commit 754bd7d

Please sign in to comment.