fix(sitemanage): resolve 3 failing unit tests (#199) - #200
Merged
Conversation
PSDispatchingPathService$PathNormalizer.normalizePath and PSDispatchingPathService$PathMatch.toFullPath were guarding null inputs with org.apache.commons.lang3.Validate.notNull, which throws NullPointerException. The IPSPathNormalizer.normalizePath contract declares throws IllegalArgumentException, and the unit tests in PSDispatchingPathServicePathParsingTest expect that exception. Replace the Validate.notNull guards with explicit null checks that throw IllegalArgumentException, matching the interface and the tests (shouldFailOnNormalizeNullPath, shouldFailToReturnProperFullPathIfGivenRelativePathIsNull). PSItemServiceTest.setUp was failing with "Cannot set rxDir system property rxdeploydir value ... does not exist" when running the full sitemanage surefire suite. Earlier test classes in the same forked JVM set rxdeploydir to a JUnit TemporaryFolder root; once that class finishes the temp folder is deleted but the system property remains, so the next class to load PathUtils.getRxDir blows up when it tries to construct PSItemService (which calls PathUtils.getRxDir(null) for PSEncryptor). Point PathUtils at a fresh TemporaryFolder via setThreadOnlyRxDir / clearRxDir in setUp and clean it up in tearDown, the same isolation pattern already used by PSEnsureMyFacesSecretTest in the same module. Verified locally: 314 tests run, 0 failures, 0 errors, 17 skipped (skipped are pre-existing @ignore'd tests, unchanged). Closes #199 > Co-Authored by mavis mavis using MiniMax-M3 with agent mavis.
natechadwick
approved these changes
Sep 8, 2026
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
2 participants
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.
Summary
Fixes #199. Resolves three failing unit tests in
projects/sitemanage:PSItemServiceTest.setUp:102—IllegalArgumentException: Cannot set rxDir system property rxdeploydir value ... does not existPSDispatchingPathServicePathParsingTest.shouldFailOnNormalizeNullPath—expected<IllegalArgumentException> but was<NullPointerException>PSDispatchingPathServicePathParsingTest.shouldFailToReturnProperFullPathIfGivenRelativePathIsNull—expected<IllegalArgumentException> but was<NullPointerException>Root cause
PSDispatchingPathService$PathNormalizer.normalizePathandPSDispatchingPathService$PathMatch.toFullPathwere guarding null inputs withorg.apache.commons.lang3.Validate.notNull(...), which throwsNullPointerException. TheIPSPathNormalizer.normalizePathinterface explicitly declaresthrows IllegalArgumentException, and the tests expect that exception.PSItemService's constructor callsPathUtils.getRxDir(null), which reads therxdeploydirsystem property. Earlier test classes in the surefire JVM setrxdeploydirto a JUnitTemporaryFolderroot; once that test class finishes, the temp folder is deleted but the property remains, so the next class to loadPathUtils.getRxDirblows up.PSItemServiceTestitself never established a validrxdeploydir.Changes
PSDispatchingPathService: replacenotNull(...)guards inPathNormalizer.normalizePathandPathMatch.toFullPathwith explicitif (... == null) throw new IllegalArgumentException(...)checks so the contract matches theIPSPathNormalizerinterface and the test expectations.PSItemServiceTest: add a@Rule TemporaryFolderand, insetUp/tearDown, usePathUtils.setThreadOnlyRxDir/unsetThreadOnlyRxDirplusPathUtils.clearRxDir()(the same pattern already used byPSEnsureMyFacesSecretTestin the same module) to isolate the test fromrxdeploydirstate left by other test classes.Verification
The 17 skipped tests are the pre-existing
@Ignored tests, unchanged from the baseline../mvn-env.sh spotless:check -pl projects/sitemanageis clean.