You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/11/14 17:24:57 UTC

[GitHub] [accumulo-proxy] DomGarguilo opened a new pull request, #41: Convert try-catch blocks in tests to JUnit assertThrows

DomGarguilo opened a new pull request, #41:
URL: https://github.com/apache/accumulo-proxy/pull/41

   Closes #36 
   
   In a lot of places in the tests, there are try-catch statements that are used to ensure that an exception is thrown. In this PR I tried to find usages of that pattern and replace them with `assertThrows()` statements which have a few advantages.


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo-proxy] DomGarguilo commented on a diff in pull request #41: Convert try-catch blocks in tests to JUnit assertThrows

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #41:
URL: https://github.com/apache/accumulo-proxy/pull/41#discussion_r1022043852


##########
src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java:
##########
@@ -974,320 +975,232 @@ public void testNamespaceClassLoadLoginFailure() {
   }
 
   @Test
-  public void tableNotFound() throws Exception {
+  public void tableNotFound() throws IOException {
     final String doesNotExist = "doesNotExists";
-    try {
-      client.addConstraint(creds, doesNotExist, NumericValueConstraint.class.getName());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.addSplits(creds, doesNotExist, Collections.<ByteBuffer> emptySet());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.addConstraint(creds, doesNotExist, NumericValueConstraint.class.getName()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.addSplits(creds, doesNotExist, Collections.emptySet()));
+
     final IteratorSetting setting = new IteratorSetting(100, "slow", SlowIterator.class.getName(),
         Collections.singletonMap("sleepTime", "200"));
-    try {
-      client.attachIterator(creds, doesNotExist, setting, EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.cancelCompaction(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.checkIteratorConflicts(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.clearLocatorCache(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      final String TABLE_TEST = getUniqueNameArray(1)[0];
-      client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.compactTable(creds, doesNotExist, null, null, null, true, false, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createBatchScanner(creds, doesNotExist, new BatchScanOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createScanner(creds, doesNotExist, new ScanOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createWriter(creds, doesNotExist, new WriterOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.deleteRows(creds, doesNotExist, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.deleteTable(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.exportTable(creds, doesNotExist, "/tmp");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.flushTable(creds, doesNotExist, null, null, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getIteratorSetting(creds, doesNotExist, "foo", IteratorScope.SCAN);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getLocalityGroups(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getMaxRow(creds, doesNotExist, Collections.<ByteBuffer> emptySet(), null, false, null,
-          false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getTableProperties(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.grantTablePermission(creds, "root", doesNotExist, TablePermission.WRITE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.hasTablePermission(creds, "root", doesNotExist, TablePermission.WRITE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
-      Path base = cluster.getTemporaryPath();
-      Path importDir = new Path(base, "importDir");
-      Path failuresDir = new Path(base, "failuresDir");
-      assertTrue(cluster.getFileSystem().mkdirs(importDir));
-      assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
-      client.importDirectory(creds, doesNotExist, importDir.toString(), failuresDir.toString(),
-          true);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.listConstraints(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.listSplits(creds, doesNotExist, 10000);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.mergeTablets(creds, doesNotExist, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.offlineTable(creds, doesNotExist, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.onlineTable(creds, doesNotExist, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeConstraint(creds, doesNotExist, 0);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeIterator(creds, doesNotExist, "name", EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.renameTable(creds, doesNotExist, "someTableName");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.revokeTablePermission(creds, "root", doesNotExist, TablePermission.ALTER_TABLE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.setTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey(), "0");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.splitRangeByTablets(creds, doesNotExist,
-          client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.updateAndFlush(creds, doesNotExist, new HashMap<ByteBuffer,List<ColumnUpdate>>());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getDiskUsage(creds, Collections.singleton(doesNotExist));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.testTableClassLoad(creds, doesNotExist, VersioningIterator.class.getName(),
-          SortedKeyValueIterator.class.getName());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createConditionalWriter(creds, doesNotExist, new ConditionalWriterOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
+
+    assertThrows(TableNotFoundException.class, () -> client.attachIterator(creds, doesNotExist,
+        setting, EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class, () -> client.cancelCompaction(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.checkIteratorConflicts(creds,
+        doesNotExist, setting, EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class, () -> client.clearLocatorCache(creds, doesNotExist));
+
+    final String TABLE_TEST = getUniqueNameArray(1)[0];
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.compactTable(creds, doesNotExist, null, null, null, true, false, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createBatchScanner(creds, doesNotExist, new BatchScanOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createScanner(creds, doesNotExist, new ScanOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createWriter(creds, doesNotExist, new WriterOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.deleteRows(creds, doesNotExist, null, null));
+
+    assertThrows(TableNotFoundException.class, () -> client.deleteTable(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.exportTable(creds, doesNotExist, "/tmp"));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.flushTable(creds, doesNotExist, null, null, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getIteratorSetting(creds, doesNotExist, "foo", IteratorScope.SCAN));
+
+    assertThrows(TableNotFoundException.class, () -> client.getLocalityGroups(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.getMaxRow(creds, doesNotExist,
+        Collections.emptySet(), null, false, null, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getTableProperties(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.grantTablePermission(creds, "root", doesNotExist, TablePermission.WRITE));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.hasTablePermission(creds, "root", doesNotExist, TablePermission.WRITE));
+
+    MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
+    Path base = cluster.getTemporaryPath();
+    Path importDir = new Path(base, "importDir");
+    Path failuresDir = new Path(base, "failuresDir");
+    assertTrue(cluster.getFileSystem().mkdirs(importDir));
+    assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
+
+    assertThrows(TableNotFoundException.class, () -> client.importDirectory(creds, doesNotExist,
+        importDir.toString(), failuresDir.toString(), true));
+
+    assertThrows(TableNotFoundException.class, () -> client.listConstraints(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.listSplits(creds, doesNotExist, 10000));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.mergeTablets(creds, doesNotExist, null, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.offlineTable(creds, doesNotExist, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.onlineTable(creds, doesNotExist, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.removeConstraint(creds, doesNotExist, 0));
+
+    assertThrows(TableNotFoundException.class, () -> client.removeIterator(creds, doesNotExist,
+        "name", EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.removeTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.renameTable(creds, doesNotExist, "someTableName"));
+
+    assertThrows(TableNotFoundException.class, () -> client.revokeTablePermission(creds, "root",
+        doesNotExist, TablePermission.ALTER_TABLE));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.setTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey(), "0"));
+
+    assertThrows(TableNotFoundException.class, () -> client.splitRangeByTablets(creds, doesNotExist,
+        client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.updateAndFlush(creds, doesNotExist, new HashMap<>()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getDiskUsage(creds, Collections.singleton(doesNotExist)));
+
+    assertThrows(TableNotFoundException.class, () -> client.testTableClassLoad(creds, doesNotExist,
+        VersioningIterator.class.getName(), SortedKeyValueIterator.class.getName()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createConditionalWriter(creds, doesNotExist, new ConditionalWriterOptions()));
   }
 
   @Test
-  public void namespaceNotFound() throws Exception {
+  public void namespaceNotFound() {
     final String doesNotExist = "doesNotExists";
-    try {
-      client.deleteNamespace(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.renameNamespace(creds, doesNotExist, "abcdefg");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.setNamespaceProperty(creds, doesNotExist, "table.compaction.major.ratio", "4");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceProperty(creds, doesNotExist, "table.compaction.major.ratio");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.getNamespaceProperties(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
-          DebugIterator.class.getName(), Collections.emptyMap());
-      client.attachNamespaceIterator(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceIterator(creds, doesNotExist, "DebugTheThings",
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.getNamespaceIteratorSetting(creds, doesNotExist, "DebugTheThings", IteratorScope.SCAN);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.listNamespaceIterators(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
-          DebugIterator.class.getName(), Collections.emptyMap());
-      client.checkNamespaceIteratorConflicts(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.addNamespaceConstraint(creds, doesNotExist, MaxMutationSize.class.getName());
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceConstraint(creds, doesNotExist, 1);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.listNamespaceConstraints(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.testNamespaceClassLoad(creds, doesNotExist, DebugIterator.class.getName(),
-          SortedKeyValueIterator.class.getName());
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
+    IteratorSetting iteratorSetting = new IteratorSetting(100, "DebugTheThings",
+        DebugIterator.class.getName(), Collections.emptyMap());
+
+    assertThrows(NamespaceNotFoundException.class,
+        () -> client.deleteNamespace(creds, doesNotExist));
+
+    assertThrows(NamespaceNotFoundException.class,
+        () -> client.renameNamespace(creds, doesNotExist, "abcdefg"));
+
+    assertThrows(NamespaceNotFoundException.class, () -> client.setNamespaceProperty(creds,
+        doesNotExist, "table.compaction.major.ratio", "4"));

Review Comment:
   Made these changes to one of the tests in e2fb8c55e1537194de54fdf8c35bd647a5eda5f4. Working on the others now.



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo-proxy] DomGarguilo commented on a diff in pull request #41: Convert try-catch blocks in tests to JUnit assertThrows

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #41:
URL: https://github.com/apache/accumulo-proxy/pull/41#discussion_r1021971334


##########
src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java:
##########
@@ -974,320 +975,232 @@ public void testNamespaceClassLoadLoginFailure() {
   }
 
   @Test
-  public void tableNotFound() throws Exception {
+  public void tableNotFound() throws IOException {
     final String doesNotExist = "doesNotExists";
-    try {
-      client.addConstraint(creds, doesNotExist, NumericValueConstraint.class.getName());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.addSplits(creds, doesNotExist, Collections.<ByteBuffer> emptySet());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.addConstraint(creds, doesNotExist, NumericValueConstraint.class.getName()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.addSplits(creds, doesNotExist, Collections.emptySet()));
+
     final IteratorSetting setting = new IteratorSetting(100, "slow", SlowIterator.class.getName(),
         Collections.singletonMap("sleepTime", "200"));
-    try {
-      client.attachIterator(creds, doesNotExist, setting, EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.cancelCompaction(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.checkIteratorConflicts(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.clearLocatorCache(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      final String TABLE_TEST = getUniqueNameArray(1)[0];
-      client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.compactTable(creds, doesNotExist, null, null, null, true, false, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createBatchScanner(creds, doesNotExist, new BatchScanOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createScanner(creds, doesNotExist, new ScanOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createWriter(creds, doesNotExist, new WriterOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.deleteRows(creds, doesNotExist, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.deleteTable(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.exportTable(creds, doesNotExist, "/tmp");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.flushTable(creds, doesNotExist, null, null, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getIteratorSetting(creds, doesNotExist, "foo", IteratorScope.SCAN);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getLocalityGroups(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getMaxRow(creds, doesNotExist, Collections.<ByteBuffer> emptySet(), null, false, null,
-          false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getTableProperties(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.grantTablePermission(creds, "root", doesNotExist, TablePermission.WRITE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.hasTablePermission(creds, "root", doesNotExist, TablePermission.WRITE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
-      Path base = cluster.getTemporaryPath();
-      Path importDir = new Path(base, "importDir");
-      Path failuresDir = new Path(base, "failuresDir");
-      assertTrue(cluster.getFileSystem().mkdirs(importDir));
-      assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
-      client.importDirectory(creds, doesNotExist, importDir.toString(), failuresDir.toString(),
-          true);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.listConstraints(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.listSplits(creds, doesNotExist, 10000);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.mergeTablets(creds, doesNotExist, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.offlineTable(creds, doesNotExist, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.onlineTable(creds, doesNotExist, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeConstraint(creds, doesNotExist, 0);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeIterator(creds, doesNotExist, "name", EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.renameTable(creds, doesNotExist, "someTableName");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.revokeTablePermission(creds, "root", doesNotExist, TablePermission.ALTER_TABLE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.setTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey(), "0");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.splitRangeByTablets(creds, doesNotExist,
-          client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.updateAndFlush(creds, doesNotExist, new HashMap<ByteBuffer,List<ColumnUpdate>>());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getDiskUsage(creds, Collections.singleton(doesNotExist));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.testTableClassLoad(creds, doesNotExist, VersioningIterator.class.getName(),
-          SortedKeyValueIterator.class.getName());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createConditionalWriter(creds, doesNotExist, new ConditionalWriterOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
+
+    assertThrows(TableNotFoundException.class, () -> client.attachIterator(creds, doesNotExist,
+        setting, EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class, () -> client.cancelCompaction(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.checkIteratorConflicts(creds,
+        doesNotExist, setting, EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class, () -> client.clearLocatorCache(creds, doesNotExist));
+
+    final String TABLE_TEST = getUniqueNameArray(1)[0];
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.compactTable(creds, doesNotExist, null, null, null, true, false, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createBatchScanner(creds, doesNotExist, new BatchScanOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createScanner(creds, doesNotExist, new ScanOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createWriter(creds, doesNotExist, new WriterOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.deleteRows(creds, doesNotExist, null, null));
+
+    assertThrows(TableNotFoundException.class, () -> client.deleteTable(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.exportTable(creds, doesNotExist, "/tmp"));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.flushTable(creds, doesNotExist, null, null, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getIteratorSetting(creds, doesNotExist, "foo", IteratorScope.SCAN));
+
+    assertThrows(TableNotFoundException.class, () -> client.getLocalityGroups(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.getMaxRow(creds, doesNotExist,
+        Collections.emptySet(), null, false, null, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getTableProperties(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.grantTablePermission(creds, "root", doesNotExist, TablePermission.WRITE));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.hasTablePermission(creds, "root", doesNotExist, TablePermission.WRITE));
+
+    MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
+    Path base = cluster.getTemporaryPath();
+    Path importDir = new Path(base, "importDir");
+    Path failuresDir = new Path(base, "failuresDir");
+    assertTrue(cluster.getFileSystem().mkdirs(importDir));
+    assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
+
+    assertThrows(TableNotFoundException.class, () -> client.importDirectory(creds, doesNotExist,
+        importDir.toString(), failuresDir.toString(), true));
+
+    assertThrows(TableNotFoundException.class, () -> client.listConstraints(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.listSplits(creds, doesNotExist, 10000));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.mergeTablets(creds, doesNotExist, null, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.offlineTable(creds, doesNotExist, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.onlineTable(creds, doesNotExist, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.removeConstraint(creds, doesNotExist, 0));
+
+    assertThrows(TableNotFoundException.class, () -> client.removeIterator(creds, doesNotExist,
+        "name", EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.removeTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.renameTable(creds, doesNotExist, "someTableName"));
+
+    assertThrows(TableNotFoundException.class, () -> client.revokeTablePermission(creds, "root",
+        doesNotExist, TablePermission.ALTER_TABLE));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.setTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey(), "0"));
+
+    assertThrows(TableNotFoundException.class, () -> client.splitRangeByTablets(creds, doesNotExist,
+        client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.updateAndFlush(creds, doesNotExist, new HashMap<>()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getDiskUsage(creds, Collections.singleton(doesNotExist)));
+
+    assertThrows(TableNotFoundException.class, () -> client.testTableClassLoad(creds, doesNotExist,
+        VersioningIterator.class.getName(), SortedKeyValueIterator.class.getName()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createConditionalWriter(creds, doesNotExist, new ConditionalWriterOptions()));
   }
 
   @Test
-  public void namespaceNotFound() throws Exception {
+  public void namespaceNotFound() {
     final String doesNotExist = "doesNotExists";
-    try {
-      client.deleteNamespace(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.renameNamespace(creds, doesNotExist, "abcdefg");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.setNamespaceProperty(creds, doesNotExist, "table.compaction.major.ratio", "4");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceProperty(creds, doesNotExist, "table.compaction.major.ratio");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.getNamespaceProperties(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
-          DebugIterator.class.getName(), Collections.emptyMap());
-      client.attachNamespaceIterator(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceIterator(creds, doesNotExist, "DebugTheThings",
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.getNamespaceIteratorSetting(creds, doesNotExist, "DebugTheThings", IteratorScope.SCAN);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.listNamespaceIterators(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
-          DebugIterator.class.getName(), Collections.emptyMap());
-      client.checkNamespaceIteratorConflicts(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.addNamespaceConstraint(creds, doesNotExist, MaxMutationSize.class.getName());
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceConstraint(creds, doesNotExist, 1);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.listNamespaceConstraints(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.testNamespaceClassLoad(creds, doesNotExist, DebugIterator.class.getName(),
-          SortedKeyValueIterator.class.getName());
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
+    IteratorSetting iteratorSetting = new IteratorSetting(100, "DebugTheThings",
+        DebugIterator.class.getName(), Collections.emptyMap());
+
+    assertThrows(NamespaceNotFoundException.class,
+        () -> client.deleteNamespace(creds, doesNotExist));
+
+    assertThrows(NamespaceNotFoundException.class,
+        () -> client.renameNamespace(creds, doesNotExist, "abcdefg"));
+
+    assertThrows(NamespaceNotFoundException.class, () -> client.setNamespaceProperty(creds,
+        doesNotExist, "table.compaction.major.ratio", "4"));

Review Comment:
   I was considering doing something like this. I'll take another look



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo-proxy] ctubbsii commented on a diff in pull request #41: Convert try-catch blocks in tests to JUnit assertThrows

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #41:
URL: https://github.com/apache/accumulo-proxy/pull/41#discussion_r1021956444


##########
src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java:
##########
@@ -974,320 +975,232 @@ public void testNamespaceClassLoadLoginFailure() {
   }
 
   @Test
-  public void tableNotFound() throws Exception {
+  public void tableNotFound() throws IOException {
     final String doesNotExist = "doesNotExists";
-    try {
-      client.addConstraint(creds, doesNotExist, NumericValueConstraint.class.getName());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.addSplits(creds, doesNotExist, Collections.<ByteBuffer> emptySet());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.addConstraint(creds, doesNotExist, NumericValueConstraint.class.getName()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.addSplits(creds, doesNotExist, Collections.emptySet()));
+
     final IteratorSetting setting = new IteratorSetting(100, "slow", SlowIterator.class.getName(),
         Collections.singletonMap("sleepTime", "200"));
-    try {
-      client.attachIterator(creds, doesNotExist, setting, EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.cancelCompaction(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.checkIteratorConflicts(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.clearLocatorCache(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      final String TABLE_TEST = getUniqueNameArray(1)[0];
-      client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.compactTable(creds, doesNotExist, null, null, null, true, false, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createBatchScanner(creds, doesNotExist, new BatchScanOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createScanner(creds, doesNotExist, new ScanOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createWriter(creds, doesNotExist, new WriterOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.deleteRows(creds, doesNotExist, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.deleteTable(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.exportTable(creds, doesNotExist, "/tmp");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.flushTable(creds, doesNotExist, null, null, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getIteratorSetting(creds, doesNotExist, "foo", IteratorScope.SCAN);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getLocalityGroups(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getMaxRow(creds, doesNotExist, Collections.<ByteBuffer> emptySet(), null, false, null,
-          false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getTableProperties(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.grantTablePermission(creds, "root", doesNotExist, TablePermission.WRITE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.hasTablePermission(creds, "root", doesNotExist, TablePermission.WRITE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
-      Path base = cluster.getTemporaryPath();
-      Path importDir = new Path(base, "importDir");
-      Path failuresDir = new Path(base, "failuresDir");
-      assertTrue(cluster.getFileSystem().mkdirs(importDir));
-      assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
-      client.importDirectory(creds, doesNotExist, importDir.toString(), failuresDir.toString(),
-          true);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.listConstraints(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.listSplits(creds, doesNotExist, 10000);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.mergeTablets(creds, doesNotExist, null, null);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.offlineTable(creds, doesNotExist, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.onlineTable(creds, doesNotExist, false);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeConstraint(creds, doesNotExist, 0);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeIterator(creds, doesNotExist, "name", EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.removeTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.renameTable(creds, doesNotExist, "someTableName");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.revokeTablePermission(creds, "root", doesNotExist, TablePermission.ALTER_TABLE);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.setTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey(), "0");
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.splitRangeByTablets(creds, doesNotExist,
-          client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10);
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.updateAndFlush(creds, doesNotExist, new HashMap<ByteBuffer,List<ColumnUpdate>>());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.getDiskUsage(creds, Collections.singleton(doesNotExist));
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.testTableClassLoad(creds, doesNotExist, VersioningIterator.class.getName(),
-          SortedKeyValueIterator.class.getName());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
-    try {
-      client.createConditionalWriter(creds, doesNotExist, new ConditionalWriterOptions());
-      fail("exception not thrown");
-    } catch (TableNotFoundException ex) {}
+
+    assertThrows(TableNotFoundException.class, () -> client.attachIterator(creds, doesNotExist,
+        setting, EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class, () -> client.cancelCompaction(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.checkIteratorConflicts(creds,
+        doesNotExist, setting, EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class, () -> client.clearLocatorCache(creds, doesNotExist));
+
+    final String TABLE_TEST = getUniqueNameArray(1)[0];
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.cloneTable(creds, doesNotExist, TABLE_TEST, false, null, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.compactTable(creds, doesNotExist, null, null, null, true, false, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createBatchScanner(creds, doesNotExist, new BatchScanOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createScanner(creds, doesNotExist, new ScanOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createWriter(creds, doesNotExist, new WriterOptions()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.deleteRows(creds, doesNotExist, null, null));
+
+    assertThrows(TableNotFoundException.class, () -> client.deleteTable(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.exportTable(creds, doesNotExist, "/tmp"));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.flushTable(creds, doesNotExist, null, null, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getIteratorSetting(creds, doesNotExist, "foo", IteratorScope.SCAN));
+
+    assertThrows(TableNotFoundException.class, () -> client.getLocalityGroups(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.getMaxRow(creds, doesNotExist,
+        Collections.emptySet(), null, false, null, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getTableProperties(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.grantTablePermission(creds, "root", doesNotExist, TablePermission.WRITE));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.hasTablePermission(creds, "root", doesNotExist, TablePermission.WRITE));
+
+    MiniAccumuloClusterImpl cluster = SharedMiniClusterBase.getCluster();
+    Path base = cluster.getTemporaryPath();
+    Path importDir = new Path(base, "importDir");
+    Path failuresDir = new Path(base, "failuresDir");
+    assertTrue(cluster.getFileSystem().mkdirs(importDir));
+    assertTrue(cluster.getFileSystem().mkdirs(failuresDir));
+
+    assertThrows(TableNotFoundException.class, () -> client.importDirectory(creds, doesNotExist,
+        importDir.toString(), failuresDir.toString(), true));
+
+    assertThrows(TableNotFoundException.class, () -> client.listConstraints(creds, doesNotExist));
+
+    assertThrows(TableNotFoundException.class, () -> client.listSplits(creds, doesNotExist, 10000));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.mergeTablets(creds, doesNotExist, null, null));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.offlineTable(creds, doesNotExist, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.onlineTable(creds, doesNotExist, false));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.removeConstraint(creds, doesNotExist, 0));
+
+    assertThrows(TableNotFoundException.class, () -> client.removeIterator(creds, doesNotExist,
+        "name", EnumSet.allOf(IteratorScope.class)));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.removeTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.renameTable(creds, doesNotExist, "someTableName"));
+
+    assertThrows(TableNotFoundException.class, () -> client.revokeTablePermission(creds, "root",
+        doesNotExist, TablePermission.ALTER_TABLE));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.setTableProperty(creds, doesNotExist, Property.TABLE_FILE_MAX.getKey(), "0"));
+
+    assertThrows(TableNotFoundException.class, () -> client.splitRangeByTablets(creds, doesNotExist,
+        client.getRowRange(ByteBuffer.wrap("row".getBytes(UTF_8))), 10));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.updateAndFlush(creds, doesNotExist, new HashMap<>()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.getDiskUsage(creds, Collections.singleton(doesNotExist)));
+
+    assertThrows(TableNotFoundException.class, () -> client.testTableClassLoad(creds, doesNotExist,
+        VersioningIterator.class.getName(), SortedKeyValueIterator.class.getName()));
+
+    assertThrows(TableNotFoundException.class,
+        () -> client.createConditionalWriter(creds, doesNotExist, new ConditionalWriterOptions()));
   }
 
   @Test
-  public void namespaceNotFound() throws Exception {
+  public void namespaceNotFound() {
     final String doesNotExist = "doesNotExists";
-    try {
-      client.deleteNamespace(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.renameNamespace(creds, doesNotExist, "abcdefg");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.setNamespaceProperty(creds, doesNotExist, "table.compaction.major.ratio", "4");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceProperty(creds, doesNotExist, "table.compaction.major.ratio");
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.getNamespaceProperties(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
-          DebugIterator.class.getName(), Collections.emptyMap());
-      client.attachNamespaceIterator(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceIterator(creds, doesNotExist, "DebugTheThings",
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.getNamespaceIteratorSetting(creds, doesNotExist, "DebugTheThings", IteratorScope.SCAN);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.listNamespaceIterators(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      IteratorSetting setting = new IteratorSetting(100, "DebugTheThings",
-          DebugIterator.class.getName(), Collections.emptyMap());
-      client.checkNamespaceIteratorConflicts(creds, doesNotExist, setting,
-          EnumSet.allOf(IteratorScope.class));
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.addNamespaceConstraint(creds, doesNotExist, MaxMutationSize.class.getName());
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.removeNamespaceConstraint(creds, doesNotExist, 1);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.listNamespaceConstraints(creds, doesNotExist);
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
-    try {
-      client.testNamespaceClassLoad(creds, doesNotExist, DebugIterator.class.getName(),
-          SortedKeyValueIterator.class.getName());
-      fail("exception not thrown");
-    } catch (NamespaceNotFoundException ex) {}
+    IteratorSetting iteratorSetting = new IteratorSetting(100, "DebugTheThings",
+        DebugIterator.class.getName(), Collections.emptyMap());
+
+    assertThrows(NamespaceNotFoundException.class,
+        () -> client.deleteNamespace(creds, doesNotExist));
+
+    assertThrows(NamespaceNotFoundException.class,
+        () -> client.renameNamespace(creds, doesNotExist, "abcdefg"));
+
+    assertThrows(NamespaceNotFoundException.class, () -> client.setNamespaceProperty(creds,
+        doesNotExist, "table.compaction.major.ratio", "4"));

Review Comment:
   Since a lot of these are checking for the same exception, I was thinking you could just loop over all these lambda's, something like:
   
   ```java
       Stream.<Executable>of(() -> client.deleteNamespace(creds, doesNotExist),
         () -> client.renameNamespace(creds, doesNotExist, "abcdefg"),
         ...
   
       .forEach(executable -> assertThrows(NamespaceNotFoundException.class, executable));
    ```
   
   This removes a lot of boilerplate, since we're testing the same thing over and over again. This is great if everything is passing, but it might be hard to troubleshoot which one had a problem when there's a failure. That might be easier to troubleshoot by adding a loop counter to track which executable failed to throw the expected exception and broke out of the loop.
   
   In any case, it's fine the way you have it now.



##########
src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java:
##########
@@ -1738,17 +1639,15 @@ public void userPermissions() throws Exception {
       assertFalse(client.listTables(creds).contains("fail"));
     }
     // denied!
-    try {
-      if (isKerberosEnabled()) {
-        // Switch back to the extra user
-        UserGroupInformation.loginUserFromKeytab(otherClient.getPrincipal(),
-            otherClient.getKeytab().getAbsolutePath());
-        client = userClient;
-      }
-      String scanner = client.createScanner(user, tableName, null);
-      client.nextK(scanner, 100);
-      fail("stooge should not read table test");
-    } catch (AccumuloSecurityException ex) {}
+    if (isKerberosEnabled()) {
+      // Switch back to the extra user
+      UserGroupInformation.loginUserFromKeytab(otherClient.getPrincipal(),
+          otherClient.getKeytab().getAbsolutePath());
+      client = userClient;
+    }
+    String scanner = client.createScanner(user, tableName, null);
+    assertThrows(AccumuloSecurityException.class, () -> client.nextK(scanner, 100),
+        "stooge should not read table test");

Review Comment:
   The block was removed, causing variable name collisions for some variables, like `scanner`. If you prefer to reuse the variable names instead of renaming them, you can put them inside an anonymous block (keep the block, but drop the `try` and `catch`), as in:
   
   ```java
       {
         String scanner = client.createScanner(user, tableName, null);
         assertThrows(AccumuloSecurityException.class, () -> client.nextK(scanner, 100),
             "stooge should not read table test");
       }
   ```
   
   Or you can just use a different variable name, like you did. It's up to you. I just wanted to make sure you were aware. I sometimes use anonymous blocks to more strictly control the scope of commonly named temporary variables, so they don't get accidentally referenced later.



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo-proxy] DomGarguilo merged pull request #41: Convert try-catch blocks in tests to JUnit assertThrows

Posted by GitBox <gi...@apache.org>.
DomGarguilo merged PR #41:
URL: https://github.com/apache/accumulo-proxy/pull/41


-- 
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: notifications-unsubscribe@accumulo.apache.org

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