[GitHub] [accumulo] ctubbsii commented on a change in pull request #2098: Fix #1628 add test where selector throws an error

[GitBox <gi...@apache.org>]

ctubbsii commented on a change in pull request #2098:
URL: https://github.com/apache/accumulo/pull/2098#discussion_r631297766



##########
File path: test/src/main/java/org/apache/accumulo/test/functional/CompactionIT.java
##########
@@ -68,6 +115,50 @@ protected int defaultTimeoutSeconds() {
     return 4 * 60;
   }
 
+  @Test
+  public void testBadSelector() throws Exception {
+    try (AccumuloClient c = Accumulo.newClient().from(getClientProps()).build()) {
+      final String tableName = getUniqueNames(1)[0];
+      NewTableConfiguration tc = new NewTableConfiguration();
+      // Ensure compactions don't kick off
+      tc.setProperties(Map.of(Property.TABLE_MAJC_RATIO.getKey(), "10.0"));
+      c.tableOperations().create(tableName, tc);
+      // Create multiple RFiles
+      try (BatchWriter bw = c.createBatchWriter(tableName)) {
+        for (int i = 1; i <= 4; i++) {
+          Mutation m = new Mutation(Integer.toString(i));
+          m.put("cf", "cq", new Value());
+          bw.addMutation(m);
+          bw.flush();
+          c.tableOperations().flush(tableName, null, null, true);
+        }
+      }
+
+      List<String> files = FunctionalTestUtils.getRFilePaths(c, tableName);
+      assertEquals(4, files.size());
+
+      String subset = files.get(0).substring(files.get(0).lastIndexOf('/') + 1) + ","
+          + files.get(3).substring(files.get(3).lastIndexOf('/') + 1);
+
+      CompactionConfig config = new CompactionConfig()
+          .setSelector(new PluginConfig(RandomErrorThrowingSelector.class.getName(),
+              Map.of(RandomErrorThrowingSelector.FILE_LIST_PARAM, subset)))
+          .setWait(true);
+      c.tableOperations().compact(tableName, config);
+
+      // check that the subset of files selected are compacted, but the others remain untouched
+      List<String> filesAfterCompact = FunctionalTestUtils.getRFilePaths(c, tableName);
+      assertFalse(filesAfterCompact.contains(files.get(0)));
+      assertTrue(filesAfterCompact.contains(files.get(1)));
+      assertTrue(filesAfterCompact.contains(files.get(2)));
+      assertFalse(filesAfterCompact.contains(files.get(3)));
+
+      List<String> rows = new ArrayList<>();
+      c.createScanner(tableName).forEach((k, v) -> rows.add(k.getRow().toString()));
+      assertEquals(List.of("1", "2", "3", "4"), actualRows);

Review comment:
       Oops, my initial suggestion renamed this variable.
   
   ```suggestion
         assertEquals(List.of("1", "2", "3", "4"), rows);
   ```




-- 
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.

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