From 969ff653c15f9428b374c1f6bb8be8df1da5ebf5 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 2 Nov 2022 20:41:01 +0100 Subject: [PATCH] Clean up InMemoryDataStoreTest Add proper assertions and use Builders instead of direct reference to immutable container builder. Change-Id: I07d725d9da13c75ad152e618a9bf1bc761167309 Signed-off-by: Robert Varga --- dom/mdsal-dom-inmemory-datastore/pom.xml | 8 + .../store/inmemory/InMemoryDataStoreTest.java | 173 ++++++++---------- 2 files changed, 86 insertions(+), 95 deletions(-) diff --git a/dom/mdsal-dom-inmemory-datastore/pom.xml b/dom/mdsal-dom-inmemory-datastore/pom.xml index 6386248c4b..eebc258982 100644 --- a/dom/mdsal-dom-inmemory-datastore/pom.xml +++ b/dom/mdsal-dom-inmemory-datastore/pom.xml @@ -58,6 +58,14 @@ org.opendaylight.yangtools yang-data-impl + + org.opendaylight.yangtools + yang-data-tree-api + + + org.opendaylight.yangtools + yang-data-tree-spi + org.opendaylight.yangtools yang-data-tree-ri diff --git a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDataStoreTest.java b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDataStoreTest.java index 9e6b7f2264..ac94386227 100644 --- a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDataStoreTest.java +++ b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDataStoreTest.java @@ -7,22 +7,28 @@ */ package org.opendaylight.mdsal.dom.store.inmemory; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; import java.util.Optional; import java.util.concurrent.ExecutionException; import org.junit.AfterClass; -import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; -import org.mockito.Mockito; import org.opendaylight.mdsal.common.api.ReadFailedException; import org.opendaylight.mdsal.dom.spi.store.DOMStoreReadTransaction; import org.opendaylight.mdsal.dom.spi.store.DOMStoreReadWriteTransaction; @@ -35,8 +41,8 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.impl.schema.Builders; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; -import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder; import org.opendaylight.yangtools.yang.data.tree.api.DataTreeModification; import org.opendaylight.yangtools.yang.data.tree.api.DataTreeSnapshot; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; @@ -63,10 +69,7 @@ public class InMemoryDataStoreTest { } @Test - public void testTransactionIsolation() throws InterruptedException, ExecutionException { - - assertNotNull(domStore); - + public void testTransactionIsolation() throws Exception { DOMStoreReadTransaction readTx = domStore.newReadOnlyTransaction(); assertNotNull(readTx); @@ -94,7 +97,7 @@ public class InMemoryDataStoreTest { } @Test - public void testTransactionCommit() throws InterruptedException, ExecutionException { + public void testTransactionCommit() throws Exception { DOMStoreReadWriteTransaction writeTx = domStore.newReadWriteTransaction(); assertNotNull(writeTx); @@ -154,7 +157,7 @@ public class InMemoryDataStoreTest { DOMStoreWriteTransaction writeTx = domStore.newWriteOnlyTransaction(); assertNotNull(writeTx); - ContainerNode containerNode = ImmutableContainerNodeBuilder.create() + ContainerNode containerNode = Builders.containerBuilder() .withNodeIdentifier(new NodeIdentifier(TestModel.TEST_QNAME)) .addChild(ImmutableNodes.mapNodeBuilder(TestModel.OUTER_LIST_QNAME) .addChild(ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, @@ -174,13 +177,13 @@ public class InMemoryDataStoreTest { writeTx = domStore.newWriteOnlyTransaction(); assertNotNull(writeTx); - containerNode = ImmutableContainerNodeBuilder.create() - .withNodeIdentifier(new NodeIdentifier(TestModel.TEST_QNAME)) - .addChild(ImmutableNodes.mapNodeBuilder(TestModel.OUTER_LIST_QNAME) - .addChild(ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, - TestModel.ID_QNAME, 1)) - .addChild(ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, - TestModel.ID_QNAME, 2)).build()).build(); + containerNode = Builders.containerBuilder() + .withNodeIdentifier(new NodeIdentifier(TestModel.TEST_QNAME)) + .addChild(ImmutableNodes.mapNodeBuilder(TestModel.OUTER_LIST_QNAME) + .addChild(ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, TestModel.ID_QNAME, 1)) + .addChild(ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, TestModel.ID_QNAME, 2)) + .build()) + .build(); writeTx.merge(TestModel.TEST_PATH, containerNode); @@ -191,14 +194,13 @@ public class InMemoryDataStoreTest { assertEquals("After commit read: data", containerNode, afterCommitRead.get()); } - @Test public void testExistsForExistingData() throws Exception { DOMStoreReadWriteTransaction writeTx = domStore.newReadWriteTransaction(); assertNotNull(writeTx); - ContainerNode containerNode = ImmutableContainerNodeBuilder.create() + ContainerNode containerNode = Builders.containerBuilder() .withNodeIdentifier(new NodeIdentifier(TestModel.TEST_QNAME)) .addChild(ImmutableNodes.mapNodeBuilder(TestModel.OUTER_LIST_QNAME) .addChild(ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, @@ -219,8 +221,7 @@ public class InMemoryDataStoreTest { DOMStoreReadTransaction readTx = domStore.newReadOnlyTransaction(); assertNotNull(readTx); - exists = - readTx.exists(TestModel.TEST_PATH); + exists = readTx.exists(TestModel.TEST_PATH); assertEquals(Boolean.TRUE, exists.get()); } @@ -231,140 +232,123 @@ public class InMemoryDataStoreTest { DOMStoreReadWriteTransaction writeTx = domStore.newReadWriteTransaction(); assertNotNull(writeTx); - ListenableFuture exists = writeTx.exists(TestModel.TEST_PATH); + var exists = writeTx.exists(TestModel.TEST_PATH); assertEquals(Boolean.FALSE, exists.get()); - DOMStoreReadTransaction readTx = domStore.newReadOnlyTransaction(); + var readTx = domStore.newReadOnlyTransaction(); assertNotNull(readTx); - exists = - readTx.exists(TestModel.TEST_PATH); + exists = readTx.exists(TestModel.TEST_PATH); assertEquals(Boolean.FALSE, exists.get()); } - @Test(expected = ReadFailedException.class) - @SuppressWarnings({"checkstyle:IllegalThrows", "checkstyle:AvoidHidingCauseException"}) - public void testExistsThrowsReadFailedException() throws Throwable { - - DOMStoreReadTransaction readTx = domStore.newReadOnlyTransaction(); + @Test + public void testExistsThrowsReadFailedException() { + var readTx = domStore.newReadOnlyTransaction(); assertNotNull(readTx); readTx.close(); - try { - readTx.exists(TestModel.TEST_PATH).get(); - } catch (ExecutionException e) { - throw e.getCause(); - } - } + final var future = readTx.exists(TestModel.TEST_PATH); + final var ex = assertThrows(ExecutionException.class, future::get).getCause(); + assertThat(ex, instanceOf(ReadFailedException.class)); + } - @SuppressWarnings("checkstyle:IllegalThrows") - @Test(expected = ReadFailedException.class) - public void testReadWithReadOnlyTransactionClosed() throws Throwable { + @Test + public void testReadWithReadOnlyTransactionClosed() { DOMStoreReadTransaction readTx = domStore.newReadOnlyTransaction(); assertNotNull(readTx); readTx.close(); - doReadAndThrowEx(readTx); + assertReadThrows(readTx); } - @SuppressWarnings("checkstyle:IllegalThrows") - @Test(expected = ReadFailedException.class) - public void testReadWithReadOnlyTransactionFailure() throws Throwable { - - DataTreeSnapshot mockSnapshot = Mockito.mock(DataTreeSnapshot.class); - Mockito.doThrow(new RuntimeException("mock ex")).when(mockSnapshot) - .readNode(Mockito.any(YangInstanceIdentifier.class)); + @Test + public void testReadWithReadOnlyTransactionFailure() { + DataTreeSnapshot mockSnapshot = mock(DataTreeSnapshot.class); + doThrow(new RuntimeException("mock ex")).when(mockSnapshot) + .readNode(any(YangInstanceIdentifier.class)); DOMStoreReadTransaction readTx = SnapshotBackedTransactions.newReadTransaction("1", true, mockSnapshot); - doReadAndThrowEx(readTx); + assertReadThrows(readTx); } - @SuppressWarnings("checkstyle:IllegalThrows") - @Test(expected = ReadFailedException.class) - public void testReadWithReadWriteTransactionClosed() throws Throwable { + @Test + public void testReadWithReadWriteTransactionClosed() { DOMStoreReadTransaction readTx = domStore.newReadWriteTransaction(); assertNotNull(readTx); readTx.close(); - doReadAndThrowEx(readTx); + assertReadThrows(readTx); } - @SuppressWarnings("checkstyle:IllegalThrows") - @Test(expected = ReadFailedException.class) - public void testReadWithReadWriteTransactionFailure() throws Throwable { - - DataTreeSnapshot mockSnapshot = Mockito.mock(DataTreeSnapshot.class); - DataTreeModification mockModification = Mockito.mock(DataTreeModification.class); - Mockito.doThrow(new RuntimeException("mock ex")).when(mockModification) - .readNode(Mockito.any(YangInstanceIdentifier.class)); - Mockito.doReturn(mockModification).when(mockSnapshot).newModification(); + @Test + public void testReadWithReadWriteTransactionFailure() { + DataTreeSnapshot mockSnapshot = mock(DataTreeSnapshot.class); + DataTreeModification mockModification = mock(DataTreeModification.class); + doThrow(new RuntimeException("mock ex")).when(mockModification) + .readNode(any(YangInstanceIdentifier.class)); + doReturn(mockModification).when(mockSnapshot).newModification(); @SuppressWarnings("unchecked") - TransactionReadyPrototype mockReady = Mockito.mock(TransactionReadyPrototype.class); + TransactionReadyPrototype mockReady = mock(TransactionReadyPrototype.class); DOMStoreReadTransaction readTx = SnapshotBackedTransactions.newReadWriteTransaction( "1", false, mockSnapshot, mockReady); - doReadAndThrowEx(readTx); + assertReadThrows(readTx); } - @SuppressWarnings({ "checkstyle:IllegalThrows", "checkstyle:avoidHidingCauseException" }) - private static void doReadAndThrowEx(final DOMStoreReadTransaction readTx) throws Throwable { - try { - readTx.read(TestModel.TEST_PATH).get(); - } catch (ExecutionException e) { - throw e.getCause(); - } + private static void assertReadThrows(final DOMStoreReadTransaction readTx) { + final var future = readTx.read(TestModel.TEST_PATH); + final var cause = assertThrows(ExecutionException.class, future::get).getCause(); + assertThat(cause, instanceOf(ReadFailedException.class)); } - @Test(expected = IllegalStateException.class) - public void testWriteWithTransactionReady() throws Exception { - - DOMStoreWriteTransaction writeTx = domStore.newWriteOnlyTransaction(); - + @Test + public void testWriteWithTransactionReady() { + var writeTx = domStore.newWriteOnlyTransaction(); writeTx.ready(); // Should throw ex - writeTx.write(TestModel.TEST_PATH, ImmutableNodes.containerNode(TestModel.TEST_QNAME)); + assertThrows(IllegalStateException.class, + () -> writeTx.write(TestModel.TEST_PATH, ImmutableNodes.containerNode(TestModel.TEST_QNAME))); } - @Test(expected = IllegalStateException.class) - public void testReadyWithTransactionAlreadyReady() throws Exception { - - DOMStoreWriteTransaction writeTx = domStore.newWriteOnlyTransaction(); + @Test + public void testReadyWithTransactionAlreadyReady() { + var writeTx = domStore.newWriteOnlyTransaction(); writeTx.ready(); // Should throw ex - writeTx.ready(); + assertThrows(IllegalStateException.class, writeTx::ready); } @Test - public void testReadyWithMissingMandatoryData() throws InterruptedException { + public void testReadyWithMissingMandatoryData() throws Exception { DOMStoreWriteTransaction writeTx = domStore.newWriteOnlyTransaction(); - NormalizedNode testNode = ImmutableContainerNodeBuilder.create() + NormalizedNode testNode = Builders.containerBuilder() .withNodeIdentifier(new NodeIdentifier(TestModel.MANDATORY_DATA_TEST_QNAME)) .addChild(ImmutableNodes.leafNode(TestModel.OPTIONAL_QNAME, "data")) .build(); writeTx.write(TestModel.MANDATORY_DATA_TEST_PATH, testNode); DOMStoreThreePhaseCommitCohort ready = writeTx.ready(); - try { - ready.canCommit().get(); - Assert.fail("Expected exception on canCommit"); - } catch (ExecutionException e) { - // nop - } + + final var future = ready.canCommit(); + final var cause = assertThrows(ExecutionException.class, future::get).getCause(); + assertThat(cause, instanceOf(IllegalArgumentException.class)); + assertThat(cause.getMessage(), containsString("mandatory-data-test is missing mandatory descendant")); } @Test - public void testTransactionAbort() throws InterruptedException, ExecutionException { + public void testTransactionAbort() throws Exception { DOMStoreReadWriteTransaction writeTx = domStore.newReadWriteTransaction(); assertNotNull(writeTx); @@ -381,7 +365,7 @@ public class InMemoryDataStoreTest { } @Test - public void testTransactionChain() throws InterruptedException, ExecutionException { + public void testTransactionChain() throws Exception { DOMStoreTransactionChain txChain = domStore.createTransactionChain(); assertNotNull(txChain); @@ -453,7 +437,7 @@ public class InMemoryDataStoreTest { @Test @Ignore - public void testTransactionConflict() throws InterruptedException, ExecutionException { + public void testTransactionConflict() throws Exception { DOMStoreReadWriteTransaction txOne = domStore.newReadWriteTransaction(); DOMStoreReadWriteTransaction txTwo = domStore.newReadWriteTransaction(); assertTestContainerWrite(txOne); @@ -470,15 +454,14 @@ public class InMemoryDataStoreTest { assertFalse(txTwo.ready().canCommit().get()); } - private static void assertThreePhaseCommit(final DOMStoreThreePhaseCommitCohort cohort) - throws InterruptedException, ExecutionException { + private static void assertThreePhaseCommit(final DOMStoreThreePhaseCommitCohort cohort) throws Exception { assertTrue(cohort.canCommit().get()); cohort.preCommit().get(); cohort.commit().get(); } private static Optional assertTestContainerWrite(final DOMStoreReadWriteTransaction writeTx) - throws InterruptedException, ExecutionException { + throws Exception { /** * * Writes /test in writeTx @@ -493,7 +476,7 @@ public class InMemoryDataStoreTest { * Reads /test from readTx Read should return container. */ private static Optional assertTestContainerExists(final DOMStoreReadTransaction readTx) - throws InterruptedException, ExecutionException { + throws Exception { ListenableFuture> writeTxContainer = readTx.read(TestModel.TEST_PATH); assertTrue(writeTxContainer.get().isPresent()); -- 2.36.6