Merged
Conversation
Adds a new unit test in ThrowableConsumerTest that calls execute(String, ThrowableConsumer, BiConsumer) with a no-op exception handler to verify the exception-handling path does not propagate an error. Improves coverage for the execute-with-handler scenario.
Codecov Report❌ Patch coverage is
... and 14 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Introduce an abstract test utility LoggingTest that manages Logback logger levels for tests. The class (microsphere-java-core/src/test/java/io/microsphere/LoggingTest.java) uses JUnit 5 annotations to run parameterized (INFO, TRACE) scenarios, with @BeforeEach/@AfterEach methods to set and restore the logger level via a shared LoggerContext. The test is currently @disabled and provides getTargetLogger() to target the package logger.
Import LoggingTest and have ShutdownHookCallbacksThreadTest extend it so the test can leverage shared logging utilities/capture. Also remove an extra trailing blank line.
Update ClassPathResourceConfigurationPropertyLoaderTest to extend LoggingTest and add the corresponding import. This enables logging support/capture for the test class (facilitating inspection of logs during test execution).
Make ArtifactDetectorTest extend LoggingTest to enable logging support in the test. Added import for io.microsphere.LoggingTest and updated the class declaration so tests can leverage the shared logging test utilities.
Wrap the logger.trace call in JarUtils with a logger.isTraceEnabled() check to avoid unnecessary message formatting and exception handling overhead when trace logging is disabled. This change improves performance on the error path when opening a JarFile fails.
Import LoggingTest and have JarUtilsTest extend it so test logging utilities are available. This enables shared logging setup/behavior for the JarUtils tests to improve diagnostics during test runs.
Wrap two logger.trace calls in ClassUtils.java with logger.isTraceEnabled() checks (in class path scanning and in the SimpleJarEntryScanner exception handler). This avoids unnecessary logging overhead/formatting when TRACE is disabled and preserves existing behavior when enabled.
Add import for io.microsphere.LoggingTest and change ClassUtilsTest to extend LoggingTest so the test can leverage shared logging/test utilities. No other test logic was modified.
Wrap trace-level logging calls in FieldUtils with logger.isTraceEnabled() checks to avoid unnecessary string construction when trace logging is disabled. Changes apply to findField and the exception handlers handleIllegalAccessException and handleIllegalArgumentException. No functional change to exception behavior; this is an optimization to reduce logging overhead.
Add io.microsphere.LoggingTest import and update FieldUtilsTest to extend LoggingTest so the test class inherits logging/diagnostic behavior. No other logic changes were made.
Add a logger.isTraceEnabled() check before invoking logger.trace in ManifestArtifactResourceResolver. This avoids unnecessary string formatting and potential expensive artifact representation creation when trace logging is disabled, reducing runtime overhead without changing behavior.
Wrap existing logger.trace(...) calls with logger.isTraceEnabled() checks to avoid unnecessary string formatting when trace logging is disabled. Applied to three locations in StreamArtifactResourceResolver where missing archive, missing JarEntry, or missing artifact metadata file are logged. No behavioral changes beyond logging performance improvement.
Import LoggingTest and make AbstractArtifactResourceResolverTest extend it so the artifact resolver tests can use shared logging utilities and capture test logs consistently.
Replace Loggable abstract class with a Loggable interface that provides default logging methods (logger is now obtained per-call). Update test classes to implement Loggable instead of extending it, adjust class/interface declaration orders where necessary, and add a LoggingTest base class to ExecutorUtilsTest (with import). Also update ModifierTest to avoid referencing the removed protected logger field by checking Proxy.h instead.
Wrap the trace log call in ExecutorUtils.shutdown(...) with a logger.isTraceEnabled() check to avoid unnecessary logging overhead when trace level is disabled. This prevents potential cost from message formatting or parameter evaluation in high-frequency shutdown paths.
Wrap the trace log call in IOUtils with logger.isTraceEnabled() to avoid unnecessary message formatting and argument evaluation when trace level is disabled. This reduces overhead in the stream copy path while preserving the original logging behavior.
Import io.microsphere.LoggingTest and update IOUtilsTest to extend LoggingTest so the test class can reuse shared logging/test utilities (captures logging or common setup/teardown).
Wrap trace logging calls with logger.isTraceEnabled() checks to avoid unnecessary message formatting and potential expensive operations when trace level is disabled. Applied to the file-dispatch log in the event dispatch path and the directory registration log in registerDirectoriesToWatchService.
Import io.microsphere.LoggingTest and update StandardFileWatchServiceTest to extend LoggingTest (while retaining Loggable). This enables reuse of logging test utilities in the test class.
Wrap the logger.trace call in StandardURLStreamHandlerFactory#createURLStreamHandlerFromDefaultFactory with a logger.isTraceEnabled() check to avoid unnecessary logging overhead and follow conditional logging best practices.
Import LoggingTest and have StandardURLStreamHandlerFactoryTest extend it so the test inherits shared logging setup/fixtures. This enables consistent logging behavior for the test class.
Wrap the logger.trace call in URLUtils.java with an if (logger.isTraceEnabled()) check to avoid unnecessary logging message formatting when trace is disabled. Preserves existing behavior (returns null for whitespace in protocol) while reducing runtime overhead during protocol validation.
Add io.microsphere.LoggingTest import and have URLUtilsTest extend LoggingTest so the test can use shared logging/test utilities (enabling log capture/configuration during test execution).
Wrap the trace log call in ClassicProcessIdResolver#current() with logger.isTraceEnabled() to avoid unnecessary message construction when trace logging is disabled. Preserves existing behavior when trace is enabled while reducing potential performance overhead.
Update the test class to extend io.microsphere.LoggingTest and add the required import so the test can use shared logging utilities/capture. No behavior changes to test logic.
Wrap the logger.trace call in ModernProcessIdResolver.current() with logger.isTraceEnabled() to avoid unnecessary logging overhead (message formatting) when TRACE is disabled. This reduces runtime cost for the PID resolution path.
Import io.microsphere.LoggingTest and make ModernProcessIdResolverTest extend LoggingTest. This enables shared logging setup/utility for the test class so test logging can be captured or configured consistently.
Wrap the logger.trace(...) call in ProcessExecutor with a logger.isTraceEnabled() check to avoid unnecessary message formatting when trace logging is disabled. The change sits in the finally block after removing unfinished processes and is a small performance optimization.
Import LoggingTest and have ConstructorUtilsTest extend it so the test inherits the shared logging setup/behavior. Adds the io.microsphere.LoggingTest import and changes the test class declaration; no other test logic was modified.
Introduce a new JUnit 5 test class io.microsphere.io.SerializersTest to verify Serializers behavior: get() returns empty lists before loadSPI and non-null results after loadSPI; getMostCompatible(), getHighestPriority(), and getLowestPriority() are also tested for String.class and Object.class. Minor non-functional change in Serializers.java (trailing newline adjustment).
Add JUnit 5 tests for io.microsphere.io.Deserializers (microsphere-java-core/src/test/java/io/microsphere/io/DeserializersTest.java). Tests verify SPI loading and retrieval methods: get, getMostCompatible, getHighestPriority, and getLowestPriority. They assert empty results before loadSPI() and non-null results after loadSPI(), ensuring SPI-based deserializer discovery and priority selection work as expected.
Remove redundant null-check around URLConnection and always disable caches/get input stream when a URL is found. Consolidate reader creation and resource cleanup into a single try-finally to simplify code and centralize stream/reader closing.
Consolidate classpath resolution logic by introducing resolveClassPaths(boolean, Supplier<String>) and delegating initBootstrapClassPaths/initClassPaths to it. This removes duplication around checking whether a classpath is supported and fetching it via RuntimeMXBean. Also adds unit test coverage (testResolveClassPaths) to assert emptySet behavior for unsupported or empty classpath suppliers and updates imports accordingly.
Refactor ArchiveFileArtifactResourceResolver by extracting the filename-to-artifact parsing into a new static createArtifact(String, URL) method. resolve(...) now delegates to createArtifact(...). Added unit test testCreateArtifact to validate parsing of names like 'a-1.0.jar', 'a.jar', and empty input (null). This improves reuse and testability of artifact creation logic.
Refactor FastByteArrayOutputStream to extract the array-size check into a package-private assertCapacity(int) method and call it from ensureCapacity. Also relax MAX_ARRAY_SIZE visibility to package-private. Update unit tests to use explicit this. and add a test asserting assertCapacity(Integer.MAX_VALUE) throws OutOfMemoryError to increase coverage of the overflow check.
Changed CompositeSubProtocolURLConnectionFactory.selectFactory and selectFactoryIndex from private to package-private to allow test access. Updated CompositeSubProtocolURLConnectionFactoryTest to use a @beforeeach setup, use NO_PROXY and ofList helpers, and added new tests (testRemove, testSupports). Removed inline test factory classes from ExtendableProtocolURLStreamHandlerTest and added them as separate test files (FalseSubProtocolURLConnectionFactory, NullSubProtocolURLConnectionFactory). Adjusted imports accordingly to improve testability and reuse of test helpers.
Replace strict getClass() check with instanceof in ValueHolder.equals so ValueHolder subclasses can be considered equal; add braces/formatting for clarity. Update ValueHolderTest to reflect the behavior (add static import for Integer.valueOf, reorder assertions, and add an assertion ensuring a ValueHolder is not equal to a raw Integer).
Refactor MethodUtils and its tests: adjust the trace log formatting, change MethodKey constructor to varargs (Class<?>...), and fix equals to use instanceof with clearer field comparisons. Simplify hashCode to use existing hash(...) helpers and normalize toString usage. Update MethodUtilsTest to extend LoggingTest, import MethodKey, add null/inequality checks and a new test for nearest overridden method on Object, and add assertions validating MethodKey equality/inequality and behavior.
Rework resource discovery and error handling when scanning a package: remove the surrounding try/catch and use ThrowableSupplier.execute to wrap checked IO operations (getResources and new File(...).toURI().toURL()). Use ListUtils.newArrayList for repository results and simplify control flow by separating resourceURL fallback (ClassDataRepository) from class name resolution and class loading loops. Clean up unused imports (IOException, ArrayList) and keep behavior of collecting and loading classes intact.
Remove the unused 'accessible' flag and simplify exception handling in MethodUtils.invoke (call trySetAccessible without tracking and merge IllegalAccessException and IllegalArgumentException into a single IllegalArgumentException with an argument-mismatch message). Update tests: import isCallerSensitiveMethod, ensure Object.class methods are excluded in the predicate test, and add a test that verifies no methods on MethodUtilsTest are treated as caller-sensitive.
Import io.microsphere.LoggingTest and have MethodHandlesLookupUtilsTest extend LoggingTest to enable logging utilities/support during test execution.
Simplify CustomizedThreadFactory by replacing the static import of System.getSecurityManager with Thread.currentThread, and remove the SecurityManager conditional. The thread group is now always obtained from the current thread (currentThread().getThreadGroup()), simplifying the logic for constructing the thread factory.
Change resolveArguments visibility from private to package-private so it can be unit tested, and add ExceptionUtilsTest#testResolveArguments to verify behavior for exceptions with null message, with a message, and with an explicit cause. Also add required imports (ArrayUtils.ofArray and Assertions.assertArrayEquals) used by the new test.
Reduce log severity when a MethodHandle can't be found in MethodHandlesLookupUtils by switching from WARN to TRACE and updating the guard from isWarnEnabled() to isTraceEnabled(). This lowers noise for expected/missing lookups while preserving the detailed message for trace-level debugging.
Modify tests in MethodUtilsTest to use MethodKey instances for String.valueOf(boolean) and String.valueOf(char) instead of toString. Add an assertion to ensure the two valueOf overload keys are not equal, and change the final inequality check to compare against 'this' (a non-MethodKey) rather than the literal "String". These changes improve coverage of MethodKey comparisons by name and parameter types.
Add an assertion in JSONObjectTest to verify numberToString(Double.valueOf(1.23)) returns '1.23'. This ensures correct formatting of positive non-integer doubles and guards against regressions in numeric string conversion.
Introduce a static getDefaultClassLoader(ClassLoader) helper to centralize null->system ClassLoader resolution and update callers to use it. Add unit test testGetDefaultClassLoaderWithNull to verify behavior, add an extra null assertion in findLoadedClass-related tests, and fix a minor test formatting/brace placement.
Introduce a package-private overload shutdownHookThreadsMap(Class<?>) and make the existing private shutdownHookThreadsMap() delegate to it. This enables tests to invoke the helper directly (including passing null) to verify behavior. Updated tests to import shutdownHookThreadsMap and emptyMap, assert that addShutdownHookCallback(null) returns false, and add testShutdownHookThreadsMapWithNullClass to confirm it returns the same empty map when given null.
Introduce Logger and safer service loading in AbstractEventDispatcher: use getDefaultClassLoader and execute(...) to swallow errors with trace logging, add logger field, replace direct null-check with assertNotNull (documented as IllegalArgumentException), refactor doInListener by adding an overload that accepts an event type and delegating to it, and update imports accordingly. Update unit tests to match behavior changes: add setup, assert IllegalArgumentException for null executor, add tests for loading listeners, classloader-failure resilience, getExecutor, and doInListener(null) handling.
Wrap definition.isPresent() with Boolean.valueOf in AbstractReflectiveDefinitionTest and add a static import for java.lang.Boolean.valueOf. This makes the boxing explicit when passing the boolean result to assertNotNull, clarifying intent in the test.
Refactor testOpenConnectionOnResourceNotFound: add MalformedURLException to the test signature, move URL construction out of the lambda, and replace the lambda with a method reference (assertThrows(IOException.class, url::openStream)). Also add the MalformedURLException import to compile and improve readability.
Replace assertEquals/assertNotEquals usages with explicit assertTrue(...equals(...)) and assertFalse(...equals(...)) in MethodUtilsTest. This makes the intent explicit that the equals(Object) method is being tested (avoiding any ambiguity from generic assertion overloads) — two assertions updated.
Wrap direct execute calls in assertDoesNotThrow and add the import to explicitly assert that no exceptions are thrown during the tests. Updated several test methods (testExecute1, testExecute2, testExecute3, testExecute3OnHandleException) to use the assertion wrapper.
Add @OverRide to the testRemove() method in ReversedDequeTest to explicitly mark it as overriding the superclass implementation, enabling compiler checks and improving code clarity. No functional change to test behavior.
Add the @OverRide annotation to the testEquals method in ReflectiveDefinitionTest to explicitly mark that it overrides the superclass implementation. This clarifies intent and enables compile-time override checking; no functional behavior change.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and refactorings across the codebase, focusing on enhanced logging, utility methods, and code simplification. The most significant changes include the addition of new utility classes and methods, improved logging for better traceability, and refactoring for consistency and maintainability.
Utility enhancements
Iteratorsfor comparing twoIteratorinstances for equality, and refactoredReversedDeque.equalsto use this utility for cleaner code. [1] [2]addIfAbsentmethod inListUtilsto add an element only if it does not already exist in the list.Logging improvements
StreamArtifactResourceResolver,ManifestArtifactResourceResolver, andExecutorUtils) to provide more detailed diagnostics when resources or metadata cannot be resolved or when executor services are shut down. [1] [2] [3] [4] [5]Refactoring and code consistency
DelegatingBlockingQueueto implementDelegatingWrapper, consistently usethis.delegate, and unwrap delegates if possible for cleaner delegation and wrapper handling. [1] [2] [3]CustomizedThreadFactoryby removing the use ofSecurityManagerand always using the current thread's group. [1] [2]AbstractEventDispatcherfor improved error handling and logging, including usingassertNotNullfor constructor validation, adding a logger instance, and simplifying listener loading with better exception handling. [1] [2] [3] [4] [5] [6]Minor improvements
DelegatingDeque.These changes collectively improve code clarity, maintainability, and observability throughout the project.This pull request adds an additional test case to improve coverage of exception handling in the
ThrowableConsumerTestclass.Testing improvements:
testExecute3OnHandleExceptiontest method toThrowableConsumerTest.javato verify the behavior of theexecutemethod when handling exceptions.