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
The following commit(s) were added to refs/heads/master by this push:
new ebbc764105 Unit tests
ebbc764105 is described below
commit ebbc76410588b9d35bb072e3e26f101d8eae97b8
Author: James Bognar <[email protected]>
AuthorDate: Mon Dec 1 08:31:37 2025 -0800
Unit tests
---
BCT_CONVERTER_OVERRIDE_FEASIBILITY.md | 439 +++++++++++++++++++++
.../apache/juneau/common/utils/StringUtils.java | 37 +-
.../juneau/common/utils/StringUtils_Test.java | 31 +-
3 files changed, 484 insertions(+), 23 deletions(-)
diff --git a/BCT_CONVERTER_OVERRIDE_FEASIBILITY.md
b/BCT_CONVERTER_OVERRIDE_FEASIBILITY.md
new file mode 100644
index 0000000000..3531379792
--- /dev/null
+++ b/BCT_CONVERTER_OVERRIDE_FEASIBILITY.md
@@ -0,0 +1,439 @@
+# Feasibility Analysis: Overriding BctAssertions.DEFAULT_CONVERTER
+
+## Overview
+
+This document analyzes the feasibility of making
`BctAssertions.DEFAULT_CONVERTER` a resettable, memoized thread-local field
that can be overridden during test setup.
+
+## Current State
+
+- `DEFAULT_CONVERTER` is currently a `static final` field:
`BasicBeanConverter.DEFAULT`
+- Used throughout `BctAssertions` via:
`args.getBeanConverter().orElse(DEFAULT_CONVERTER)`
+- Custom converters can be provided per-assertion via
`AssertionArgs.setBeanConverter()`
+- TODO-88 exists to eliminate the need for `AssertionArgs` by making the
default converter resettable
+
+## Proposed Solution
+
+### Core Implementation
+
+```java
+public class BctAssertions {
+ // Thread-local with memoized default
+ private static final ThreadLocal<ResettableSupplier<BeanConverter>>
DEFAULT_CONVERTER =
+ ThreadLocal.withInitial(() -> memoizeResettable(() ->
BasicBeanConverter.DEFAULT));
+
+ // Get the current converter for this thread
+ private static BeanConverter getDefaultConverter() {
+ return DEFAULT_CONVERTER.get().get();
+ }
+
+ // Override the converter for this thread
+ public static void setDefaultConverter(BeanConverter converter) {
+ DEFAULT_CONVERTER.get().reset();
+ // Need to set a new value - see implementation options below
+ }
+
+ // Reset to default for this thread
+ public static void resetDefaultConverter() {
+ DEFAULT_CONVERTER.get().reset();
+ }
+}
+```
+
+## JUnit 5 Parallel Execution Considerations
+
+### How JUnit 5 Parallelization Works
+
+1. **Test Method Parallelization**: When
`junit.jupiter.execution.parallel.mode.default = concurrent`, each test method
can run in its own thread
+2. **Test Class Parallelization**: When
`junit.jupiter.execution.parallel.mode.classes.default = concurrent`, different
test classes run in parallel
+3. **Thread Isolation**: Each test method thread has its own `ThreadLocal`
storage
+
+### Thread-Local Behavior
+
+✅ **Advantages:**
+- Each parallel test method gets its own converter instance
+- No cross-thread interference
+- Thread-safe by design
+
+⚠️ **Challenges:**
+- Tests in the same class running in parallel will have **separate** converter
instances
+- Cannot easily share a converter across all tests in a class when running in
parallel
+
+## Options for Class-Level Converter Sharing
+
+### Option 1: Thread-Local Only (Method-Level Isolation)
+
+**Behavior:**
+- Each test method gets its own converter instance
+- Tests in the same class running in parallel use different converters
+- `@BeforeEach` can set converter per test method
+- `@AfterEach` can reset converter per test method
+
+**Pros:**
+- Simple implementation
+- No cross-test interference
+- Works perfectly with parallel execution
+
+**Cons:**
+- Cannot share converter across all tests in a class when running in parallel
+- Requires setting converter in each test method if you want customization
+
+**Use Case:**
+```java
+@BeforeEach
+void setUp() {
+ var customConverter = BasicBeanConverter.builder()
+ .defaultSettings()
+ .addStringifier(MyType.class, obj -> obj.customFormat())
+ .build();
+ BctAssertions.setDefaultConverter(customConverter);
+}
+
+@AfterEach
+void tearDown() {
+ BctAssertions.resetDefaultConverter();
+}
+```
+
+### Option 2: Class-Level Thread-Local with Synchronization
+
+**Behavior:**
+- Use a `ConcurrentHashMap<Class<?>, BeanConverter>` to store class-level
converters
+- Thread-local checks class-level first, then falls back to thread-local
+- Requires synchronization or atomic operations
+
+**Implementation Sketch:**
+```java
+private static final ConcurrentHashMap<Class<?>, BeanConverter>
CLASS_CONVERTERS = new ConcurrentHashMap<>();
+private static final ThreadLocal<ResettableSupplier<BeanConverter>>
THREAD_CONVERTER =
+ ThreadLocal.withInitial(() -> memoizeResettable(() -> {
+ // Check class-level first
+ Class<?> testClass = findTestClass(); // Need to detect current test
class
+ BeanConverter classConverter = CLASS_CONVERTERS.get(testClass);
+ return classConverter != null ? classConverter :
BasicBeanConverter.DEFAULT;
+ }));
+
+public static void setDefaultConverterForClass(Class<?> testClass,
BeanConverter converter) {
+ CLASS_CONVERTERS.put(testClass, converter);
+ // Invalidate thread-local cache for all threads running this class
+ THREAD_CONVERTER.get().reset();
+}
+```
+
+**Pros:**
+- Can share converter across all tests in a class
+- Still thread-safe
+
+**Cons:**
+- Complex implementation
+- Requires detecting current test class (reflection/stack trace)
+- Thread-local cache invalidation is tricky
+- May not work well with nested test classes
+
+### Option 3: Hybrid Approach with Test Class Detection
+
+**Behavior:**
+- Use `ThreadLocal` with a `ResettableSupplier` that checks for class-level
overrides
+- Detect test class from stack trace or via explicit registration
+- Cache converter per thread, but check class-level map on cache miss
+
+**Implementation Sketch:**
+```java
+private static final ConcurrentHashMap<Class<?>, BeanConverter>
CLASS_CONVERTERS = new ConcurrentHashMap<>();
+private static final ThreadLocal<ResettableSupplier<BeanConverter>>
THREAD_CONVERTER =
+ ThreadLocal.withInitial(() -> memoizeResettable(() -> {
+ Class<?> testClass = findTestClassFromStack();
+ return CLASS_CONVERTERS.getOrDefault(testClass,
BasicBeanConverter.DEFAULT);
+ }));
+
+private static Class<?> findTestClassFromStack() {
+ StackTraceElement[] stack = Thread.currentThread().getStackTrace();
+ for (StackTraceElement element : stack) {
+ try {
+ Class<?> clazz = Class.forName(element.getClassName());
+ if (clazz.getName().endsWith("_Test") ||
clazz.isAnnotationPresent(TestClass.class)) {
+ return clazz;
+ }
+ } catch (ClassNotFoundException e) {
+ // Continue
+ }
+ }
+ return null;
+}
+```
+
+**Pros:**
+- Automatic class detection
+- Supports both class-level and method-level overrides
+- Thread-safe
+
+**Cons:**
+- Stack trace inspection is expensive (but memoized)
+- May incorrectly detect class in some scenarios
+- Complex implementation
+
+### Option 4: Explicit Test Class Registration (Recommended)
+
+**Behavior:**
+- Provide explicit methods for class-level and method-level overrides
+- Use `@BeforeAll` to set class-level converter
+- Use `@BeforeEach` to set method-level converter
+- Thread-local stores the active converter
+
+**Implementation:**
+```java
+public class BctAssertions {
+ // Class-level converters (shared across all threads for a class)
+ private static final ConcurrentHashMap<Class<?>, BeanConverter>
CLASS_CONVERTERS = new ConcurrentHashMap<>();
+
+ // Thread-local converter (method-level override)
+ private static final ThreadLocal<ResettableSupplier<BeanConverter>>
THREAD_CONVERTER =
+ ThreadLocal.withInitial(() -> memoizeResettable(() -> {
+ // Check if current thread has a class-level converter
+ Class<?> testClass = getTestClassForThread();
+ if (testClass != null) {
+ BeanConverter classConverter = CLASS_CONVERTERS.get(testClass);
+ if (classConverter != null) {
+ return classConverter;
+ }
+ }
+ return BasicBeanConverter.DEFAULT;
+ }));
+
+ // Set converter for all tests in a class
+ public static void setDefaultConverterForClass(Class<?> testClass,
BeanConverter converter) {
+ CLASS_CONVERTERS.put(testClass, converter);
+ // Note: Existing threads will continue using their cached value
+ // New threads will pick up the class-level converter
+ }
+
+ // Set converter for current thread (method-level)
+ public static void setDefaultConverter(BeanConverter converter) {
+ // Store in a separate thread-local for method-level override
+ METHOD_CONVERTER.set(converter);
+ THREAD_CONVERTER.get().reset(); // Force recomputation
+ }
+
+ private static BeanConverter getDefaultConverter() {
+ // Check method-level first, then class-level, then default
+ BeanConverter methodConverter = METHOD_CONVERTER.get();
+ if (methodConverter != null) {
+ return methodConverter;
+ }
+ return THREAD_CONVERTER.get().get();
+ }
+}
+```
+
+**Usage:**
+```java
+public class MyTest extends TestBase {
+ @BeforeAll
+ static void setUpClass() {
+ var classConverter = BasicBeanConverter.builder()
+ .defaultSettings()
+ .addStringifier(MyType.class, obj -> obj.customFormat())
+ .build();
+ BctAssertions.setDefaultConverterForClass(MyTest.class,
classConverter);
+ }
+
+ @AfterAll
+ static void tearDownClass() {
+ BctAssertions.clearDefaultConverterForClass(MyTest.class);
+ }
+
+ @BeforeEach
+ void setUp() {
+ // Optional: Override for this specific test method
+ // BctAssertions.setDefaultConverter(customConverter);
+ }
+
+ @AfterEach
+ void tearDown() {
+ // Optional: Reset method-level override
+ // BctAssertions.resetDefaultConverter();
+ }
+}
+```
+
+**Pros:**
+- Clear and explicit API
+- Supports both class-level and method-level overrides
+- Works with parallel execution
+- No stack trace inspection needed
+- Easy to understand and maintain
+
+**Cons:**
+- Requires explicit class registration in `@BeforeAll`
+- Class-level converters persist until explicitly cleared (but that's usually
desired)
+
+## Recommended Implementation
+
+I recommend **Option 4 (Explicit Test Class Registration)** because:
+
+1. **Clarity**: Explicit is better than implicit - developers know exactly
what's happening
+2. **Performance**: No stack trace inspection overhead
+3. **Flexibility**: Supports both class-level and method-level overrides
+4. **Parallel-Safe**: Works correctly with JUnit 5 parallel execution
+5. **Maintainability**: Simple to understand and debug
+
+### Implementation Details
+
+```java
+public class BctAssertions {
+ // Class-level converters (shared across threads for a class)
+ private static final ConcurrentHashMap<Class<?>, BeanConverter>
CLASS_CONVERTERS = new ConcurrentHashMap<>();
+
+ // Method-level converter override (thread-local)
+ private static final ThreadLocal<BeanConverter> METHOD_CONVERTER = new
ThreadLocal<>();
+
+ // Thread-local memoized supplier for class-level or default converter
+ private static final ThreadLocal<ResettableSupplier<BeanConverter>>
THREAD_CONVERTER =
+ ThreadLocal.withInitial(() -> memoizeResettable(() -> {
+ // Find the test class for this thread
+ Class<?> testClass = findTestClassForThread();
+ if (testClass != null) {
+ BeanConverter classConverter = CLASS_CONVERTERS.get(testClass);
+ if (classConverter != null) {
+ return classConverter;
+ }
+ }
+ return BasicBeanConverter.DEFAULT;
+ }));
+
+ // Internal: Get converter (checks method-level first, then
class-level/default)
+ private static BeanConverter getDefaultConverter() {
+ BeanConverter methodConverter = METHOD_CONVERTER.get();
+ if (methodConverter != null) {
+ return methodConverter;
+ }
+ return THREAD_CONVERTER.get().get();
+ }
+
+ // Public API: Set converter for all tests in a class
+ public static void setDefaultConverterForClass(Class<?> testClass,
BeanConverter converter) {
+ assertArgNotNull("testClass", testClass);
+ assertArgNotNull("converter", converter);
+ CLASS_CONVERTERS.put(testClass, converter);
+ // Invalidate thread-local cache for this class's threads
+ // Note: This is best-effort; existing threads may continue with
cached value
+ // until they call getDefaultConverter() again
+ }
+
+ // Public API: Clear converter for a class
+ public static void clearDefaultConverterForClass(Class<?> testClass) {
+ assertArgNotNull("testClass", testClass);
+ CLASS_CONVERTERS.remove(testClass);
+ }
+
+ // Public API: Set converter for current thread (method-level override)
+ public static void setDefaultConverter(BeanConverter converter) {
+ assertArgNotNull("converter", converter);
+ METHOD_CONVERTER.set(converter);
+ }
+
+ // Public API: Reset converter for current thread (clears method-level
override)
+ public static void resetDefaultConverter() {
+ METHOD_CONVERTER.remove();
+ THREAD_CONVERTER.get().reset(); // Also reset class-level cache
+ }
+
+ // Internal: Find test class for current thread (simplified - may need
refinement)
+ private static Class<?> findTestClassForThread() {
+ // This is a simplified version - you may want to cache this per thread
+ // or use a more sophisticated detection mechanism
+ StackTraceElement[] stack = Thread.currentThread().getStackTrace();
+ for (StackTraceElement element : stack) {
+ String className = element.getClassName();
+ if (className.endsWith("_Test") || className.contains("Test")) {
+ try {
+ return Class.forName(className);
+ } catch (ClassNotFoundException e) {
+ // Continue
+ }
+ }
+ }
+ return null;
+ }
+
+ // Update all methods to use getDefaultConverter() instead of
DEFAULT_CONVERTER
+ // Example:
+ public static void assertBean(Object actual, String fields, String
expected) {
+ assertBean(args(), actual, fields, expected);
+ }
+
+ public static void assertBean(AssertionArgs args, Object actual, String
fields, String expected) {
+ // ... existing code ...
+ var converter = args.getBeanConverter().orElse(getDefaultConverter());
+ // ... rest of method ...
+ }
+}
+```
+
+## Alternative: Simpler Thread-Local Only Approach
+
+If class-level sharing is not a requirement, a simpler implementation is:
+
+```java
+public class BctAssertions {
+ private static final ThreadLocal<ResettableSupplier<BeanConverter>>
DEFAULT_CONVERTER =
+ ThreadLocal.withInitial(() -> memoizeResettable(() ->
BasicBeanConverter.DEFAULT));
+
+ private static BeanConverter getDefaultConverter() {
+ return DEFAULT_CONVERTER.get().get();
+ }
+
+ public static void setDefaultConverter(BeanConverter converter) {
+ assertArgNotNull("converter", converter);
+ // Store override in a separate thread-local
+ CONVERTER_OVERRIDE.set(converter);
+ DEFAULT_CONVERTER.get().reset(); // Invalidate cache
+ }
+
+ public static void resetDefaultConverter() {
+ CONVERTER_OVERRIDE.remove();
+ DEFAULT_CONVERTER.get().reset();
+ }
+
+ private static final ThreadLocal<BeanConverter> CONVERTER_OVERRIDE = new
ThreadLocal<>();
+
+ private static BeanConverter getDefaultConverter() {
+ BeanConverter override = CONVERTER_OVERRIDE.get();
+ if (override != null) {
+ return override;
+ }
+ return DEFAULT_CONVERTER.get().get();
+ }
+}
+```
+
+This simpler approach:
+- ✅ Works perfectly with parallel execution
+- ✅ Each test method can set its own converter
+- ✅ No class-level sharing (each test is independent)
+- ✅ Much simpler implementation
+
+## Questions to Answer
+
+1. **Do you need class-level converter sharing?**
+ - If YES → Use Option 4 (Explicit Registration)
+ - If NO → Use simpler thread-local only approach
+
+2. **How should converter be set in test setup?**
+ - `@BeforeAll` for class-level?
+ - `@BeforeEach` for method-level?
+ - Both?
+
+3. **Should converter persist across test methods in a class?**
+ - If tests run sequentially → Yes, can share
+ - If tests run in parallel → Each thread gets its own
+
+## Recommendation
+
+Start with the **simpler thread-local only approach** unless you have a
specific need for class-level sharing. You can always add class-level support
later if needed.
+
+The simpler approach:
+- Solves the core problem (eliminating need for AssertionArgs)
+- Works perfectly with parallel execution
+- Is easy to implement and maintain
+- Can be enhanced later if class-level sharing is needed
+
diff --git
a/juneau-core/juneau-common/src/main/java/org/apache/juneau/common/utils/StringUtils.java
b/juneau-core/juneau-common/src/main/java/org/apache/juneau/common/utils/StringUtils.java
index d487bb4ab7..a0992c6bec 100644
---
a/juneau-core/juneau-common/src/main/java/org/apache/juneau/common/utils/StringUtils.java
+++
b/juneau-core/juneau-common/src/main/java/org/apache/juneau/common/utils/StringUtils.java
@@ -5279,9 +5279,18 @@ public class StringUtils {
public static long parseLongWithSuffix(String s) {
assertArgNotNull("s", s);
var m = multiplierLong(s);
- if (m == 1)
+ if (m == 1) {
+ // If multiplier is 1, try to decode the whole string
+ // This will throw NumberFormatException if it contains
a suffix character
return Long.decode(s);
- return Long.decode(s.substring(0, s.length() - 1).trim()) * m;
// NOSONAR - NPE not possible here.
+ }
+ var baseStr = s.substring(0, s.length() - 1).trim();
+ var base = Long.decode(baseStr); // NOSONAR - NPE not possible
here.
+ try {
+ return Math.multiplyExact(base, m);
+ } catch (ArithmeticException e) {
+ throw new NumberFormatException("Value " + s + "
exceeds Long.MAX_VALUE");
+ }
}
/**
@@ -8150,27 +8159,29 @@ public class StringUtils {
* @return The multiplier value (1 if no valid suffix found).
*/
private static long multiplierLong(String s) {
- var c = isEmpty(s) ? 'z' : s.charAt(s.length() - 1);
+ if (isEmpty(s))
+ return 1;
+ var c = s.charAt(s.length() - 1);
if (c == 'P')
- return 1024 * 1024 * 1024 * 1024 * 1024l;
+ return 1125899906842624L; // 1024^5
if (c == 'T')
- return 1024 * 1024 * 1024 * 1024l;
+ return 1099511627776L; // 1024^4
if (c == 'G')
- return 1024 * 1024 * 1024l;
+ return 1073741824L; // 1024^3
if (c == 'M')
- return 1024 * 1024l;
+ return 1048576L; // 1024^2
if (c == 'K')
- return 1024l;
+ return 1024L;
if (c == 'p')
- return 1000 * 1000 * 1000 * 1000 * 1000l;
+ return 1000000000000000L; // 1000^5
if (c == 't')
- return 1000 * 1000 * 1000 * 1000l;
+ return 1000000000000L; // 1000^4
if (c == 'g')
- return 1000 * 1000 * 1000l;
+ return 1000000000L; // 1000^3
if (c == 'm')
- return 1000 * 1000l;
+ return 1000000L; // 1000^2
if (c == 'k')
- return 1000l;
+ return 1000L;
return 1;
}
diff --git
a/juneau-utest/src/test/java/org/apache/juneau/common/utils/StringUtils_Test.java
b/juneau-utest/src/test/java/org/apache/juneau/common/utils/StringUtils_Test.java
index 53f6236224..f01214b505 100755
---
a/juneau-utest/src/test/java/org/apache/juneau/common/utils/StringUtils_Test.java
+++
b/juneau-utest/src/test/java/org/apache/juneau/common/utils/StringUtils_Test.java
@@ -1510,12 +1510,11 @@ class StringUtils_Test extends TestBase {
assertEquals("Progress: 50%", format("Progress: %d%%", 50));
assertEquals("Discount: 25% off", format("Discount: %d%% off",
25));
- // Line separator - %n is supported by printf-style formatting
- var result = format("Line 1%nLine 2");
- assertTrue(result.contains("Line 1"));
- assertTrue(result.contains("Line 2"));
- // %n may or may not be replaced depending on StringFormat
implementation
- // Just verify the result contains both lines
+ // Line separator - %n is supported by printf-style formatting
and replaced with line separator
+ // Note: format() returns pattern as-is when args.length == 0,
so we need to pass at least one arg
+ // to trigger token processing. However, %n doesn't consume
arguments, so we can pass any arg.
+ var lineSep = System.lineSeparator();
+ assertEquals("Line 1" + lineSep + "Line 2", format("Line
1%nLine 2", "dummy"));
// MessageFormat style
assertEquals("Hello John, you are 30 years old", format("Hello
{0}, you are {1} years old", "John", 30));
@@ -3188,7 +3187,7 @@ class StringUtils_Test extends TestBase {
assertEquals(30, cal4.get(Calendar.MINUTE));
assertEquals(0, cal4.get(Calendar.SECOND));
- // Should throw for invalid dates
+ // Should throw for invalid dates (DateTimeParseException is
thrown by DateUtils, not IllegalArgumentException)
assertThrows(Exception.class, () ->
parseIsoCalendar("invalid"));
assertThrows(Exception.class, () ->
parseIsoCalendar("2023-13-25")); // Invalid month
}
@@ -3205,7 +3204,7 @@ class StringUtils_Test extends TestBase {
var date2 = parseIsoDate("2023-12-25T14:30:00");
assertNotNull(date2);
- // Should throw for invalid dates
+ // Should throw for invalid dates (DateTimeParseException is
thrown by DateUtils, not IllegalArgumentException)
assertThrows(Exception.class, () -> parseIsoDate("invalid"));
}
@@ -3233,14 +3232,26 @@ class StringUtils_Test extends TestBase {
assertEquals(1024L * 1024, parseLongWithSuffix("1M"));
assertEquals(1024L * 1024 * 1024, parseLongWithSuffix("1G"));
assertEquals(1024L * 1024 * 1024 * 1024,
parseLongWithSuffix("1T"));
- // Note: "P" (petabyte) multiplier may overflow, so we skip
testing it
+ // Petabyte multiplier (1024^5 = 1,125,899,906,842,624)
+ // Test small values that fit in long
+ assertEquals(1125899906842624L, parseLongWithSuffix("1P")); //
1024^5
+ assertEquals(2251799813685248L, parseLongWithSuffix("2P")); //
2 * 1024^5
+ // Test overflow - values that would exceed Long.MAX_VALUE
+ // Long.MAX_VALUE / (1024^5) = 8191, so "8192P" and above
should overflow
+ assertThrows(NumberFormatException.class, () ->
parseLongWithSuffix("8192P"));
// Decimal multipliers (1000-based)
assertEquals(1000L, parseLongWithSuffix("1k"));
assertEquals(1000L * 1000, parseLongWithSuffix("1m"));
assertEquals(1000L * 1000 * 1000, parseLongWithSuffix("1g"));
assertEquals(1000L * 1000 * 1000 * 1000,
parseLongWithSuffix("1t"));
- // Note: "1p" (petabyte) multiplier may overflow, so we skip
testing it
+ // Petabyte multiplier (1000^5 = 1,000,000,000,000,000)
+ // Test small values that fit in long
+ assertEquals(1000000000000000L, parseLongWithSuffix("1p")); //
1000^5
+ assertEquals(2000000000000000L, parseLongWithSuffix("2p")); //
2 * 1000^5
+ // Test overflow - values that would exceed Long.MAX_VALUE
+ // Long.MAX_VALUE / (1000^5) = 9223, so "9224p" and above
should overflow
+ assertThrows(NumberFormatException.class, () ->
parseLongWithSuffix("9224p"));
// No suffix
assertEquals(123L, parseLongWithSuffix("123"));
