Updates per PR comments

Signed-off-by: Chris Helma <chelma+github@amazon.com>

DocumentsFromSnapshotMigration/build.gradle

@@ -148,10 +148,6 @@ test {
 }
 
 task slowTest(type: Test) {
-    doFirst {
-        println "Running slow tests..."
-    }
-
     useJUnitPlatform()
     dependsOn(':TrafficCapture:dockerSolution:buildDockerImage_elasticsearchTestConsole')
     jacoco {
@@ -163,7 +159,7 @@ task slowTest(type: Test) {
         showExceptions true
         showCauses true
         showStackTraces true
-        // showStandardStreams = true
+        // showStandardStreams true
     }
     reports {
         html.required = true
@@ -174,8 +170,6 @@ task slowTest(type: Test) {
 jacocoTestReport {
     dependsOn slowTest
     reports {
-        xml.required = true
-        xml.destination file("${buildDir}/reports/jacoco/test/jacocoTestReport.xml")
         html.required = true
         html.destination file("${buildDir}/reports/jacoco/test/html")
     }

DocumentsFromSnapshotMigration/src/main/java/com/rfs/RfsMigrateDocuments.java

@@ -56,14 +56,14 @@ public static class Args {
                 description = ("The absolute path to the directory on local disk where the snapshot exists.  Use this parameter"
                     + " if have a copy of the snapshot disk.  Mutually exclusive with --s3-local-dir, --s3-repo-uri, and --s3-region."
                 ))
-        public String snapshotLocalDirPath = null;
+        public String snapshotLocalDir = null;
 
         @Parameter(names = {"--s3-local-dir"},
                 required = false,
                 description = ("The absolute path to the directory on local disk to download S3 files to.  If you supply this, you must"
                     + " also supply --s3-repo-uri and --s3-region.  Mutually exclusive with --snapshot-local-dir."
                 ))
-        public String s3LocalDirPath = null;
+        public String s3LocalDir = null;
 
         @Parameter(names = {"--s3-repo-uri"},
                 required = false,
@@ -82,7 +82,7 @@ public static class Args {
         @Parameter(names = {"--lucene-dir"},
                 required = true,
                 description = "The absolute path to the directory where we'll put the Lucene docs")
-        public String luceneDirPath;
+        public String luceneDir;
 
         @Parameter(names = {"--target-host"},
                 required = true,
@@ -113,9 +113,9 @@ public NoWorkLeftException(String message) {
     }
 
     public static void validateArgs(Args args) {
-        boolean isSnapshotLocalDirProvided = args.snapshotLocalDirPath != null;
-        boolean areAllS3ArgsProvided = args.s3LocalDirPath != null && args.s3RepoUri != null && args.s3Region != null;
-        boolean areAnyS3ArgsProvided = args.s3LocalDirPath != null || args.s3RepoUri != null || args.s3Region != null;
+        boolean isSnapshotLocalDirProvided = args.snapshotLocalDir != null;
+        boolean areAllS3ArgsProvided = args.s3LocalDir != null && args.s3RepoUri != null && args.s3Region != null;
+        boolean areAnyS3ArgsProvided = args.s3LocalDir != null || args.s3RepoUri != null || args.s3Region != null;
 
         if (isSnapshotLocalDirProvided && areAnyS3ArgsProvided) {
             throw new ParameterException("You must provide either --snapshot-local-dir or --s3-local-dir, --s3-repo-uri, and --s3-region, but not both.");
@@ -140,9 +140,12 @@ public static void main(String[] args) throws Exception {
 
         validateArgs(arguments);
 
-        var luceneDirPath = Paths.get(arguments.luceneDirPath);
+        var luceneDirPath = Paths.get(arguments.luceneDir);
+        var snapshotLocalDirPath = arguments.snapshotLocalDir != null ? Paths.get(arguments.snapshotLocalDir) : null;
+
+
         try (var processManager = new LeaseExpireTrigger(workItemId->{
-            log.error("terminating RunRfsWorker because its lease has expired for " + workItemId);
+            log.error("Terminating RunRfsWorker because its lease has expired for " + workItemId);
             System.exit(PROCESS_TIMED_OUT);
         }, Clock.systemUTC())) {
             var workCoordinator = new OpenSearchWorkCoordinator(new ApacheHttpClient(new URI(arguments.targetHost)),
@@ -156,14 +159,14 @@ public static void main(String[] args) throws Exception {
                 DocumentReindexer reindexer = new DocumentReindexer(targetClient);
 
                 SourceRepo sourceRepo;
-                if (arguments.snapshotLocalDirPath == null) {
+                if (snapshotLocalDirPath == null) {
                     sourceRepo = S3Repo.create(
-                        Paths.get(arguments.s3LocalDirPath),
+                        Paths.get(arguments.s3LocalDir),
                         new S3Uri(arguments.s3RepoUri),
                         arguments.s3Region
                     );
                 } else {
-                    sourceRepo = new FileSystemRepo(Paths.get(arguments.snapshotLocalDirPath));
+                    sourceRepo = new FileSystemRepo(snapshotLocalDirPath);
                 }
                 SnapshotRepo.Provider repoDataProvider = new SnapshotRepoProvider_ES_7_10(sourceRepo);
 

DocumentsFromSnapshotMigration/src/test/java/com/rfs/FullTest.java

@@ -47,8 +47,10 @@
 import org.slf4j.event.Level;
 import reactor.core.publisher.Flux;
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -315,19 +317,20 @@ private DocumentsRunner.CompletionStatus migrateDocumentsWithOneWorker(SourceRep
     }
 
     public static Stream<Arguments> makeProcessExitArgs() {
-        var sourceImageArgs = SOURCE_IMAGES.stream().map(name -> makeParamsForBase(name)).collect(Collectors.toList());
-        var targetImageNames = TARGET_IMAGES.stream().map(SearchClusterContainer.Version::getImageName).collect(Collectors.toList());
-
-        return sourceImageArgs.stream()
-            .flatMap(a->
-            targetImageNames.stream().map(b->Arguments.of(a[0], a[1], a[2], b)));
+        return Stream.of(
+            Arguments.of(true, 0),
+            Arguments.of(false, 1)
+        );
     }
 
     @ParameterizedTest
     @MethodSource("makeProcessExitArgs")
-    public void testProcessExitsAsExpected(SearchClusterContainer.Version baseSourceImageVersion,
-            String generatorImage, String[] generatorArgs,
-            String targetImageName) throws Exception {
+    public void testProcessExitsAsExpected(boolean targetAvailable, int expectedExitCode) throws Exception {
+        var sourceImageArgs = makeParamsForBase(SearchClusterContainer.ES_V7_10_2);
+        var baseSourceImageVersion = (SearchClusterContainer.Version) sourceImageArgs[0];
+        var generatorImage = (String) sourceImageArgs[1];
+        var generatorArgs = (String[]) sourceImageArgs[2];
+        var targetImageName = SearchClusterContainer.OS_V2_14_0.getImageName();
 
         try (var esSourceContainer = new PreloadedSearchClusterContainer(baseSourceImageVersion,
                     SOURCE_SERVER_ALIAS, generatorImage, generatorArgs);
@@ -345,20 +348,28 @@ public void testProcessExitsAsExpected(SearchClusterContainer.Version baseSource
             var tempDirSnapshot = Files.createTempDirectory("opensearchMigrationReindexFromSnapshot_test_snapshot");
             var tempDirLucene = Files.createTempDirectory("opensearchMigrationReindexFromSnapshot_test_lucene");
 
+            String targetAddress = osTargetContainer.getHttpHostAddress();
+
             String[] args = {
                 "--snapshot-name", SNAPSHOT_NAME,
                 "--snapshot-local-dir", tempDirSnapshot.toString(),
                 "--lucene-dir", tempDirLucene.toString(),
-                "--target-host", osTargetContainer.getHttpHostAddress()
+                "--target-host", targetAddress
             };
 
             try {
                 esSourceContainer.copySnapshotData(tempDirSnapshot.toString());
 
-                var targetClient = new OpenSearchClient(osTargetContainer.getHttpHostAddress(), null);
+                var targetClient = new OpenSearchClient(targetAddress, null);
                 var sourceRepo = new FileSystemRepo(tempDirSnapshot);
                 migrateMetadata(sourceRepo, targetClient, SNAPSHOT_NAME, INDEX_ALLOWLIST);
 
+                // Stop the target container if we don't want it to be available.  We've already cached the address it was
+                // using, so we can have reasonable confidence that nothing else will be using it and bork our test.
+                if (!targetAvailable) {
+                    osTargetContainer.stop();
+                }
+
                 String classpath = System.getProperty("java.class.path");
                 String javaHome = System.getProperty("java.home");
                 String javaExecutable = javaHome + File.separator + "bin" + File.separator + "java";
@@ -369,27 +380,40 @@ public void testProcessExitsAsExpected(SearchClusterContainer.Version baseSource
                         javaExecutable, "-cp", classpath, "com.rfs.RfsMigrateDocuments"
                 );
                 processBuilder.command().addAll(Arrays.asList(args));
+                processBuilder.redirectErrorStream(true);
 
                 Process process = processBuilder.start();
                 log.atInfo().setMessage("Process started with ID: " + Long.toString(process.toHandle().pid())).log();
 
                 // Kill the process and fail if we have to wait too long
-                boolean finished = process.waitFor(2, TimeUnit.MINUTES);
+                int timeoutSeconds = 90;
+                boolean finished = process.waitFor(timeoutSeconds, TimeUnit.SECONDS);
                 if (!finished) {
+                    // Print the process output
+                    StringBuilder output = new StringBuilder();
+                    try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) {
+                        String line;
+                        while ((line = reader.readLine()) != null) {
+                            output.append(line).append(System.lineSeparator());
+                        }
+                    }
+                    log.atError().setMessage("Process Output:").log();
+                    log.atError().setMessage(output.toString()).log();
+
                     log.atError().setMessage("Process timed out, attempting to kill it...").log();
                     process.destroy(); // Try to be nice about things first...
                     if (!process.waitFor(10, TimeUnit.SECONDS)) {
                         log.atError().setMessage("Process still running, attempting to force kill it...").log();
                         process.destroyForcibly(); // ..then avada kedavra
                     }
-                    Assertions.fail("The process did not finish within the timeout period.");
+                    Assertions.fail("The process did not finish within the timeout period (" + timeoutSeconds + " seconds).");
                 }
 
-                int exitCode = process.exitValue();
+                int actualExitCode = process.exitValue();
+                log.atInfo().setMessage("Process exited with code: " + actualExitCode).log();
 
                 // Check if the exit code is as expected
-                int expectedExitCode = 0;
-                Assertions.assertEquals(expectedExitCode, exitCode, "The program did not exit with the expected status code.");
+                Assertions.assertEquals(expectedExitCode, actualExitCode, "The program did not exit with the expected status code.");
 
             } finally {
                 deleteTree(tempDirSnapshot);

RFS/build.gradle

@@ -115,8 +115,8 @@ test {
     }
 
     testLogging {
-        exceptionFormat = 'full'
-        events "failed"
+        events "passed", "skipped", "failed"
+        exceptionFormat "full"
         showExceptions true
         showCauses true
         showStackTraces true