This is an automated email from the ASF dual-hosted git repository. jamesbognar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 7901a7a66d85321b2125bb5d16b951fd929981fb Author: James Bognar <[email protected]> AuthorDate: Thu Jul 23 09:12:42 2026 -0400 TODO-279: triage @Disabled tests; drop LogRecord source-detection feature - Remove the unreliable JUL-style source detection from LogRecord: findSource() stack-walk, the memoized source field, getSourceClassName()/getSourceMethodName() overrides, and the {source}/{class}/{method} placeholder cases in formatted(). 10.0.0 behavior change: formatted() no longer supports those placeholders, and positional %N$s specifiers shift down one index. Deletes the 2 chronically @Disabled tests that exercised it (feature confirmed self-contained, no other consumers). - Convert BsonArrayDebug_Test and RoundTripLargeObjects_Test from @Disabled to @Tag("manual") so they're runnable on demand; add a property-driven ${excludedGroups} (default "manual") Surefire mechanism so -DexcludedGroups= can re-enable them. Co-authored-by: Cursor <[email protected]> --- .../apache/juneau/commons/logging/LogRecord.java | 78 +++---------------- .../commons/logging/LogRecordCapture_Test.java | 2 +- .../juneau/commons/logging/LogRecord_Test.java | 90 +--------------------- .../juneau/marshall/bson/BsonArrayDebug_Test.java | 6 +- juneau-integration-tests/pom.xml | 6 +- .../a/rttests/RoundTripLargeObjects_Test.java | 7 +- pom.xml | 12 +++ 7 files changed, 37 insertions(+), 164 deletions(-) diff --git a/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/logging/LogRecord.java b/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/logging/LogRecord.java index 63559fe860..8e7c1af3eb 100644 --- a/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/logging/LogRecord.java +++ b/juneau-core/juneau-commons/src/main/java/org/apache/juneau/commons/logging/LogRecord.java @@ -16,7 +16,6 @@ */ package org.apache.juneau.commons.logging; -import static org.apache.juneau.commons.function.Suppliers.*; import static org.apache.juneau.commons.utils.CollectionUtils.*; import static org.apache.juneau.commons.utils.Shorts.*; import static org.apache.juneau.commons.utils.StringUtils.*; @@ -75,20 +74,15 @@ public class LogRecord extends java.util.logging.LogRecord { // Key constants for format placeholders private static final String KEY_date = "date"; - private static final String KEY_source = "source"; private static final String KEY_logger = "logger"; private static final String KEY_level = "level"; private static final String KEY_msg = "msg"; private static final String KEY_thrown = "thrown"; private static final String KEY_timestamp = "timestamp"; - private static final String KEY_class = "class"; - private static final String KEY_method = "method"; private static final String KEY_thread = "thread"; private static final String KEY_threadid = "threadid"; private static final String KEY_exception = "exception"; - private transient Supplier<Optional<StackTraceElement>> source = memoize(()->findSource()); - /** * Constructor. * @@ -121,49 +115,6 @@ public class LogRecord extends java.util.logging.LogRecord { return isEmptyArray(p) ? m : f(m, p); } - /** - * Returns the source class name, calculating it lazily from the stack trace if not already set. - * - * @return The source class name, or <jk>null</jk> if not available. - */ - @Override - public String getSourceClassName() { - if (source == null) - source = memoize(()->findSource()); - return source.get().map(x -> x.getClassName()).orElse(null); - } - - /** - * Returns the source method name, calculating it lazily from the stack trace if not already set. - * - * @return The source method name, or <jk>null</jk> if not available. - */ - @Override - public String getSourceMethodName() { - if (source == null) - source = memoize(()->findSource()); - return source.get().map(x -> x.getMethodName()).orElse(null); - } - - @SuppressWarnings({ - "java:S3776" // Cognitive complexity acceptable for stack trace filtering - }) - private static Optional<StackTraceElement> findSource() { - for (var e : new Throwable().getStackTrace()) { - var c = e.getClassName(); - var m = e.getMethodName(); - // Skip internal classes, logging classes, lambda methods/classes, and synthetic methods - if (eq(c, cn(LogRecord.class)) || eq(c, cn(Logger.class)) || eq(c, cn(StringUtils.class)) - || c.startsWith("java.util.logging.") - || (m != null && (m.contains("lambda$") || m.startsWith("access$"))) - || c.contains("$$Lambda$") || (c.contains("/") && c.contains("$Lambda"))) { - continue; - } - return o(e); - } - return oe(); - } - /** * Formats this log record as a string using the specified format pattern. * @@ -179,7 +130,6 @@ public class LogRecord extends java.util.logging.LogRecord { * {@link java.util.Formatter Formatter} with the following arguments: * <ol> * <li><c>date</c> - The date/time as a {@link Date} object - * <li><c>source</c> - The source class name and method name (e.g., "com.example.MyClass myMethod") * <li><c>logger</c> - The logger name * <li><c>level</c> - The log level * <li><c>message</c> - The formatted log message @@ -190,9 +140,6 @@ public class LogRecord extends java.util.logging.LogRecord { * <ul> * <li><js>"{date}"</js> - The date/time formatted using {@link SimpleFormatter SimpleFormatter}'s default date format * <li><js>"{timestamp}"</js> - The date/time formatted as ISO-8601 (yyyy-MM-dd'T'HH:mm:ss.SSSZ) - * <li><js>"{class}"</js> - The source class name - * <li><js>"{method}"</js> - The source method name - * <li><js>"{source}"</js> - The source class name and method name (e.g., "com.example.MyClass myMethod") * <li><js>"{logger}"</js> - The logger name * <li><js>"{level}"</js> - The log level name * <li><js>"{msg}"</js> - The log message (formatted if parameters are present) @@ -209,11 +156,10 @@ public class LogRecord extends java.util.logging.LogRecord { * </p> * <ul> * <li><js>"%1$s"</js> or <js>"%1$tc"</js> - The date/time (argument 1) - * <li><js>"%2$s"</js> - The source (argument 2) - * <li><js>"%3$s"</js> - The logger name (argument 3) - * <li><js>"%4$s"</js> - The log level (argument 4) - * <li><js>"%5$s"</js> - The log message (argument 5) - * <li><js>"%6$s"</js> - The throwable and its backtrace (argument 6) + * <li><js>"%2$s"</js> - The logger name (argument 2) + * <li><js>"%3$s"</js> - The log level (argument 3) + * <li><js>"%4$s"</js> - The log message (argument 4) + * <li><js>"%5$s"</js> - The throwable and its backtrace (argument 5) * </ul> * * <p> @@ -245,7 +191,7 @@ public class LogRecord extends java.util.logging.LogRecord { * <jc>// Result: "[2025-01-22T07:42:45.123-0500 INFO] User John logged in"</jc> * * <jc>// Using Formatter-style specifiers</jc> - * String <jv>formatted2</jv> = <jv>record</jv>.formatted(<js>"%4$s: %5$s [%1$tc]%n"</js>); + * String <jv>formatted2</jv> = <jv>record</jv>.formatted(<js>"%3$s: %4$s [%1$tc]%n"</js>); * <jc>// Result: "INFO: User John logged in [Tue Jan 22 07:42:45 PST 2025]"</jc> * * <jc>// Mixed named placeholders and Formatter specifiers</jc> @@ -263,23 +209,19 @@ public class LogRecord extends java.util.logging.LogRecord { }) public String formatted(String format) { var date = new Date(getMillis()); - Supplier<String> sourceName = () -> getSourceClassName() + ' ' + getSourceMethodName(); Function<String, Object> resolver = key -> switch (key) { case KEY_date -> "%1$s"; - case KEY_source -> sourceName.get(); // Override default behavior since logging class doesn't handle classes outside of java.util.logging. - case KEY_logger -> "%3$s"; - case KEY_level -> "%4$s"; - case KEY_msg -> "%5$s"; - case KEY_thrown -> "%6$s"; + case KEY_logger -> "%2$s"; + case KEY_level -> "%3$s"; + case KEY_msg -> "%4$s"; + case KEY_thrown -> "%5$s"; case KEY_timestamp -> new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(date); - case KEY_class -> getSourceClassName(); - case KEY_method -> getSourceMethodName(); case KEY_thread, KEY_threadid -> s(getThreadID()); case KEY_exception -> o(getThrown()).map(x -> x.getMessage()).orElse(""); default -> ""; }; - return safeOptCatch(()->f(formatNamed(format, resolver), date, sourceName, getLoggerName(), getLevel(), getMessage(), getThrown()), x -> x.getLocalizedMessage()).orElse(null); + return safeOptCatch(()->f(formatNamed(format, resolver), date, getLoggerName(), getLevel(), getMessage(), getThrown()), x -> x.getLocalizedMessage()).orElse(null); } } diff --git a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecordCapture_Test.java b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecordCapture_Test.java index 8666cff19a..2efef2beff 100644 --- a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecordCapture_Test.java +++ b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecordCapture_Test.java @@ -141,7 +141,7 @@ class LogRecordCapture_Test extends TestBase { try (var capture = logger.captureEvents()) { logger.info("Message"); - var formatted = capture.getRecords("%4$s: %5$s"); + var formatted = capture.getRecords("%3$s: %4$s"); assertSize(1, formatted); assertTrue(formatted.get(0).contains("INFO")); assertTrue(formatted.get(0).contains("Message")); diff --git a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecord_Test.java b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecord_Test.java index 976c958664..2d6dde98fc 100644 --- a/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecord_Test.java +++ b/juneau-core/juneau-commons/src/test/java/org/apache/juneau/commons/logging/LogRecord_Test.java @@ -108,58 +108,6 @@ class LogRecord_Test extends TestBase { assertNull(rec.getParameters()); } - //==================================================================================================== - // Source class and method name calculation - //==================================================================================================== - - @Test void c01_getSourceClassName_calculatedLazily() { - // Create LogRecord through Logger to get proper call stack - // findSource() filters out LogRecord, Logger, and lambda methods, so test classes are included - try (var capture = getLogger("c01").captureEvents()) { - var logger = getLogger("c01"); - logger.info("Message"); - - var rec = capture.getRecords().get(0); - var className = rec.getSourceClassName(); - // Should be calculated from stack trace - assertNotNull(className); - assertTrue(className.contains("LogRecord_Test")); // Should be this test class - } - } - - @Test void c02_getSourceMethodName_calculatedLazily() { - // Create LogRecord through Logger to get proper call stack - // findSource() filters out LogRecord, Logger, and lambda methods, so test methods are included - try (var capture = getLogger("c02").captureEvents()) { - var logger = getLogger("c02"); - logger.info("Message"); - - var rec = capture.getRecords().get(0); - var methodName = rec.getSourceMethodName(); - // Should be calculated from stack trace - assertNotNull(methodName); - assertEquals("c02_getSourceMethodName_calculatedLazily", methodName); - } - } - - @Test void c03_source_cachedAfterFirstAccess() { - // Create LogRecord through Logger to get proper call stack - try (var capture = getLogger("c03").captureEvents()) { - var logger = getLogger("c03"); - logger.info("Message"); - - var rec = capture.getRecords().get(0); - var className1 = rec.getSourceClassName(); - var className2 = rec.getSourceClassName(); - var methodName1 = rec.getSourceMethodName(); - var methodName2 = rec.getSourceMethodName(); - - // Should return same values (cached) - assertEquals(className1, className2); - assertEquals(methodName1, methodName2); - } - } - //==================================================================================================== // formatted() method - named placeholders //==================================================================================================== @@ -195,23 +143,6 @@ class LogRecord_Test extends TestBase { ); } - // Stack-walk source detection is unreliable in this JVM/test-runner combination; revisit if source-detection accuracy becomes important. - @Disabled("stack-walk source detection is unreliable in this JVM/test-runner combination") - @Test void d04_formatted_classAndMethod() { - // Create LogRecord through Logger to get proper call stack - // findSource() filters out LogRecord, Logger, and lambda methods, so test classes are included - try (var capture = getLogger("d04").captureEvents()) { - var logger = getLogger("d04"); - logger.info("Message"); - - var rec = capture.getRecords().get(0); - var formatted = rec.formatted("{class}.{method}"); - assertNotNull(formatted); - assertTrue(formatted.contains("LogRecord_Test")); - assertTrue(formatted.contains("d04_formatted_classAndMethod")); - } - } - @Test void d05_formatted_logger() { var rec = new LogRecord("test.logger", Level.INFO, "Message", null, null); @@ -266,25 +197,6 @@ class LogRecord_Test extends TestBase { assertTrue(formatted.contains("RuntimeException")); } - // Stack-walk source detection is unreliable in this JVM/test-runner combination; revisit if source-detection accuracy becomes important. - @Disabled("stack-walk source detection is unreliable in this JVM/test-runner combination") - @Test void d12_formatted_source() { - // Create LogRecord through Logger to get proper call stack - // findSource() now only filters out LogRecord and Logger classes, so test classes are included - try (var capture = getLogger("d12").captureEvents()) { - var logger = getLogger("d12"); - logger.info("Message"); - - var rec = capture.getRecords().get(0); - var formatted = rec.formatted("{source}"); - assertNotNull(formatted); - // The {source} placeholder resolves to %2$s which formats the source supplier - // The source supplier returns "className methodName" - assertTrue(formatted.contains("LogRecord_Test")); - assertTrue(formatted.contains("d12_formatted_source")); - } - } - //==================================================================================================== // formatted() method - Formatter-style specifiers //==================================================================================================== @@ -292,7 +204,7 @@ class LogRecord_Test extends TestBase { @Test void e01_formatted_formatterSpecifiers() { var rec = new LogRecord("test.logger", Level.INFO, "Message", null, null); - var formatted = rec.formatted("%4$s: %5$s"); + var formatted = rec.formatted("%3$s: %4$s"); assertTrue(formatted.contains("INFO")); assertTrue(formatted.contains("Message")); } diff --git a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonArrayDebug_Test.java b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonArrayDebug_Test.java index a96c5e645c..b88750a83f 100644 --- a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonArrayDebug_Test.java +++ b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonArrayDebug_Test.java @@ -21,8 +21,8 @@ import static org.junit.jupiter.api.Assertions.*; import org.junit.jupiter.api.*; -/** Debug test to inspect BSON output for arrays. Disabled to avoid stdout noise during normal test runs. */ -@Disabled("Debug test - prints hex dumps for manual inspection; run manually when needed") +/** Debug test to inspect BSON output for arrays. Tagged manual to avoid stdout noise during normal test runs; run explicitly with -Dgroups=manual. */ +@Tag("manual") class BsonArrayDebug_Test { @Test @@ -35,7 +35,7 @@ class BsonArrayDebug_Test { @Test void dumpJsonListBson() { var s = BsonSerializer.create().keepNullProperties().addBeanTypes().addRootType().build(); - var x = new org.apache.juneau.marshall.collections.JsonList("['abc',123]"); + var x = new org.apache.juneau.marshall.collections.JsonList("[\"abc\",123]"); var bytes = s.write(x); assertTrue(bytes.length > 0); } diff --git a/juneau-integration-tests/pom.xml b/juneau-integration-tests/pom.xml index 63758c684e..4048af59f9 100644 --- a/juneau-integration-tests/pom.xml +++ b/juneau-integration-tests/pom.xml @@ -681,7 +681,9 @@ <execution> <id>default-test</id> <configuration> - <excludedGroups>container,benchmark</excludedGroups> + <!-- ${excludedGroups} (default "manual", see root pom) is merged in here so it stays + overridable via -DexcludedGroups= the same way as every other module. --> + <excludedGroups>container,benchmark,${excludedGroups}</excludedGroups> <reportsDirectory>${project.build.directory}/surefire-reports/core</reportsDirectory> <!-- Process-level parallelism for the in-memory core bucket: each fork is a @@ -701,7 +703,7 @@ </goals> <configuration> <groups>container</groups> - <excludedGroups>benchmark</excludedGroups> + <excludedGroups>benchmark,${excludedGroups}</excludedGroups> <reportsDirectory>${project.build.directory}/surefire-reports/container</reportsDirectory> </configuration> </execution> diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/a/rttests/RoundTripLargeObjects_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/a/rttests/RoundTripLargeObjects_Test.java index 570e959881..98aeb1fa0f 100755 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/a/rttests/RoundTripLargeObjects_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/a/rttests/RoundTripLargeObjects_Test.java @@ -44,8 +44,13 @@ import org.junit.jupiter.params.provider.*; /** * Tests designed to serialize and parse objects to make sure we end up * with the same objects for all serializers and parsers. + * + * <p> + * Round-trips a 20000-element object across every serializer/parser pair in the reactor, 10 times each; + * tagged manual because the runtime cost is too high for the normal build, not because of a correctness issue. + * </p> */ -@Disabled +@Tag("manual") @SuppressWarnings({ "serial" // serialVersionUID not required for test classes. }) diff --git a/pom.xml b/pom.xml index 21de6b65e3..aa6d485eee 100644 --- a/pom.xml +++ b/pom.xml @@ -79,6 +79,9 @@ <jakarta.validation-api.version>3.0.2</jakarta.validation-api.version> <logback.version>1.5.18</logback.version> <log4j.version>2.24.3</log4j.version> + <!-- Default Surefire excludedGroups; overridable on the CLI (e.g. -DexcludedGroups= to run "manual"-tagged + tests) because it's referenced via ${excludedGroups} rather than hardcoded in the plugin config. --> + <excludedGroups>manual</excludedGroups> </properties> <modules> @@ -296,11 +299,20 @@ WARNING/SEVERE still surface. Modules that define their own surefire <systemPropertyVariables> (e.g. java.locale.providers) merge with these; module-level keys win on conflict. See src/test/resources/. + + Also excludes the "manual" JUnit5 tag by default so on-demand/manual-inspection tests + (e.g. debug harnesses, expensive stress tests) are skipped during normal builds; run them + explicitly with -DexcludedGroups= -Dgroups=manual. The value comes from the ${excludedGroups} + property (default "manual") rather than a literal, so it can be overridden on the CLI - + a literal value in this <configuration> would take precedence over any -D override. + Modules that already override <excludedGroups> per execution (e.g. juneau-integration-tests) + merge ${excludedGroups} into their own exclusion lists the same way. --> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> <configuration> + <excludedGroups>${excludedGroups}</excludedGroups> <systemPropertyVariables> <logback.configurationFile>${maven.multiModuleProjectDirectory}/src/test/resources/logback-test.xml</logback.configurationFile> <java.util.logging.config.file>${maven.multiModuleProjectDirectory}/src/test/resources/logging.properties</java.util.logging.config.file>
