gharris1727 commented on code in PR #14195:
URL: https://github.com/apache/kafka/pull/14195#discussion_r1294055068


##########
tools/src/test/java/org/apache/kafka/tools/ConnectPluginPathTest.java:
##########
@@ -192,6 +194,51 @@ public void 
testListMultipleWorkerConfigs(PluginLocationType type) {
                 TestPlugins.TestPlugin.SERVICE_LOADER);
     }
 
+    @ParameterizedTest
+    @EnumSource
+    public void testSyncManifests(PluginLocationType type) {
+        CommandResult res = runCommand(
+                "sync-manifests",
+                "--plugin-location",
+                setupLocation(workspace.resolve("location-a"), type, 
TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER)
+        );
+        assertEquals(0, res.returnCode);
+        assertScanResult(true, TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER, 
res.reflective);
+        assertScanResult(true, TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER, 
res.serviceLoading);
+    }
+
+    @ParameterizedTest
+    @EnumSource
+    public void testSyncManifestsDryRun(PluginLocationType type) {
+        CommandResult res = runCommand(
+                "sync-manifests",
+                "--plugin-location",
+                setupLocation(workspace.resolve("location-a"), type, 
TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER),
+                "--dry-run"
+        );
+        assertEquals(0, res.returnCode);
+        assertScanResult(true, TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER, 
res.reflective);
+        assertScanResult(false, TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER, 
res.serviceLoading);
+    }
+
+    @ParameterizedTest
+    @EnumSource
+    public void testSyncManifestsKeepNotFound(PluginLocationType type) {
+        CommandResult res = runCommand(
+                "sync-manifests",
+                "--plugin-location",
+                setupLocation(workspace.resolve("location-a"), type, 
TestPlugins.TestPlugin.BAD_PACKAGING_STATIC_INITIALIZER_THROWS_REST_EXTENSION),
+                "--plugin-location",
+                setupLocation(workspace.resolve("location-b"), type, 
TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER),
+                "--keep-not-found"
+        );
+        assertEquals(0, res.returnCode);
+        assertScanResult(true, TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER, 
res.reflective);
+        assertScanResult(true, TestPlugins.TestPlugin.NON_MIGRATED_CONVERTER, 
res.serviceLoading);
+        assertScanResult(false, 
TestPlugins.TestPlugin.BAD_PACKAGING_STATIC_INITIALIZER_THROWS_REST_EXTENSION, 
res.reflective);
+        assertScanResult(false, 
TestPlugins.TestPlugin.BAD_PACKAGING_STATIC_INITIALIZER_THROWS_REST_EXTENSION, 
res.serviceLoading);

Review Comment:
   You're right, it was very annoying to read the manifest files for assertions 
due to the parameterization of the test.
   
   I added assertions to each of these `sync-manifests` tests that runs a 
`list` afterwards and asserts on the result, which is how users will commonly 
verify the result of a `sync-manifests` command.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to