diff --git a/docs/ai-generated/tasks/PR199-sitemanage-failing-tests/task.md b/docs/ai-generated/tasks/PR199-sitemanage-failing-tests/task.md new file mode 100644 index 0000000000..dc093ee9b6 --- /dev/null +++ b/docs/ai-generated/tasks/PR199-sitemanage-failing-tests/task.md @@ -0,0 +1,40 @@ +# Issue 199: sitemanage — fix 3 failing unit tests + +## Scope + +Two production code changes in `projects/sitemanage` plus one test isolation +fix in `projects/sitemanage/src/test`. No cross-module impact. + +## Files changed + +- `projects/sitemanage/src/main/java/com/percussion/pathmanagement/service/impl/PSDispatchingPathService.java` + - `PathNormalizer.normalizePath`: replace `notNull(path, "Path cannot be null")` with an explicit + `if (path == null) throw new IllegalArgumentException("Path cannot be null");` so the + implementation honours the `IPSPathNormalizer.normalizePath(...) throws IllegalArgumentException` + contract and the `shouldFailOnNormalizeNullPath` test expectation. + - `PathMatch.toFullPath`: same fix — replace `notNull(relativePath, ...)` with an explicit + `IllegalArgumentException` throw to satisfy `shouldFailToReturnProperFullPathIfGivenRelativePathIsNull`. + +- `projects/sitemanage/src/test/java/com/percussion/itemmanagement/service/impl/PSItemServiceTest.java` + - Add `@Rule TemporaryFolder temporaryFolder`. + - In `setUp`: `PathUtils.clearRxDir()` then `PathUtils.setThreadOnlyRxDir(temporaryFolder.getRoot())`. + - In `tearDown`: `PathUtils.unsetThreadOnlyRxDir(temporaryFolder.getRoot())` then + `PathUtils.clearRxDir()`. + - This is the same isolation pattern already used by + `projects/sitemanage/src/test/java/com/percussion/share/extension/PSEnsureMyFacesSecretTest.java` + and is needed because earlier test classes in the surefire JVM set `rxdeploydir` to a + JUnit `TemporaryFolder` root that is deleted after their class finishes, leaving a stale, + nonexistent `rxdeploydir` for any later class that constructs an `Rx`-aware service. + +## Verification + +`./mvn-env.sh test -pl projects/sitemanage` after the change: + +``` +[INFO] Tests run: 314, Failures: 0, Errors: 0, Skipped: 17 +[INFO] BUILD SUCCESS +``` + +The 17 skipped tests are the pre-existing `@Ignore`d tests, unchanged from the prior baseline. + +`./mvn-env.sh spotless:check -pl projects/sitemanage` is clean. diff --git a/projects/sitemanage/src/main/java/com/percussion/pathmanagement/service/impl/PSDispatchingPathService.java b/projects/sitemanage/src/main/java/com/percussion/pathmanagement/service/impl/PSDispatchingPathService.java index d4873016e1..66fa2e0074 100644 --- a/projects/sitemanage/src/main/java/com/percussion/pathmanagement/service/impl/PSDispatchingPathService.java +++ b/projects/sitemanage/src/main/java/com/percussion/pathmanagement/service/impl/PSDispatchingPathService.java @@ -757,7 +757,9 @@ public PathMatch( * @return never null. */ public String toFullPath(String relativePath) { - notNull(relativePath, "relative path cannot be null"); + if (relativePath == null) { + throw new IllegalArgumentException("relative path cannot be null"); + } relativePath = StringUtils.removeStart(relativePath, "/").trim(); return pathPrefix + relativePath; } @@ -1142,7 +1144,9 @@ public static class PathNormalizer implements IPSPathNormalizer { /** {@inheritDoc} */ public String normalizePath(String path) { - notNull(path, "Path cannot be null"); + if (path == null) { + throw new IllegalArgumentException("Path cannot be null"); + } String rvalue = path.trim(); if (!StringUtils.endsWith(rvalue, "/")) { rvalue = rvalue + "/"; diff --git a/projects/sitemanage/src/test/java/com/percussion/itemmanagement/service/impl/PSItemServiceTest.java b/projects/sitemanage/src/test/java/com/percussion/itemmanagement/service/impl/PSItemServiceTest.java index 1a80226f6c..f281a3de42 100644 --- a/projects/sitemanage/src/test/java/com/percussion/itemmanagement/service/impl/PSItemServiceTest.java +++ b/projects/sitemanage/src/test/java/com/percussion/itemmanagement/service/impl/PSItemServiceTest.java @@ -38,6 +38,7 @@ import com.percussion.share.dao.IPSFolderHelper; import com.percussion.share.data.PSItemProperties; import com.percussion.share.service.IPSIdMapper; +import com.percussion.utils.io.PathUtils; import com.percussion.utils.request.PSRequestInfo; import com.percussion.utils.request.PSRequestInfoBase; import com.percussion.webservices.content.IPSContentWs; @@ -49,10 +50,14 @@ import org.jmock.integration.junit4.JUnit4Mockery; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; public class PSItemServiceTest { + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + private Mockery context = new JUnit4Mockery(); private PSItemService service; @@ -75,6 +80,15 @@ public class PSItemServiceTest { @Before public void setUp() throws Exception { + // Point PathUtils.getRxDir() at a fresh temp folder so the PSItemService constructor + // (which reads the Rx install directory for PSEncryptor) does not see a stale or missing + // rxdeploydir left behind by a previous test class running in the same surefire JVM. + // clearRxDir() must run before setThreadOnlyRxDir() so the static cache is dropped first; + // otherwise the prior RX_DIR value would short-circuit getRxDir() and ignore the new + // thread-local value. + PathUtils.clearRxDir(); + PathUtils.setThreadOnlyRxDir(temporaryFolder.getRoot()); + idMapper = context.mock(IPSIdMapper.class); systemService = context.mock(IPSSystemService.class); workflowHelper = context.mock(IPSWorkflowHelper.class); @@ -124,6 +138,8 @@ public void setUp() throws Exception { @After public void tearDown() throws Exception { PSRequestInfoBase.resetRequestInfo(); + PathUtils.unsetThreadOnlyRxDir(temporaryFolder.getRoot()); + PathUtils.clearRxDir(); } @Test