From 48ab844de09a54fcaa4efd300c806ade41db2314 Mon Sep 17 00:00:00 2001 From: Mike Drob Date: Thu, 28 Jun 2018 13:44:41 -0500 Subject: [PATCH] HBASE-20814 error-prone assertion failure ignored --- hbase-build-configuration/pom.xml | 1 + .../hbase/security/access/AccessControlClient.java | 3 +- .../apache/hadoop/hbase/TestCellComparator.java | 39 +++-- .../java/org/apache/hadoop/hbase/TestKeyValue.java | 26 +-- .../hadoop/hbase/coprocessor/TestSecureExport.java | 7 +- .../hadoop/hbase/ipc/IntegrationTestRpcClient.java | 1 + .../hadoop/hbase/mapreduce/TestSyncTable.java | 112 ++++-------- .../hbase/procedure2/TestProcedureNonce.java | 111 ++++++------ .../hadoop/hbase/rsgroup/TestRSGroupsWithACL.java | 11 +- .../hadoop/hbase/client/TestFromClientSide.java | 1 + .../master/procedure/TestServerCrashProcedure.java | 15 +- .../procedure/TestTruncateTableProcedure.java | 32 +--- .../security/access/TestAccessController.java | 65 +++---- .../security/access/TestAccessController3.java | 6 +- .../TestVisibilityLabelsWithDeletes.java | 189 +++------------------ 15 files changed, 191 insertions(+), 428 deletions(-) diff --git a/hbase-build-configuration/pom.xml b/hbase-build-configuration/pom.xml index a617083e48..3c0dc48c13 100644 --- a/hbase-build-configuration/pom.xml +++ b/hbase-build-configuration/pom.xml @@ -84,6 +84,7 @@ -Xep:FallThrough:OFF -Xep:ClassNewInstance:ERROR -Xep:MissingDefault:ERROR + -Xep:AssertionFailureIgnored:ERROR diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java index 0363ba269d..569cd37eea 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlClient.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import java.util.regex.Pattern; +import com.google.protobuf.ServiceException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.MasterNotRunningException; @@ -259,7 +260,7 @@ public class AccessControlClient { * @throws Throwable */ public static List getUserPermissions(Connection connection, String tableRegex) - throws Throwable { + throws IOException, ServiceException { /** TODO: Pass an rpcController HBaseRpcController controller = ((ClusterConnection) connection).getRpcControllerFactory().newController(); diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestCellComparator.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestCellComparator.java index a318515832..4d2cd4c674 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestCellComparator.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestCellComparator.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.nio.ByteBuffer; import java.util.Collections; @@ -207,7 +208,6 @@ public class TestCellComparator { @Test public void testBinaryKeys() throws Exception { - Set set = new TreeSet<>(CellComparatorImpl.COMPARATOR); final byte [] fam = Bytes.toBytes("col"); final byte [] qf = Bytes.toBytes("umn"); final byte [] nb = new byte[0]; @@ -225,26 +225,31 @@ public class TestCellComparator { createByteBufferKeyValueFromKeyValue( new KeyValue(Bytes.toBytes("a,a,0"), fam, qf, 0, nb)), }; - // Add to set with bad comparator + // Add to set with bad comparator, expect keys to output incorrectly + Set set = new TreeSet<>(CellComparatorImpl.COMPARATOR); Collections.addAll(set, keys); - // This will output the keys incorrectly. - boolean assertion = false; - int count = 0; - try { - for (Cell k: set) { - assertTrue("count=" + count + ", " + k.toString(), count++ == k.getTimestamp()); - } - } catch (AssertionError e) { - // Expected - assertion = true; - } - assertTrue(assertion); + expectIncorrectOrder(set); // Make set with good comparator set = new TreeSet<>(CellComparatorImpl.META_COMPARATOR); Collections.addAll(set, keys); - count = 0; - for (Cell k: set) { - assertTrue("count=" + count + ", " + k.toString(), count++ == k.getTimestamp()); + expectCorrectOrder(set); + } + + public static void expectIncorrectOrder(Set set) { + int count = 0; + for (Cell cell : set) { + if(count++ != cell.getTimestamp()) { + return; + } + } + fail("Data should not have been returned in timestamp order: " + set); + } + + public static void expectCorrectOrder(Set set) { + int count = 0; + for (Cell cell : set) { + assertEquals("count=" + count + ", " + cell, count, cell.getTimestamp()); + count++; } } } diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java index 167a0302f4..1b917b2f1e 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java @@ -39,6 +39,7 @@ import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.ByteBufferUtils; import org.apache.hadoop.hbase.util.Bytes; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; import org.slf4j.Logger; @@ -239,8 +240,7 @@ public class TestKeyValue { } @Test - public void testBinaryKeys() throws Exception { - Set set = new TreeSet<>(CellComparatorImpl.COMPARATOR); + public void testBinaryKeys() { final byte [] fam = Bytes.toBytes("col"); final byte [] qf = Bytes.toBytes("umn"); final byte [] nb = new byte[0]; @@ -251,29 +251,17 @@ public class TestKeyValue { new KeyValue(Bytes.toBytes("aaaaa,a,4"), fam, qf, 4, nb), new KeyValue(Bytes.toBytes("a,a,0"), fam, qf, 0, nb), }; - // Add to set with bad comparator + // Add to set with bad comparator, expect keys to output incorrectly + Set set = new TreeSet<>(CellComparatorImpl.COMPARATOR); Collections.addAll(set, keys); - // This will output the keys incorrectly. - boolean assertion = false; - int count = 0; - try { - for (KeyValue k: set) { - assertTrue(count++ == k.getTimestamp()); - } - } catch (java.lang.AssertionError e) { - // Expected - assertion = true; - } - assertTrue(assertion); + TestCellComparator.expectIncorrectOrder(set); // Make set with good comparator set = new TreeSet<>(CellComparatorImpl.META_COMPARATOR); Collections.addAll(set, keys); - count = 0; - for (KeyValue k: set) { - assertTrue(count++ == k.getTimestamp()); - } + TestCellComparator.expectCorrectOrder(set); } + @Ignore @Test public void testStackedUpKeyValue() { // Test multiple KeyValues in a single blob. diff --git a/hbase-endpoint/src/test/java/org/apache/hadoop/hbase/coprocessor/TestSecureExport.java b/hbase-endpoint/src/test/java/org/apache/hadoop/hbase/coprocessor/TestSecureExport.java index 21f17f76a8..2811c53cfd 100644 --- a/hbase-endpoint/src/test/java/org/apache/hadoop/hbase/coprocessor/TestSecureExport.java +++ b/hbase-endpoint/src/test/java/org/apache/hadoop/hbase/coprocessor/TestSecureExport.java @@ -270,6 +270,7 @@ public class TestSecureExport { * Test the ExportEndpoint's access levels. The {@link Export} test is ignored * since the access exceptions cannot be collected from the mappers. */ + @SuppressWarnings("AssertionFailureIgnored") @Test public void testAccessCase() throws Throwable { final String exportTable = name.getMethodName(); @@ -318,9 +319,9 @@ public class TestSecureExport { final Path output = fs.makeQualified(new Path(openDir, "output")); AccessTestAction exportAction = () -> { try { - String[] args = new String[]{exportTable, output.toString()}; - Map result - = Export.run(new Configuration(UTIL.getConfiguration()), args); + String[] args = new String[] { exportTable, output.toString() }; + Map result = + Export.run(new Configuration(UTIL.getConfiguration()), args); long rowCount = 0; long cellCount = 0; for (Export.Response r : result.values()) { diff --git a/hbase-it/src/test/java/org/apache/hadoop/hbase/ipc/IntegrationTestRpcClient.java b/hbase-it/src/test/java/org/apache/hadoop/hbase/ipc/IntegrationTestRpcClient.java index 2dd163305b..383c2dc4e0 100644 --- a/hbase-it/src/test/java/org/apache/hadoop/hbase/ipc/IntegrationTestRpcClient.java +++ b/hbase-it/src/test/java/org/apache/hadoop/hbase/ipc/IntegrationTestRpcClient.java @@ -247,6 +247,7 @@ public class IntegrationTestRpcClient { this.setName(id); } + @SuppressWarnings("AssertionFailureIgnored") @Override public void run() { while (running.get()) { diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestSyncTable.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestSyncTable.java index ad02039fb9..2d9fccb3c9 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestSyncTable.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestSyncTable.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.mapreduce; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.util.Arrays; @@ -181,38 +182,9 @@ public class TestSyncTable { } Cell[] sourceCells = sourceRow.rawCells(); Cell[] targetCells = targetRow.rawCells(); - if (sourceCells.length != targetCells.length) { - LOG.debug("Source cells: " + Arrays.toString(sourceCells)); - LOG.debug("Target cells: " + Arrays.toString(targetCells)); - Assert.fail("Row " + Bytes.toInt(sourceRow.getRow()) - + " has " + sourceCells.length - + " cells in source table but " + targetCells.length - + " cells in target table"); - } - for (int j = 0; j < sourceCells.length; j++) { - Cell sourceCell = sourceCells[j]; - Cell targetCell = targetCells[j]; - try { - if (!CellUtil.matchingRows(sourceCell, targetCell)) { - Assert.fail("Rows don't match"); - } - if (!CellUtil.matchingFamily(sourceCell, targetCell)) { - Assert.fail("Families don't match"); - } - if (!CellUtil.matchingQualifier(sourceCell, targetCell)) { - Assert.fail("Qualifiers don't match"); - } - if (!CellUtil.matchingTimestamp(sourceCell, targetCell)) { - Assert.fail("Timestamps don't match"); - } - if (!CellUtil.matchingValue(sourceCell, targetCell)) { - Assert.fail("Values don't match"); - } - } catch (Throwable t) { - LOG.debug("Source cell: " + sourceCell + " target cell: " + targetCell); - Throwables.propagate(t); - } - } + assertEquals("Source and Target tables have different number of cells in row " + + Bytes.toInt(sourceRow.getRow()), sourceCells.length, targetCells.length); + assertCellsEqual(sourceCells, targetCells); } Result sourceRow = sourceScanner.next(); if (sourceRow != null) { @@ -230,6 +202,23 @@ public class TestSyncTable { targetTable.close(); } + private void assertCellsEqual(Cell[] sourceCells, Cell[] targetCells) { + for (int j = 0; j < sourceCells.length; j++) { + Cell sourceCell = sourceCells[j]; + Cell targetCell = targetCells[j]; + assertTrue("Rows don't match at cell #" + j, + CellUtil.matchingRows(sourceCell, targetCell)); + assertTrue("Families don't match at cell #" + j, + CellUtil.matchingFamily(sourceCell, targetCell)); + assertTrue("Qualifiers don't match at cell #" + j, + CellUtil.matchingQualifier(sourceCell, targetCell)); + assertTrue("Timestamps don't match at cell #" + j, + CellUtil.matchingTimestamp(sourceCell, targetCell)); + assertTrue("Values don't match at cell #" + j, + CellUtil.matchingValue(sourceCell, targetCell)); + } + } + private void assertTargetDoDeletesFalse(int expectedRows, TableName sourceTableName, TableName targetTableName) @@ -282,29 +271,18 @@ public class TestSyncTable { for (int j = 0; j < sourceCells.length; j++) { Cell sourceCell = sourceCells[j]; Cell targetCell = targetCells[j]; - try { - if (!CellUtil.matchingRow(sourceCell, targetCell)) { - Assert.fail("Rows don't match"); - } - if (!CellUtil.matchingFamily(sourceCell, targetCell)) { - Assert.fail("Families don't match"); - } - if (!CellUtil.matchingQualifier(sourceCell, targetCell)) { - Assert.fail("Qualifiers don't match"); - } - if(targetRowKey < 80 && targetRowKey >= 90){ - if (!CellUtil.matchingTimestamp(sourceCell, targetCell)) { - Assert.fail("Timestamps don't match"); - } - } - if (!CellUtil.matchingValue(sourceCell, targetCell)) { - Assert.fail("Values don't match"); - } - } catch (Throwable t) { - LOG.debug("Source cell: " + sourceCell + " target cell: " - + targetCell); - Throwables.propagate(t); + assertTrue("Rows don't match at cell #" + j, + CellUtil.matchingRows(sourceCell, targetCell)); + assertTrue("Families don't match at cell #" + j, + CellUtil.matchingFamily(sourceCell, targetCell)); + assertTrue("Qualifiers don't match at cell #" + j, + CellUtil.matchingQualifier(sourceCell, targetCell)); + if (targetRowKey < 80 && targetRowKey >= 90) { + assertTrue("Timestamps don't match at cell #" + j, + CellUtil.matchingTimestamp(sourceCell, targetCell)); } + assertTrue("Values don't match at cell #" + j, + CellUtil.matchingValue(sourceCell, targetCell)); } targetRow = targetScanner.next(); sourceRow = sourceScanner.next(); @@ -380,31 +358,7 @@ public class TestSyncTable { } } } else { - for (int j = 0; j < sourceCells.length; j++) { - Cell sourceCell = sourceCells[j]; - Cell targetCell = targetCells[j]; - try { - if (!CellUtil.matchingRow(sourceCell, targetCell)) { - Assert.fail("Rows don't match"); - } - if (!CellUtil.matchingFamily(sourceCell, targetCell)) { - Assert.fail("Families don't match"); - } - if (!CellUtil.matchingQualifier(sourceCell, targetCell)) { - Assert.fail("Qualifiers don't match"); - } - if (!CellUtil.matchingTimestamp(sourceCell, targetCell)) { - Assert.fail("Timestamps don't match"); - } - if (!CellUtil.matchingValue(sourceCell, targetCell)) { - Assert.fail("Values don't match"); - } - } catch (Throwable t) { - LOG.debug( - "Source cell: " + sourceCell + " target cell: " + targetCell); - Throwables.propagate(t); - } - } + assertCellsEqual(sourceCells, targetCells); } rowsCount++; targetRow = targetScanner.next(); diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureNonce.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureNonce.java index 2bf11fbaa0..d565b664e6 100644 --- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureNonce.java +++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureNonce.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.procedure2; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import java.util.concurrent.CountDownLatch; @@ -164,17 +165,18 @@ public class TestProcedureNonce { } @Test - public void testConcurrentNonceRegistration() throws IOException { + public void testConcurrentNonceRegistration() throws Throwable { testConcurrentNonceRegistration(true, 567, 44444); } @Test - public void testConcurrentNonceRegistrationWithRollback() throws IOException { + public void testConcurrentNonceRegistrationWithRollback() throws Throwable { testConcurrentNonceRegistration(false, 890, 55555); } + @SuppressWarnings("AssertionFailureIgnored") // We keep the exception and throw it later private void testConcurrentNonceRegistration(final boolean submitProcedure, - final long nonceGroup, final long nonce) throws IOException { + final long nonceGroup, final long nonce) throws Throwable { // register the nonce final NonceKey nonceKey = procExecutor.createNonceKey(nonceGroup, nonce); @@ -184,66 +186,63 @@ public class TestProcedureNonce { final CountDownLatch t1NonceRegisteredLatch = new CountDownLatch(1); final CountDownLatch t2BeforeNonceRegisteredLatch = new CountDownLatch(1); final Thread[] threads = new Thread[2]; - threads[0] = new Thread() { - @Override - public void run() { - try { - // release the nonce and wake t2 - assertFalse("unexpected already registered nonce", - procExecutor.registerNonce(nonceKey) >= 0); - t1NonceRegisteredLatch.countDown(); - - // hold the submission until t2 is registering the nonce - t2BeforeNonceRegisteredLatch.await(); - Threads.sleep(1000); - - if (submitProcedure) { - CountDownLatch latch = new CountDownLatch(1); - TestSingleStepProcedure proc = new TestSingleStepProcedure(); - procEnv.setWaitLatch(latch); - - procExecutor.submitProcedure(proc, nonceKey); - Threads.sleep(100); - - // complete the procedure - latch.countDown(); - } else { - procExecutor.unregisterNonceIfProcedureWasNotSubmitted(nonceKey); - } - } catch (Throwable e) { - t1Exception.set(e); - } finally { - t1NonceRegisteredLatch.countDown(); - t2BeforeNonceRegisteredLatch.countDown(); + threads[0] = new Thread(() -> { + try { + // release the nonce and wake t2 + assertFalse("unexpected already registered nonce", + procExecutor.registerNonce(nonceKey) >= 0); + t1NonceRegisteredLatch.countDown(); + + // hold the submission until t2 is registering the nonce + t2BeforeNonceRegisteredLatch.await(); + Threads.sleep(1000); + + if (submitProcedure) { + CountDownLatch latch = new CountDownLatch(1); + TestSingleStepProcedure proc = new TestSingleStepProcedure(); + procEnv.setWaitLatch(latch); + + procExecutor.submitProcedure(proc, nonceKey); + Threads.sleep(100); + + // complete the procedure + latch.countDown(); + } else { + procExecutor.unregisterNonceIfProcedureWasNotSubmitted(nonceKey); } + } catch (Throwable e) { + t1Exception.set(e); + } finally { + t1NonceRegisteredLatch.countDown(); + t2BeforeNonceRegisteredLatch.countDown(); } - }; - - threads[1] = new Thread() { - @Override - public void run() { - try { - // wait until t1 has registered the nonce - t1NonceRegisteredLatch.await(); - - // register the nonce - t2BeforeNonceRegisteredLatch.countDown(); - assertFalse("unexpected non registered nonce", - procExecutor.registerNonce(nonceKey) < 0); - } catch (Throwable e) { - t2Exception.set(e); - } finally { - t1NonceRegisteredLatch.countDown(); - t2BeforeNonceRegisteredLatch.countDown(); - } + }); + + threads[1] = new Thread(() -> { + try { + // wait until t1 has registered the nonce + t1NonceRegisteredLatch.await(); + + // register the nonce + t2BeforeNonceRegisteredLatch.countDown(); + assertFalse("unexpected non registered nonce", + procExecutor.registerNonce(nonceKey) < 0); + } catch (Throwable e) { + t2Exception.set(e); + } finally { + t1NonceRegisteredLatch.countDown(); + t2BeforeNonceRegisteredLatch.countDown(); } - }; + }); for (int i = 0; i < threads.length; ++i) threads[i].start(); for (int i = 0; i < threads.length; ++i) Threads.shutdown(threads[i]); ProcedureTestingUtility.waitNoProcedureRunning(procExecutor); - assertEquals(null, t1Exception.get()); - assertEquals(null, t2Exception.get()); + if (t1Exception.get() != null) { + throw t1Exception.get(); + } else if (t2Exception.get() != null) { + throw t2Exception.get(); + } } public static class TestSingleStepProcedure extends SequentialProcedure { diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java index a63626d994..ad5f0abe27 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java @@ -173,14 +173,11 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ grantGlobal(TEST_UTIL, toGroupEntry(GROUP_READ), Permission.Action.READ); grantGlobal(TEST_UTIL, toGroupEntry(GROUP_WRITE), Permission.Action.WRITE); + LOG.info("Checking table permissions"); assertEquals(4, AccessControlLists.getTablePermissions(conf, TEST_TABLE).size()); - try { - assertEquals(4, AccessControlClient.getUserPermissions(systemUserConnection, - TEST_TABLE.toString()).size()); - } catch (Throwable e) { - LOG.error("error during call of AccessControlClient.getUserPermissions. ", e); - fail("error during call of AccessControlClient.getUserPermissions."); - } + LOG.info("Checking user permissions"); + assertEquals(4, AccessControlClient.getUserPermissions(systemUserConnection, + TEST_TABLE.toString()).size()); } private static void cleanUp() throws Exception { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java index 13fa59f2a5..b7788beb48 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java @@ -4750,6 +4750,7 @@ public class TestFromClientSide { for (int versions = numVersions; versions < numVersions * 2; versions++) { final int versionsCopy = versions; executorService.submit(new Callable() { + @SuppressWarnings("AssertionFailureIgnored") @Override public Void call() { try { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerCrashProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerCrashProcedure.java index 9f7fafeca6..36967a56f6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerCrashProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerCrashProcedure.java @@ -115,9 +115,8 @@ public class TestServerCrashProcedure { throws Exception { final TableName tableName = TableName.valueOf( "testRecoveryAndDoubleExecution-carryingMeta-" + carryingMeta); - final Table t = this.util.createTable(tableName, HBaseTestingUtility.COLUMNS, - HBaseTestingUtility.KEYS_FOR_HBA_CREATE_TABLE); - try { + try (Table t = this.util.createTable(tableName, HBaseTestingUtility.COLUMNS, + HBaseTestingUtility.KEYS_FOR_HBA_CREATE_TABLE)) { // Load the table with a bit of data so some logs to split and some edits in each region. this.util.loadTable(t, HBaseTestingUtility.COLUMNS[0]); final int count = util.countRows(t); @@ -154,16 +153,6 @@ public class TestServerCrashProcedure { // Assert all data came back. assertEquals(count, util.countRows(t)); assertEquals(checksum, util.checksumRows(t)); - } catch(Throwable throwable) { - LOG.error("Test failed!", throwable); - throw throwable; - } finally { - t.close(); } } - - private void collectMasterMetrics() { - serverCrashSubmittedCount = serverCrashProcMetrics.getSubmittedCounter().getCount(); - serverCrashFailedCount = serverCrashProcMetrics.getFailedCounter().getCount(); - } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java index a8f7db5b7c..9e349c1f71 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestTruncateTableProcedure.java @@ -73,21 +73,13 @@ public class TestTruncateTableProcedure extends TestTableDDLProcedureBase { final ProcedureExecutor procExec = getMasterProcedureExecutor(); // HBASE-20178 has us fail-fast, in the constructor, so add try/catch for this case. - // Keep old way of looking at procedure too. - Throwable cause = null; try { - long procId = ProcedureTestingUtility.submitAndWait(procExec, + ProcedureTestingUtility.submitAndWait(procExec, new TruncateTableProcedure(procExec.getEnvironment(), tableName, true)); - - // Second delete should fail with TableNotFound - Procedure result = procExec.getResult(procId); - assertTrue(result.isFailed()); - cause = ProcedureTestingUtility.getExceptionCause(result); - } catch (Throwable t) { - cause = t; + fail("Should have thrown TNFE"); + } catch (TableNotFoundException e) { + // expected } - LOG.debug("Truncate failed with exception: " + cause); - assertTrue(cause instanceof TableNotFoundException); } @Test @@ -98,21 +90,13 @@ public class TestTruncateTableProcedure extends TestTableDDLProcedureBase { MasterProcedureTestingUtility.createTable(procExec, tableName, null, "f"); // HBASE-20178 has us fail-fast, in the constructor, so add try/catch for this case. - // Keep old way of looking at procedure too. - Throwable cause = null; try { - long procId = ProcedureTestingUtility.submitAndWait(procExec, + ProcedureTestingUtility.submitAndWait(procExec, new TruncateTableProcedure(procExec.getEnvironment(), tableName, false)); - - // Second delete should fail with TableNotDisabled - Procedure result = procExec.getResult(procId); - assertTrue(result.isFailed()); - cause = ProcedureTestingUtility.getExceptionCause(result); - } catch (Throwable t) { - cause = t; + fail("Should have thrown TNDE"); + } catch (TableNotDisabledException e) { + // expected } - LOG.debug("Truncate failed with exception: " + cause); - assertTrue(cause instanceof TableNotDisabledException); } @Test diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java index d2aa6823db..0cb9fe7a5f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java @@ -32,6 +32,7 @@ import com.google.protobuf.Service; import com.google.protobuf.ServiceException; import java.io.IOException; import java.security.PrivilegedAction; +import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -2688,53 +2689,39 @@ public class TestAccessController extends SecureTestUtil { * @param namespaceRegexWithoutPrefix the regualar expression for namespace, without NAMESPACE_PREFIX * @param expectedAmount the expected amount of user permissions returned * @param expectedNamespace the expected namespace of each user permission returned - * @throws HBaseException in the case of any HBase exception when accessing hbase:acl table + * @throws Exception in the case of any HBase exception when accessing hbase:acl table */ private void getNamespacePermissionsAndVerify(String namespaceRegexWithoutPrefix, - int expectedAmount, String expectedNamespace) throws HBaseException { - try { - List namespacePermissions = AccessControlClient.getUserPermissions( + int expectedAmount, String expectedNamespace) throws Exception { + List namespacePermissions = AccessControlClient.getUserPermissions( systemUserConnection, AccessControlLists.toNamespaceEntry(namespaceRegexWithoutPrefix)); - assertTrue(namespacePermissions != null); - assertEquals(expectedAmount, namespacePermissions.size()); - for (UserPermission namespacePermission : namespacePermissions) { - assertFalse(namespacePermission.isGlobal()); // Verify it is not a global user permission - assertEquals(expectedNamespace, namespacePermission.getNamespace()); // Verify namespace is set - } - } catch (Throwable thw) { - throw new HBaseException(thw); + assertNotNull(namespacePermissions); + assertEquals(expectedAmount, namespacePermissions.size()); + for (UserPermission namespacePermission : namespacePermissions) { + assertFalse(namespacePermission.isGlobal()); // Verify it is not a global user permission + assertEquals(expectedNamespace, namespacePermission.getNamespace()); // Verify namespace is set } } @Test public void testTruncatePerms() throws Exception { - try { - List existingPerms = AccessControlClient.getUserPermissions( - systemUserConnection, TEST_TABLE.getNameAsString()); - assertTrue(existingPerms != null); - assertTrue(existingPerms.size() > 1); - TEST_UTIL.getAdmin().disableTable(TEST_TABLE); - TEST_UTIL.truncateTable(TEST_TABLE); - TEST_UTIL.waitTableAvailable(TEST_TABLE); - List perms = AccessControlClient.getUserPermissions( - systemUserConnection, TEST_TABLE.getNameAsString()); - assertTrue(perms != null); - assertEquals(existingPerms.size(), perms.size()); - } catch (Throwable e) { - throw new HBaseIOException(e); - } - } - - private PrivilegedAction> getPrivilegedAction(final String regex) { - return new PrivilegedAction>() { - @Override - public List run() { - try(Connection conn = ConnectionFactory.createConnection(conf)) { - return AccessControlClient.getUserPermissions(conn, regex); - } catch (Throwable e) { - LOG.error("error during call of AccessControlClient.getUserPermissions.", e); - return null; - } + List existingPerms = AccessControlClient.getUserPermissions( + systemUserConnection, TEST_TABLE.getNameAsString()); + assertNotNull(existingPerms); + assertTrue(existingPerms.size() > 1); + TEST_UTIL.getAdmin().disableTable(TEST_TABLE); + TEST_UTIL.truncateTable(TEST_TABLE); + TEST_UTIL.waitTableAvailable(TEST_TABLE); + List perms = AccessControlClient.getUserPermissions( + systemUserConnection, TEST_TABLE.getNameAsString()); + assertNotNull(perms); + assertEquals(existingPerms.size(), perms.size()); + } + + private PrivilegedExceptionAction> getPrivilegedAction(final String regex) { + return () -> { + try(Connection conn = ConnectionFactory.createConnection(conf)) { + return AccessControlClient.getUserPermissions(conn, regex); } }; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java index 6ca2ef8a50..a52353a553 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java @@ -251,12 +251,8 @@ public class TestAccessController3 extends SecureTestUtil { grantGlobal(TEST_UTIL, toGroupEntry(GROUP_WRITE), Permission.Action.WRITE); assertEquals(5, AccessControlLists.getTablePermissions(conf, TEST_TABLE).size()); - try { - assertEquals(5, AccessControlClient.getUserPermissions(systemUserConnection, + assertEquals(5, AccessControlClient.getUserPermissions(systemUserConnection, TEST_TABLE.toString()).size()); - } catch (Throwable e) { - LOG.error("error during call of AccessControlClient.getUserPermissions. ", e); - } } private static void cleanUp() throws Exception { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java index b0cabed46d..db6ecdd2c9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityLabelsWithDeletes.java @@ -292,6 +292,19 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes } } + private void doScan(final int expectedRowCount, final TableName tableName) + throws IOException, InterruptedException { + SUPERUSER.runAs((PrivilegedExceptionAction) () -> { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + ResultScanner scanner = table.getScanner(new Scan()); + Result[] next = scanner.next(3); + assertEquals(expectedRowCount, next.length); + } + return null; + }); + } + @Test public void testDeleteColumnsWithoutAndWithVisibilityLabels() throws Exception { TableName tableName = createTable(); @@ -304,43 +317,13 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes // without visibility d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); table.delete(d); - PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(1, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(1, tableName); d = new Delete(row1); // with visibility d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); table.delete(d); - scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(0, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(0, tableName); } } @@ -357,42 +340,12 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); table.delete(d); - PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(0, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(0, tableName); d = new Delete(row1); // without visibility d.addColumns(fam, qual, HConstants.LATEST_TIMESTAMP); table.delete(d); - scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(0, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(0, tableName); } } @@ -408,43 +361,13 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes // without visibility d.addFamily(fam); table.delete(d); - PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(1, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(1, tableName); d = new Delete(row1); // with visibility d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); d.addFamily(fam); table.delete(d); - scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(0, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(0, tableName); } } @@ -461,42 +384,12 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes // with visibility d.addFamily(fam); table.delete(d); - PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(0, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(0, tableName); d = new Delete(row1); // without visibility d.addFamily(fam); table.delete(d); - scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - assertEquals(0, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(0, tableName); } } @@ -512,45 +405,13 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes // without visibility d.addColumn(fam, qual); table.delete(d); - PrivilegedExceptionAction scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - // The delete would not be able to apply it because of visibility mismatch - Result[] next = scanner.next(3); - assertEquals(1, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(1, tableName); d = new Delete(row1); // with visibility d.setCellVisibility(new CellVisibility(CONFIDENTIAL)); d.addColumn(fam, qual); table.delete(d); - scanAction = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Scan s = new Scan(); - ResultScanner scanner = table.getScanner(s); - Result[] next = scanner.next(3); - // this will alone match - assertEquals(0, next.length); - } catch (Throwable t) { - throw new IOException(t); - } - return null; - } - }; - SUPERUSER.runAs(scanAction); + doScan(0, tableName); } } @@ -834,7 +695,7 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes s.setAuthorizations(new Authorizations(SECRET, PRIVATE, CONFIDENTIAL, TOPSECRET)); ResultScanner scanner = table.getScanner(s); Result[] next = scanner.next(3); - assertTrue(next.length == 2); + assertEquals(2, next.length); CellScanner cellScanner = next[0].cellScanner(); cellScanner.advance(); Cell current = cellScanner.current(); @@ -2812,8 +2673,6 @@ public class TestVisibilityLabelsWithDeletes extends VisibilityLabelsWithDeletes d.addColumn(fam, qual, 124L); d.setCellVisibility(new CellVisibility(PRIVATE)); table.delete(d); - } catch (Throwable t) { - throw new IOException(t); } return null; } -- 2.16.1