madrob commented on a change in pull request #736:
URL: https://github.com/apache/solr/pull/736#discussion_r825127349



##########
File path: solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
##########
@@ -69,214 +68,168 @@
   private Phaser phaser;
 
   @Before
-  public void setup() {
+  public void setup() throws Exception {
     System.setProperty("enable.packages", "true");
     phaser = new Phaser();
+
+    int nodes = TEST_NIGHTLY ? 4 : 2;
+    cluster = configureCluster(nodes).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
+    
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry().setPhaser(phaser);
   }
 
   @After
-  public void teardown() {
+  public void teardown() throws Exception {
+    shutdownCluster();
     System.clearProperty("enable.packages");
   }
 
   @Test
   public void testApi() throws Exception {
-    MiniSolrCloudCluster cluster =
-        configureCluster(4).withJettyConfig(jetty -> 
jetty.enableV2(true)).configure();
-    ContainerPluginsRegistry pluginsRegistry =
-        
cluster.getOpenOverseer().getCoreContainer().getContainerPluginsRegistry();
-    pluginsRegistry.setPhaser(phaser);
-
     int version = phaser.getPhase();
 
-    String errPath = "/error/details[0]/errorMessages[0]";
-    try {
-      PluginMeta plugin = new PluginMeta();
+    PluginMeta plugin = new PluginMeta();
+    V2Request addPlugin =
+        new V2Request.Builder("/cluster/plugin")
+            .forceV2(true)
+            .POST()
+            .withPayload(singletonMap("add", plugin))
+            .build();
+
+    // test with an invalid class
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("TestContainerPlugin$C2")) {
       plugin.name = "testplugin";
       plugin.klass = C2.class.getName();
-      // test with an invalid class
-      V2Request req =
-          new V2Request.Builder("/cluster/plugin")
-              .forceV2(true)
-              .POST()
-              .withPayload(singletonMap("add", plugin))
-              .build();
-      expectError(req, cluster.getSolrClient(), errPath, "No method with 
@Command in class");
-
-      // test with a valid class. This should succeed now
-      plugin.klass = C3.class.getName();
-      req.process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      V2Request readPluginState =
-          new V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      V2Response rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", 
null));
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/plugin/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/testkey", "testval"));
-
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : testplugin}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // verify it is removed
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(null, rsp._get("/plugin/testplugin/class", null));
+      expectError(addPlugin, "No method with @Command in class");
+      assertEquals(1, errors.getCount());
+    }
+
+    // test with a valid class. This should succeed now
+    plugin.klass = C3.class.getName();
+    addPlugin.process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
 
+    // just check if the plugin is indeed registered
+    Callable<V2Response> readPluginState = getPlugin("/cluster/plugin");
+    V2Response rsp = readPluginState.call();
+    assertEquals(C3.class.getName(), rsp._getStr("/plugin/testplugin/class", 
null));
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/plugin/my/plugin"), Map.of("/testkey", "testval"));
+
+    // now remove the plugin
+    new V2Request.Builder("/cluster/plugin")
+        .POST()
+        .forceV2(true)
+        .withPayload("{remove : testplugin}")
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // verify it is removed
+    rsp = readPluginState.call();
+    assertNull(rsp._get("/plugin/testplugin/class", null));
+
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("TestContainerPlugin$C4")) {
       // test with a class  @EndPoint methods. This also uses a template in 
the path name
       plugin.klass = C4.class.getName();
       plugin.name = "collections";
       plugin.pathPrefix = "collections";
-      expectError(
-          req, cluster.getSolrClient(), errPath, "path must not have a prefix: 
collections");
-
-      plugin.name = "my-random-name";
-      plugin.pathPrefix = "my-random-prefix";
-
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // let's test the plugin
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-name/my/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m1"));
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of("/method.name", "m2"));
-      // now remove the plugin
-      new V2Request.Builder("/cluster/plugin")
-          .POST()
-          .forceV2(true)
-          .withPayload("{remove : my-random-name}")
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-      expectFail(
-          () ->
-              new V2Request.Builder("/my-random-prefix/their/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()));
-
-      // test ClusterSingleton plugin
-      plugin.name = "clusterSingleton";
-      plugin.klass = C6.class.getName();
-      req.process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      // just check if the plugin is indeed registered
-      readPluginState = new 
V2Request.Builder("/cluster/plugin").forceV2(true).GET().build();
-      rsp = readPluginState.process(cluster.getSolrClient());
-      assertEquals(C6.class.getName(), 
rsp._getStr("/plugin/clusterSingleton/class", null));
-
-      assertTrue("ccProvided", C6.ccProvided);
-      assertTrue("startCalled", C6.startCalled);
-      assertFalse("stopCalled", C6.stopCalled);
-
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC1()));
-      assertEquals(CConfig.class, ContainerPluginsRegistry.getConfigClass(new 
CC2()));
-
-      CConfig cfg = new CConfig();
-      cfg.boolVal = Boolean.TRUE;
-      cfg.strVal = "Something";
-      cfg.longVal = 1234L;
-      PluginMeta p = new PluginMeta();
-      p.name = "hello";
-      p.klass = CC.class.getName();
-      p.config = cfg;
-
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("add", p))
-          .build()
-          .process(cluster.getSolrClient());
-
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", "Something", 
"/config/longVal", "1234"));
-
-      cfg.strVal = "Something else";
-      new V2Request.Builder("/cluster/plugin")
-          .forceV2(true)
-          .POST()
-          .withPayload(singletonMap("update", p))
-          .build()
-          .process(cluster.getSolrClient());
-      version = phaser.awaitAdvanceInterruptibly(version, 10, 
TimeUnit.SECONDS);
-
-      TestDistribPackageStore.assertResponseValues(
-          10,
-          () ->
-              new V2Request.Builder("hello/plugin")
-                  .forceV2(true)
-                  .GET()
-                  .build()
-                  .process(cluster.getSolrClient()),
-          ImmutableMap.of(
-              "/config/boolVal", "true", "/config/strVal", cfg.strVal, 
"/config/longVal", "1234"));
-
-      // kill the Overseer leader
-      for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
-        if 
(!jetty.getCoreContainer().getZkController().getOverseer().isClosed()) {
-          cluster.stopJettySolrRunner(jetty);
-          cluster.waitForJettyToStop(jetty);
-        }
+      expectError(addPlugin, "path must not have a prefix: collections");
+      assertEquals(1, errors.getCount());
+    }
+
+    plugin.name = "my-random-name";
+    plugin.pathPrefix = "my-random-prefix";
+
+    addPlugin.process(cluster.getSolrClient());
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    // let's test the plugin
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-name/my/plugin"), Map.of("/method.name", "m1"));
+
+    TestDistribPackageStore.assertResponseValues(
+        getPlugin("/my-random-prefix/their/plugin"), Map.of("/method.name", 
"m2"));
+    // now remove the plugin
+    new V2Request.Builder("/cluster/plugin")
+        .POST()
+        .forceV2(true)
+        .withPayload("{remove : my-random-name}")
+        .build()
+        .process(cluster.getSolrClient());
+
+    version = phaser.awaitAdvanceInterruptibly(version, 10, TimeUnit.SECONDS);
+
+    try (ErrorLogMuter errors = 
ErrorLogMuter.substring("/my-random-prefix/their/plugin")) {
+      expectFail(() -> getPlugin("/my-random-prefix/their/plugin").call());
+      assertEquals(2, errors.getCount());
+    }

Review comment:
       I think the two calls before was just a copy-paste bug.
   
   The "two errors" now is because V2Http will log at 146 then log-and-throw at 
171. I added some more detail to the test there.




-- 
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: issues-unsubscr...@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to