From b5fe7442b568224badc56e323ba23d57de3f3b3a Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Thu, 22 Aug 2024 12:21:47 -0400 Subject: [PATCH 1/2] Improve recently added log messages by adding context to indicate which build is relevant Also remove redundant class and method names from log messages and make some internal methods package-private instead of public. --- .../java/hudson/tasks/junit/CaseResult.java | 23 ++++-------- .../java/hudson/tasks/junit/SuiteResult.java | 37 +++++++------------ .../java/hudson/tasks/junit/TestResult.java | 22 +++++------ .../hudson/tasks/junit/TestResultAction.java | 6 +-- 4 files changed, 33 insertions(+), 55 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/CaseResult.java b/src/main/java/hudson/tasks/junit/CaseResult.java index 7095da0a..9ecfeeb3 100644 --- a/src/main/java/hudson/tasks/junit/CaseResult.java +++ b/src/main/java/hudson/tasks/junit/CaseResult.java @@ -41,7 +41,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.logging.Level; import java.util.logging.Logger; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -269,7 +268,7 @@ public static float clampDuration(float d) { return Math.min(365.0f * 24 * 60 * 60, Math.max(0.0f, d)); } - public static CaseResult parse(SuiteResult parent, final XMLStreamReader reader, String ver) + static CaseResult parse(SuiteResult parent, final XMLStreamReader reader, String context, String ver) throws XMLStreamException { CaseResult r = new CaseResult(parent, null, null, null); while (reader.hasNext()) { @@ -318,19 +317,17 @@ public static CaseResult parse(SuiteResult parent, final XMLStreamReader reader, break; case "properties": r.properties = new HashMap<>(); - parseProperties(r.properties, reader, ver); + parseProperties(r.properties, reader, context, ver); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("CaseResult.parse encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } return r; } - public static void parseProperties(Map r, final XMLStreamReader reader, String ver) + static void parseProperties(Map r, final XMLStreamReader reader, String context, String ver) throws XMLStreamException { while (reader.hasNext()) { final int event = reader.next(); @@ -341,18 +338,16 @@ public static void parseProperties(Map r, final XMLStreamReader final String elementName = reader.getLocalName(); switch (elementName) { case "entry": - parseProperty(r, reader, ver); + parseProperty(r, reader, context, ver); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("CaseResult.parseProperties encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } } - public static void parseProperty(Map r, final XMLStreamReader reader, String ver) + static void parseProperty(Map r, final XMLStreamReader reader, String context, String ver) throws XMLStreamException { String name = null, value = null; while (reader.hasNext()) { @@ -374,9 +369,7 @@ public static void parseProperty(Map r, final XMLStreamReader re } break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("CaseResult.parseProperty encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } diff --git a/src/main/java/hudson/tasks/junit/SuiteResult.java b/src/main/java/hudson/tasks/junit/SuiteResult.java index 746da3ee..1b41158e 100644 --- a/src/main/java/hudson/tasks/junit/SuiteResult.java +++ b/src/main/java/hudson/tasks/junit/SuiteResult.java @@ -40,7 +40,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -160,7 +159,7 @@ public SuiteResult(SuiteResult src) { this.properties.putAll(src.properties); } - public static SuiteResult parse(final XMLStreamReader reader, String ver) throws XMLStreamException { + static SuiteResult parse(final XMLStreamReader reader, String context, String ver) throws XMLStreamException { SuiteResult r = new SuiteResult("", null, null, null); while (reader.hasNext()) { final int event = reader.next(); @@ -171,7 +170,7 @@ public static SuiteResult parse(final XMLStreamReader reader, String ver) throws final String elementName = reader.getLocalName(); switch (elementName) { case "cases": - parseCases(r, reader, ver); + parseCases(r, reader, context, ver); break; case "file": r.file = reader.getElementText(); @@ -199,10 +198,10 @@ public static SuiteResult parse(final XMLStreamReader reader, String ver) throws r.nodeId = reader.getElementText(); break; case "enclosingBlocks": - parseEnclosingBlocks(r, reader, ver); + parseEnclosingBlocks(r, reader, context, ver); break; case "enclosingBlockNames": - parseEnclosingBlockNames(r, reader, ver); + parseEnclosingBlockNames(r, reader, context, ver); break; case "stdout": r.stdout = reader.getElementText(); @@ -212,19 +211,17 @@ public static SuiteResult parse(final XMLStreamReader reader, String ver) throws break; case "properties": r.properties = new HashMap<>(); - CaseResult.parseProperties(r.properties, reader, ver); + CaseResult.parseProperties(r.properties, reader, context, ver); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("SuiteResult.parse encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } return r; } - public static void parseEnclosingBlocks(SuiteResult r, final XMLStreamReader reader, String ver) + static void parseEnclosingBlocks(SuiteResult r, final XMLStreamReader reader, String context, String ver) throws XMLStreamException { while (reader.hasNext()) { final int event = reader.next(); @@ -238,16 +235,13 @@ public static void parseEnclosingBlocks(SuiteResult r, final XMLStreamReader rea r.enclosingBlocks.add(reader.getElementText()); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest( - "SuiteResult.parseEnclosingBlocks encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } } - public static void parseEnclosingBlockNames(SuiteResult r, final XMLStreamReader reader, String ver) + static void parseEnclosingBlockNames(SuiteResult r, final XMLStreamReader reader, String context, String ver) throws XMLStreamException { while (reader.hasNext()) { final int event = reader.next(); @@ -261,16 +255,13 @@ public static void parseEnclosingBlockNames(SuiteResult r, final XMLStreamReader r.enclosingBlockNames.add(reader.getElementText()); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("SuiteResult.parseEnclosingBlockNames encountered an unknown field: " - + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } } - public static void parseCases(SuiteResult r, final XMLStreamReader reader, String ver) throws XMLStreamException { + static void parseCases(SuiteResult r, final XMLStreamReader reader, String context, String ver) throws XMLStreamException { while (reader.hasNext()) { final int event = reader.next(); if (event == XMLStreamReader.END_ELEMENT && reader.getLocalName().equals("cases")) { @@ -280,12 +271,10 @@ public static void parseCases(SuiteResult r, final XMLStreamReader reader, Strin final String elementName = reader.getLocalName(); switch (elementName) { case "case": - r.cases.add(CaseResult.parse(r, reader, ver)); + r.cases.add(CaseResult.parse(r, reader, context, ver)); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("SuiteResult.parseCases encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 3ab1c7db..79e060c4 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -312,25 +312,25 @@ public TestResult(TestResult src) { } } - public static XMLInputFactory getXmlFactory() { + static XMLInputFactory getXmlFactory() { return xmlFactory; } - public void parse(XmlFile f) throws XMLStreamException, IOException { + void parse(XmlFile f) throws XMLStreamException, IOException { try (Reader r = f.readRaw()) { final XMLStreamReader reader = getXmlFactory().createXMLStreamReader(r); while (reader.hasNext()) { final int event = reader.next(); if (event == XMLStreamReader.START_ELEMENT && reader.getName().getLocalPart().equals("result")) { - parseXmlResult(reader); + parseXmlResult(reader, f.getFile().toString()); } } r.close(); } } - private void parseXmlResult(final XMLStreamReader reader) throws XMLStreamException { + private void parseXmlResult(final XMLStreamReader reader, String context) throws XMLStreamException { String ver = reader.getAttributeValue(null, "plugin"); while (reader.hasNext()) { int event = reader.next(); @@ -341,7 +341,7 @@ private void parseXmlResult(final XMLStreamReader reader) throws XMLStreamExcept final String elementName = reader.getLocalName(); switch (elementName) { case "suites": - parseXmlSuites(reader, ver); + parseXmlSuites(reader, context, ver); break; case "duration": duration = CaseResult.clampDuration(new TimeToFloat(reader.getElementText()).parse()); @@ -366,15 +366,13 @@ private void parseXmlResult(final XMLStreamReader reader) throws XMLStreamExcept startTime = Long.parseLong(reader.getElementText()); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("TestResult.parseXmlResult encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } } - private void parseXmlSuites(final XMLStreamReader reader, String ver) throws XMLStreamException { + private void parseXmlSuites(final XMLStreamReader reader, String context, String ver) throws XMLStreamException { while (reader.hasNext()) { final int event = reader.next(); if (event == XMLStreamReader.END_ELEMENT && reader.getLocalName().equals("suites")) { @@ -384,12 +382,10 @@ private void parseXmlSuites(final XMLStreamReader reader, String ver) throws XML final String elementName = reader.getLocalName(); switch (elementName) { case "suite": - suites.add(SuiteResult.parse(reader, ver)); + suites.add(SuiteResult.parse(reader, context, ver)); break; default: - if (LOGGER.isLoggable(Level.FINEST)) { - LOGGER.finest("TestResult.parseXmlSuites encountered an unknown field: " + elementName); - } + LOGGER.finest(() -> "Unknown field in " + context + ": " + elementName); } } } diff --git a/src/main/java/hudson/tasks/junit/TestResultAction.java b/src/main/java/hudson/tasks/junit/TestResultAction.java index ff87ab02..3c75bb53 100644 --- a/src/main/java/hudson/tasks/junit/TestResultAction.java +++ b/src/main/java/hudson/tasks/junit/TestResultAction.java @@ -51,6 +51,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; import jenkins.tasks.SimpleBuildStep; @@ -69,7 +70,6 @@ @SuppressFBWarnings(value = "UG_SYNC_SET_UNSYNC_GET", justification = "False positive") public class TestResultAction extends AbstractTestResultAction implements StaplerProxy, SimpleBuildStep.LastBuildAction { - private static final Logger LOGGER = Logger.getLogger(TestResultAction.class.getName()); private transient WeakReference result; /** null only if there is a {@link JunitTestResultStorage} */ @@ -179,8 +179,8 @@ public synchronized TestResult getResult() { skipCount = r.getSkipCount(); } long d = System.nanoTime() - started; - if (d > 500000000L && LOGGER.isLoggable(Level.WARNING)) { - LOGGER.warning("TestResultAction.load took " + d / 1000000L + " ms."); + if (d > TimeUnit.MILLISECONDS.toNanos(500)) { + logger.warning(() -> "Took " + TimeUnit.NANOSECONDS.toMillis(d) + " ms to load test results for " + run); } return r; } From 3c7bc62332ccc7cf2dda4328d5d7712e24b1fab1 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Thu, 22 Aug 2024 13:07:06 -0400 Subject: [PATCH 2/2] Spotless --- src/main/java/hudson/tasks/junit/SuiteResult.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/tasks/junit/SuiteResult.java b/src/main/java/hudson/tasks/junit/SuiteResult.java index 1b41158e..dc7029c4 100644 --- a/src/main/java/hudson/tasks/junit/SuiteResult.java +++ b/src/main/java/hudson/tasks/junit/SuiteResult.java @@ -261,7 +261,8 @@ static void parseEnclosingBlockNames(SuiteResult r, final XMLStreamReader reader } } - static void parseCases(SuiteResult r, final XMLStreamReader reader, String context, String ver) throws XMLStreamException { + static void parseCases(SuiteResult r, final XMLStreamReader reader, String context, String ver) + throws XMLStreamException { while (reader.hasNext()) { final int event = reader.next(); if (event == XMLStreamReader.END_ELEMENT && reader.getLocalName().equals("cases")) {