From 7ee8a44f7dc06f8ff7dda28ff60fa77377072ae3 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 Upgrade error-prone version to 2.3.x for detecting try-catch-rethrow --- hbase-build-configuration/pom.xml | 1 + .../hbase/security/access/AccessControlClient.java | 10 +- .../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 | 114 ++++-------- .../hbase/procedure2/TestProcedureNonce.java | 108 ++++++------ .../hadoop/hbase/rsgroup/TestRSGroupsWithACL.java | 12 +- .../hadoop/hbase/client/TestFromClientSide.java | 1 + .../hbase/client/TestMultipleTimestamps.java | 1 - .../TestMasterProcedureSchedulerConcurrency.java | 2 + .../master/procedure/TestServerCrashProcedure.java | 10 +- .../procedure/TestTruncateTableProcedure.java | 34 +--- .../hbase/master/snapshot/TestAssignProcedure.java | 21 +-- .../regionserver/TestRegionReplicaFailover.java | 31 ++-- .../hbase/regionserver/TestServerNonceManager.java | 1 + .../security/access/TestAccessController.java | 68 +++----- .../security/access/TestAccessController2.java | 9 +- .../security/access/TestAccessController3.java | 14 +- .../TestVisibilityLabelsWithDeletes.java | 191 +++------------------ .../visibility/TestVisibilityWithCheckAuths.java | 45 +++-- .../org/apache/hadoop/hbase/wal/TestWALSplit.java | 15 +- pom.xml | 4 +- 24 files changed, 257 insertions(+), 508 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 981db767c8..d410db4f77 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 @@ -39,6 +39,8 @@ import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService.BlockingInterface; import org.apache.hadoop.hbase.util.Bytes; +import com.google.protobuf.ServiceException; + /** * Utility client for doing access control admin operations. */ @@ -257,10 +259,10 @@ public class AccessControlClient { * @param connection The Connection instance to use * @param tableRegex The regular expression string to match against * @return List of UserPermissions - * @throws Throwable + * @throws Exception */ public static List getUserPermissions(Connection connection, String tableRegex) - throws Throwable { + throws Exception { return getUserPermissions(connection, tableRegex, HConstants.EMPTY_STRING); } @@ -270,10 +272,10 @@ public class AccessControlClient { * @param tableRegex The regular expression string to match against * @param userName User name, if empty then all user permissions will be retrieved. * @return List of UserPermissions - * @throws Throwable on failure + * @throws Exception on failure */ public static List getUserPermissions(Connection connection, String tableRegex, - String userName) throws Throwable { + String userName) throws Exception { /** * 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..f0f29b2a7f 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; @@ -48,8 +49,6 @@ import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.common.base.Throwables; - /** * Basic test for the SyncTable M/R tool */ @@ -181,38 +180,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 +200,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 +269,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 +356,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..dc29c0e192 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 @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.procedure2; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -164,17 +165,18 @@ public class TestProcedureNonce { } @Test - public void testConcurrentNonceRegistration() throws IOException { + public void testConcurrentNonceRegistration() { testConcurrentNonceRegistration(true, 567, 44444); } @Test - public void testConcurrentNonceRegistrationWithRollback() throws IOException { + public void testConcurrentNonceRegistrationWithRollback() { 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) { // register the nonce final NonceKey nonceKey = procExecutor.createNonceKey(nonceGroup, nonce); @@ -184,66 +186,60 @@ 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()); + assertNull(t1Exception.get()); + assertNull(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..6e6b4c5901 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 @@ -20,7 +20,6 @@ package org.apache.hadoop.hbase.rsgroup; import static org.apache.hadoop.hbase.AuthUtil.toGroupEntry; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -173,14 +172,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/client/TestMultipleTimestamps.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java index ac6b9d12c1..116aa9bf75 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java @@ -448,7 +448,6 @@ public class TestMultipleTimestamps { Integer[] rowIndexes, Integer[] columnIndexes, Long[] versions, int maxVersions) throws IOException { - Arrays.asList(rowIndexes); byte startRow[] = Bytes.toBytes("row:" + Collections.min( Arrays.asList(rowIndexes))); byte endRow[] = Bytes.toBytes("row:" + diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java index 1313cdba85..9f8bec2726 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureSchedulerConcurrency.java @@ -87,6 +87,7 @@ public class TestMasterProcedureSchedulerConcurrency { AtomicInteger concurrentCount = new AtomicInteger(0); for (int i = 0; i < threads.length; ++i) { threads[i] = new Thread() { + @SuppressWarnings("AssertionFailureIgnored") @Override public void run() { while (opsCount.get() > 0) { @@ -171,6 +172,7 @@ public class TestMasterProcedureSchedulerConcurrency { final AtomicInteger concurrentCount = new AtomicInteger(0); for (int i = 0; i < threads.length; ++i) { threads[i] = new Thread() { + @SuppressWarnings("AssertionFailureIgnored") @Override public void run() { while (opsCount.get() > 0) { 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..acc261f5ef 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,11 +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(); } } 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..00ffd11482 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 @@ -18,7 +18,6 @@ package org.apache.hadoop.hbase.master.procedure; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.IOException; @@ -38,7 +37,6 @@ import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.master.MasterFileSystem; -import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; import org.apache.hadoop.hbase.testclassification.MasterTests; @@ -73,21 +71,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 +88,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/master/snapshot/TestAssignProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java index 0e8f97e206..47f8399c60 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/snapshot/TestAssignProcedure.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.hbase.master.snapshot; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -197,19 +198,19 @@ public class TestAssignProcedure { Collections.shuffle(procedures); procedures.sort(AssignProcedure.COMPARATOR); try { - assertTrue(procedures.get(0).getRegionInfo().equals(RegionInfoBuilder.FIRST_META_REGIONINFO)); - assertTrue(procedures.get(1).getRegionInfo().equals(meta0)); - assertTrue(procedures.get(2).getRegionInfo().equals(meta1)); - assertTrue(procedures.get(3).getRegionInfo().equals(meta2)); - assertTrue(procedures.get(4).getRegionInfo().getTable().equals(TableName.NAMESPACE_TABLE_NAME)); - assertTrue(procedures.get(5).getRegionInfo().equals(user1)); - assertTrue(procedures.get(6).getRegionInfo().equals(user2)); - assertTrue(procedures.get(7).getRegionInfo().equals(user3)); - } catch (Throwable t) { + assertEquals(RegionInfoBuilder.FIRST_META_REGIONINFO, procedures.get(0).getRegionInfo()); + assertEquals(meta0, procedures.get(1).getRegionInfo()); + assertEquals(meta1, procedures.get(2).getRegionInfo()); + assertEquals(meta2, procedures.get(3).getRegionInfo()); + assertEquals(TableName.NAMESPACE_TABLE_NAME, procedures.get(4).getRegionInfo().getTable()); + assertEquals(user1, procedures.get(5).getRegionInfo()); + assertEquals(user2, procedures.get(6).getRegionInfo()); + assertEquals(user3, procedures.get(7).getRegionInfo()); + } catch (AssertionError e) { for (AssignProcedure proc : procedures) { LOG.debug(Objects.toString(proc)); } - throw t; + throw e; } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaFailover.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaFailover.java index 5cf19795c2..6fb86bfeab 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaFailover.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaFailover.java @@ -279,29 +279,22 @@ public class TestRegionReplicaFailover { }; loader.start(); - Thread aborter = new Thread() { - @Override - public void run() { - try { - boolean aborted = false; - for (RegionServerThread rs : HTU.getMiniHBaseCluster().getRegionServerThreads()) { - for (Region r : rs.getRegionServer().getRegions(htd.getTableName())) { - if (r.getRegionInfo().getReplicaId() == 1) { - LOG.info("Aborting region server hosting secondary region replica"); - rs.getRegionServer().abort("for test"); - aborted = true; - } - } + try { + boolean aborted = false; + for (RegionServerThread rs : HTU.getMiniHBaseCluster().getRegionServerThreads()) { + for (Region r : rs.getRegionServer().getRegions(htd.getTableName())) { + if (r.getRegionInfo().getReplicaId() == 1) { + LOG.info("Aborting region server hosting secondary region replica"); + rs.getRegionServer().abort("for test"); + aborted = true; } - assertTrue(aborted); - } catch (Throwable e) { - ex.compareAndSet(null, e); } } - }; + assertTrue(aborted); + } finally { + done.set(true); + } - aborter.start(); - aborter.join(); done.set(true); loader.join(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestServerNonceManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestServerNonceManager.java index e2525db73f..ef2f5474f9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestServerNonceManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestServerNonceManager.java @@ -268,6 +268,7 @@ public class TestServerNonceManager { return t; } + @SuppressWarnings("AssertionFailureIgnored") @Override public void run() { startedLatch.countDown(); 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 a0b5d9d34d..cb66f74cea 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 @@ -31,7 +31,7 @@ import com.google.protobuf.RpcController; 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; @@ -46,7 +46,6 @@ import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.HBaseClassTestRule; -import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; @@ -92,7 +91,6 @@ import org.apache.hadoop.hbase.coprocessor.protobuf.generated.PingProtos.NoopRes import org.apache.hadoop.hbase.coprocessor.protobuf.generated.PingProtos.PingRequest; import org.apache.hadoop.hbase.coprocessor.protobuf.generated.PingProtos.PingResponse; import org.apache.hadoop.hbase.coprocessor.protobuf.generated.PingProtos.PingService; -import org.apache.hadoop.hbase.exceptions.HBaseException; import org.apache.hadoop.hbase.io.hfile.CacheConfig; import org.apache.hadoop.hbase.io.hfile.HFile; import org.apache.hadoop.hbase.io.hfile.HFileContext; @@ -2701,53 +2699,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("Should not be global user permission", namespacePermission.isGlobal()); + assertEquals("Namespace was not set", expectedNamespace, namespacePermission.getNamespace()); } } @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/TestAccessController2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController2.java index 21c1438e68..a4ec495cc4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController2.java @@ -147,13 +147,8 @@ public class TestAccessController2 extends SecureTestUtil { } assertEquals(1, AccessControlLists.getTablePermissions(conf, tableName).size()); - try { - assertEquals(1, AccessControlClient.getUserPermissions(systemUserConnection, - tableName.toString()).size()); - } catch (Throwable e) { - LOG.error("Error during call of AccessControlClient.getUserPermissions. ", e); - } - + assertEquals(1, AccessControlClient.getUserPermissions(systemUserConnection, + tableName.toString()).size()); } @AfterClass 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..2fa313a959 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, - TEST_TABLE.toString()).size()); - } catch (Throwable e) { - LOG.error("error during call of AccessControlClient.getUserPermissions. ", e); - } + assertEquals(5, AccessControlClient.getUserPermissions(systemUserConnection, + TEST_TABLE.toString()).size()); } private static void cleanUp() throws Exception { @@ -272,10 +268,8 @@ public class TestAccessController3 extends SecureTestUtil { } // Verify all table/namespace permissions are erased assertEquals(0, AccessControlLists.getTablePermissions(conf, TEST_TABLE).size()); - assertEquals( - 0, - AccessControlLists.getNamespacePermissions(conf, - TEST_TABLE.getNamespaceAsString()).size()); + assertEquals(0, + AccessControlLists.getNamespacePermissions(conf, TEST_TABLE.getNamespaceAsString()).size()); } @Test 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..ac85df4f45 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,15 @@ 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); + // The delete would not be able to apply it because of visibility mismatch + 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); + // this will alone match + doScan(0, tableName); } } @@ -834,7 +697,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 +2675,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; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityWithCheckAuths.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityWithCheckAuths.java index 153a892f11..7a2d739b90 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityWithCheckAuths.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/visibility/TestVisibilityWithCheckAuths.java @@ -36,6 +36,7 @@ import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.protobuf.generated.VisibilityLabelsProtos.VisibilityLabelsResponse; +import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.SecurityTests; @@ -133,28 +134,24 @@ public class TestVisibilityWithCheckAuths { HTableDescriptor desc = new HTableDescriptor(tableName); desc.addFamily(colDesc); hBaseAdmin.createTable(desc); - try { - TEST_UTIL.getAdmin().flush(tableName); - PrivilegedExceptionAction actiona = new PrivilegedExceptionAction() { - @Override - public Void run() throws Exception { - try (Connection connection = ConnectionFactory.createConnection(conf); - Table table = connection.getTable(tableName)) { - Put p = new Put(row1); - p.setCellVisibility(new CellVisibility(PUBLIC + "&" + TOPSECRET)); - p.addColumn(fam, qual, 125L, value); - table.put(p); - Assert.fail("Testcase should fail with AccesDeniedException"); - } catch (Throwable t) { - assertTrue(t.getMessage().contains("AccessDeniedException")); - } - return null; + TEST_UTIL.getAdmin().flush(tableName); + PrivilegedExceptionAction actiona = new PrivilegedExceptionAction() { + @Override + public Void run() throws Exception { + try (Connection connection = ConnectionFactory.createConnection(conf); + Table table = connection.getTable(tableName)) { + Put p = new Put(row1); + p.setCellVisibility(new CellVisibility(PUBLIC + "&" + TOPSECRET)); + p.addColumn(fam, qual, 125L, value); + table.put(p); + Assert.fail("Expected AccessDeniedException"); + } catch (AccessDeniedException e) { + // expected } - }; - USER.runAs(actiona); - } catch (Exception e) { - throw new IOException(e); - } + return null; + } + }; + USER.runAs(actiona); } @Test @@ -212,9 +209,9 @@ public class TestVisibilityWithCheckAuths { append.addColumn(fam, qual, Bytes.toBytes("c")); append.setCellVisibility(new CellVisibility(PUBLIC)); table.append(append); - Assert.fail("Testcase should fail with AccesDeniedException"); - } catch (Throwable t) { - assertTrue(t.getMessage().contains("AccessDeniedException")); + Assert.fail("Expected AccessDeniedException"); + } catch (AccessDeniedException e) { + // expected } return null; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALSplit.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALSplit.java index 0d5aa0d2ff..39224450d8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALSplit.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/wal/TestWALSplit.java @@ -110,13 +110,14 @@ public class TestWALSplit { public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestWALSplit.class); - { - // Uncomment the following lines if more verbosity is needed for - // debugging (see HBASE-12285 for details). - //((Log4JLogger)DataNode.LOG).getLogger().setLevel(Level.ALL); - //((Log4JLogger)LeaseManager.LOG).getLogger().setLevel(Level.ALL); - //((Log4JLogger)FSNamesystem.LOG).getLogger().setLevel(Level.ALL); - } + // Uncomment the following lines if more verbosity is needed for + // debugging (see HBASE-12285 for details). + // { + // ((Log4JLogger)DataNode.LOG).getLogger().setLevel(Level.ALL); + // ((Log4JLogger)LeaseManager.LOG).getLogger().setLevel(Level.ALL); + // ((Log4JLogger)FSNamesystem.LOG).getLogger().setLevel(Level.ALL); + // } + private final static Logger LOG = LoggerFactory.getLogger(TestWALSplit.class); private static Configuration conf; diff --git a/pom.xml b/pom.xml index f65091d404..a34c24dc08 100755 --- a/pom.xml +++ b/pom.xml @@ -1516,7 +1516,9 @@ 1.4 8.2 1.6.0 - 2.2.0 + + 2.3.2-SNAPSHOT 1.3.9-1 3.0.4 2.4.2 -- 2.16.1