From 037bbf70c524ae03290ec27a04dbde8065d6eb77 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 28 May 2023 20:59:24 +0200 Subject: [PATCH] Perform partial modification matching Now that we have a collector, we can assert a particular modification state at any given moment. Change-Id: I9f9f3436b0def8d3ddd8612343c41c4bfdd2c017 Signed-off-by: Robert Varga --- .../dom/adapter/Bug1125RegressionTest.java | 6 +-- .../Bug1333DataChangeListenerTest.java | 19 +++++--- .../dom/adapter/Bug1418AugmentationTest.java | 26 +++++++---- .../Bug2562DeserializedUnkeyedListTest.java | 4 +- .../dom/adapter/Bug3090MultiKeyList.java | 4 +- .../ListInsertionDataChangeListenerTest.java | 32 +++++++------- .../AbstractDataTreeChangeListenerTest.java | 44 ++++++++++++------- 7 files changed, 83 insertions(+), 52 deletions(-) diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1125RegressionTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1125RegressionTest.java index cbe44d5c53..4a77244baf 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1125RegressionTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1125RegressionTest.java @@ -59,13 +59,13 @@ public class Bug1125RegressionTest extends AbstractDataTreeChangeListenerTest { private void deleteAndListenAugment(final InstanceIdentifier path) { final var augment = writeInitialState(); try (var collector = createCollector(LogicalDatastoreType.OPERATIONAL, WILDCARDED_AUGMENT_PATH)) { + collector.verifyModifications(added(FOO_AUGMENT_PATH, augment)); + final var tx = getDataBroker().newWriteOnlyTransaction(); tx.delete(LogicalDatastoreType.OPERATIONAL, path); assertCommit(tx.commit()); - collector.assertModifications( - added(FOO_AUGMENT_PATH, augment), - deleted(FOO_AUGMENT_PATH, augment)); + collector.verifyModifications(deleted(FOO_AUGMENT_PATH, augment)); } } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1333DataChangeListenerTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1333DataChangeListenerTest.java index 9328e8cb5b..50bfca6606 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1333DataChangeListenerTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1333DataChangeListenerTest.java @@ -66,18 +66,22 @@ public class Bug1333DataChangeListenerTest extends AbstractDataTreeChangeListene @Test public void writeTopWithListItemAugmentedListenTopSubtree() { try (var collector = createCollector(LogicalDatastoreType.CONFIGURATION, TOP_PATH)) { + collector.verifyModifications(); + writeTopWithListItem(LogicalDatastoreType.CONFIGURATION); - collector.assertModifications(added(TOP_PATH, topWithListItem())); + collector.verifyModifications(added(TOP_PATH, topWithListItem())); } } @Test public void writeTopWithListItemAugmentedListenAugmentSubtreeWildcarded() { try (var collector = createCollector(LogicalDatastoreType.CONFIGURATION, AUGMENT_WILDCARD)) { + collector.verifyModifications(); + writeTopWithListItem(LogicalDatastoreType.CONFIGURATION); - collector.assertModifications( + collector.verifyModifications( added(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugment(USES_ONE_KEY, USES_TWO_KEY))); } } @@ -87,10 +91,11 @@ public class Bug1333DataChangeListenerTest extends AbstractDataTreeChangeListene final var top = writeTopWithListItem(LogicalDatastoreType.CONFIGURATION); try (var collector = createCollector(LogicalDatastoreType.CONFIGURATION, TOP_PATH)) { + collector.verifyModifications(added(TOP_PATH, top)); + deleteItem(LogicalDatastoreType.CONFIGURATION, path(TOP_FOO_KEY, USES_ONE_KEY)); - collector.assertModifications( - added(TOP_PATH, top), + collector.verifyModifications( subtreeModified(TOP_PATH, top, top(topLevelList(TOP_FOO_KEY, complexUsesAugment(USES_TWO_KEY))))); } } @@ -100,10 +105,12 @@ public class Bug1333DataChangeListenerTest extends AbstractDataTreeChangeListene writeTopWithListItem(LogicalDatastoreType.CONFIGURATION); try (var collector = createCollector(LogicalDatastoreType.CONFIGURATION, AUGMENT_WILDCARD)) { + collector.verifyModifications( + added(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugment(USES_ONE_KEY, USES_TWO_KEY))); + deleteItem(LogicalDatastoreType.CONFIGURATION, path(TOP_FOO_KEY, USES_ONE_KEY)); - collector.assertModifications( - added(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugment(USES_ONE_KEY, USES_TWO_KEY)), + collector.verifyModifications( subtreeModified(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugment(USES_ONE_KEY, USES_TWO_KEY), complexUsesAugment(USES_TWO_KEY))); } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1418AugmentationTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1418AugmentationTest.java index b413993a7d..eb4c0c637b 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1418AugmentationTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug1418AugmentationTest.java @@ -50,13 +50,16 @@ public class Bug1418AugmentationTest extends AbstractDataTreeChangeListenerTest public void leafOnlyAugmentationCreatedTest() { final var leafOnlyUsesAugment = leafOnlyUsesAugment("test leaf"); try (var collector = createCollector(CONFIGURATION, SIMPLE_AUGMENT)) { + collector.verifyModifications(); + final var writeTx = getDataBroker().newWriteOnlyTransaction(); writeTx.put(CONFIGURATION, TOP, top()); writeTx.put(CONFIGURATION, TOP_FOO, topLevelList(new TopLevelListKey(TOP_FOO_KEY))); writeTx.put(CONFIGURATION, SIMPLE_AUGMENT, leafOnlyUsesAugment); assertCommit(writeTx.commit()); - collector.assertModifications(added(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugment)); + collector.verifyModifications( + added(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugment)); } } @@ -71,12 +74,14 @@ public class Bug1418AugmentationTest extends AbstractDataTreeChangeListenerTest final var leafOnlyUsesAugmentAfter = leafOnlyUsesAugment("test leaf changed"); try (var collector = createCollector(CONFIGURATION, SIMPLE_AUGMENT)) { + collector.verifyModifications( + added(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugmentBefore)); + writeTx = getDataBroker().newWriteOnlyTransaction(); writeTx.put(CONFIGURATION, SIMPLE_AUGMENT, leafOnlyUsesAugmentAfter); assertCommit(writeTx.commit()); - collector.assertModifications( - added(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugmentBefore), + collector.verifyModifications( replaced(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugmentBefore, leafOnlyUsesAugmentAfter)); } @@ -92,12 +97,14 @@ public class Bug1418AugmentationTest extends AbstractDataTreeChangeListenerTest assertCommit(writeTx.commit()); try (var collector = createCollector(CONFIGURATION, SIMPLE_AUGMENT)) { + collector.verifyModifications( + added(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugment)); + writeTx = getDataBroker().newWriteOnlyTransaction(); writeTx.delete(CONFIGURATION, SIMPLE_AUGMENT); assertCommit(writeTx.commit()); - collector.assertModifications( - added(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugment), + collector.verifyModifications( deleted(path(TOP_FOO_KEY, TreeLeafOnlyUsesAugment.class), leafOnlyUsesAugment)); } } @@ -112,7 +119,8 @@ public class Bug1418AugmentationTest extends AbstractDataTreeChangeListenerTest writeTx.put(CONFIGURATION, COMPLEX_AUGMENT, complexUsesAugment); assertCommit(writeTx.commit()); - collector.assertModifications(added(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugment)); + collector.verifyModifications( + added(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugment)); } } @@ -126,13 +134,15 @@ public class Bug1418AugmentationTest extends AbstractDataTreeChangeListenerTest assertCommit(writeTx.commit()); try (var collector = createCollector(CONFIGURATION, COMPLEX_AUGMENT)) { + collector.verifyModifications( + added(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugmentBefore)); + final var complexUsesAugmentAfter = complexUsesAugment(LIST_VIA_USES_KEY_MOD); writeTx = getDataBroker().newWriteOnlyTransaction(); writeTx.put(CONFIGURATION, COMPLEX_AUGMENT, complexUsesAugmentAfter); assertCommit(writeTx.commit()); - collector.assertModifications( - added(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugmentBefore), + collector.verifyModifications( // While we are overwriting the augment, at the transaction ends up replacing one child with another, // so the Augmentation ends up not being overwritten, but modified subtreeModified(path(TOP_FOO_KEY, TreeComplexUsesAugment.class), complexUsesAugmentBefore, diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug2562DeserializedUnkeyedListTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug2562DeserializedUnkeyedListTest.java index 5ab6c8466e..4aed5155bd 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug2562DeserializedUnkeyedListTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug2562DeserializedUnkeyedListTest.java @@ -37,11 +37,13 @@ public class Bug2562DeserializedUnkeyedListTest extends AbstractDataTreeChangeLi final var root = new RootBuilder().setFooroot(List.of(fooRoot)).build(); try (var collector = createCollector(LogicalDatastoreType.CONFIGURATION, ROOT_PATH)) { + collector.verifyModifications(); + final var readWriteTransaction = getDataBroker().newReadWriteTransaction(); readWriteTransaction.put(LogicalDatastoreType.CONFIGURATION, ROOT_PATH, root); assertCommit(readWriteTransaction.commit()); - collector.assertModifications(added(ROOT_PATH, root)); + collector.verifyModifications(added(ROOT_PATH, root)); } } } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug3090MultiKeyList.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug3090MultiKeyList.java index c4b533ebef..2afd118b39 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug3090MultiKeyList.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/Bug3090MultiKeyList.java @@ -48,11 +48,13 @@ public class Bug3090MultiKeyList extends AbstractDataTreeChangeListenerTest { final var root = new RootBuilder().setListInRoot(BindingMap.of(listInRoots)).build(); try (var collector = createCollector(LogicalDatastoreType.CONFIGURATION, ROOT_PATH)) { + collector.verifyModifications(); + final var readWriteTransaction = getDataBroker().newReadWriteTransaction(); readWriteTransaction.put(LogicalDatastoreType.CONFIGURATION, ROOT_PATH, root); assertCommit(readWriteTransaction.commit()); - collector.assertModifications(match(ModificationType.WRITE, ROOT_PATH, Objects::isNull, + collector.verifyModifications(match(ModificationType.WRITE, ROOT_PATH, Objects::isNull, (DataMatcher) dataAfter -> checkData(root, dataAfter))); } } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ListInsertionDataChangeListenerTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ListInsertionDataChangeListenerTest.java index fb61d7a4cd..4a33fef41e 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ListInsertionDataChangeListenerTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ListInsertionDataChangeListenerTest.java @@ -70,16 +70,16 @@ public class ListInsertionDataChangeListenerTest extends AbstractDataTreeChangeL writeTx.put(CONFIGURATION, TOP, top); assertCommit(writeTx.commit()); - barListener.assertModifications(added(TOP_BAR, topBar)); + barListener.verifyModifications(added(TOP_BAR, topBar)); } - fooListener.assertModifications(added(TOP_FOO, topFoo), deleted(TOP_FOO, topFoo)); + fooListener.verifyModifications(added(TOP_FOO, topFoo), deleted(TOP_FOO, topFoo)); } - allListener.assertModifications( + allListener.verifyModifications( added(TOP_FOO, topFoo), added(TOP_BAR, topBar), deleted(TOP_FOO, topFoo)); } - topListener.assertModifications( + topListener.verifyModifications( added(TOP, top(topLevelList(TOP_FOO_KEY))), replaced(TOP, top(topFoo), top)); } @@ -98,13 +98,13 @@ public class ListInsertionDataChangeListenerTest extends AbstractDataTreeChangeL writeTx.merge(CONFIGURATION, TOP, top(topLevelList(TOP_BAR_KEY))); assertCommit(writeTx.commit()); - barListener.assertModifications(added(TOP_BAR, topBar)); + barListener.verifyModifications(added(TOP_BAR, topBar)); } - fooListener.assertModifications(added(TOP_FOO, topFoo)); + fooListener.verifyModifications(added(TOP_FOO, topFoo)); } - allListener.assertModifications(added(TOP_FOO, topFoo), added(TOP_BAR, topBar)); + allListener.verifyModifications(added(TOP_FOO, topFoo), added(TOP_BAR, topBar)); } - topListener.assertModifications( + topListener.verifyModifications( added(TOP, top(topLevelList(TOP_FOO_KEY))), topSubtreeModified(topFoo, topBar)); } } @@ -122,13 +122,13 @@ public class ListInsertionDataChangeListenerTest extends AbstractDataTreeChangeL writeTx.put(CONFIGURATION, TOP_BAR, topLevelList(TOP_BAR_KEY)); assertCommit(writeTx.commit()); - barListener.assertModifications(added(TOP_BAR, topBar)); + barListener.verifyModifications(added(TOP_BAR, topBar)); } - fooListener.assertModifications(added(TOP_FOO, topFoo)); + fooListener.verifyModifications(added(TOP_FOO, topFoo)); } - allListener.assertModifications(added(TOP_FOO, topFoo), added(TOP_BAR, topBar)); + allListener.verifyModifications(added(TOP_FOO, topFoo), added(TOP_BAR, topBar)); } - topListener.assertModifications( + topListener.verifyModifications( added(TOP, top(topLevelList(TOP_FOO_KEY))), topSubtreeModified(topFoo, topBar)); } } @@ -146,13 +146,13 @@ public class ListInsertionDataChangeListenerTest extends AbstractDataTreeChangeL writeTx.merge(CONFIGURATION, TOP_BAR, topLevelList(TOP_BAR_KEY)); assertCommit(writeTx.commit()); - barListener.assertModifications(added(TOP_BAR, topBar)); + barListener.verifyModifications(added(TOP_BAR, topBar)); } - fooListener.assertModifications(added(TOP_FOO, topFoo)); + fooListener.verifyModifications(added(TOP_FOO, topFoo)); } - allListener.assertModifications(added(TOP_FOO, topFoo), added(TOP_BAR, topBar)); + allListener.verifyModifications(added(TOP_FOO, topFoo), added(TOP_BAR, topBar)); } - topListener.assertModifications( + topListener.verifyModifications( added(TOP, top(topLevelList(TOP_FOO_KEY))), topSubtreeModified(topFoo, topBar)); } } diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/AbstractDataTreeChangeListenerTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/AbstractDataTreeChangeListenerTest.java index 249b765838..42e723fd2f 100644 --- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/AbstractDataTreeChangeListenerTest.java +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/AbstractDataTreeChangeListenerTest.java @@ -16,7 +16,6 @@ import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collection; import java.util.Deque; -import java.util.List; import java.util.Objects; import java.util.concurrent.TimeUnit; import org.eclipse.jdt.annotation.NonNull; @@ -57,26 +56,25 @@ public class AbstractDataTreeChangeListenerTest extends AbstractConcurrentDataBr } @SafeVarargs - public final void assertModifications(final Matcher... inOrder) { + public final void verifyModifications(final Matcher... inOrder) { final var matchers = new ArrayDeque<>(Arrays.asList(inOrder)); - final var changes = new ArrayDeque<>(listener.awaitChanges(matchers.size())); + final var changes = listener.awaitChanges(matchers.size()); while (!changes.isEmpty()) { final var mod = changes.pop(); - final var matcher = matchers.peek(); - if (matcher == null || !matcher.apply(mod)) { + final var matcher = matchers.pop(); + if (!matcher.apply(mod)) { final var rootNode = mod.getRootNode(); fail("Received unexpected notification: type: %s, path: %s, before: %s, after: %s".formatted( rootNode.getModificationType(), mod.getRootPath().getRootIdentifier(), rootNode.getDataBefore(), rootNode.getDataAfter())); return; } - - matchers.pop(); } - if (!matchers.isEmpty()) { - fail("Unsatisfied matchers " + matchers); + final var count = listener.changeCount(); + if (count != 0) { + throw new AssertionError("Expected no more changes, %s remain".formatted(count)); } } @@ -102,7 +100,7 @@ public class AbstractDataTreeChangeListenerTest extends AbstractConcurrentDataBr synced = true; } - private void awaitSync() { + void awaitSync() { final var sw = Stopwatch.createStarted(); do { @@ -118,23 +116,35 @@ public class AbstractDataTreeChangeListenerTest extends AbstractConcurrentDataBr throw new AssertionError("Failed to achieve initial sync"); } - private List> awaitChanges(final int expectedCount) { + Deque> awaitChanges(final int expectedCount) { + final var ret = new ArrayDeque>(expectedCount); final var sw = Stopwatch.createStarted(); + int remaining = expectedCount; do { synchronized (this) { - if (accumulatedChanges.size() >= expectedCount) { - return List.copyOf(accumulatedChanges); + while (remaining != 0) { + final var change = accumulatedChanges.poll(); + if (change == null) { + break; + } + + remaining--; + ret.add(change); } } + if (remaining == 0) { + return ret; + } Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS); } while (sw.elapsed(TimeUnit.SECONDS) < 5); - synchronized (this) { - throw new AssertionError("Expected %s changes, received only %s".formatted(expectedCount, - accumulatedChanges.size())); - } + throw new AssertionError("Expected %s changes, received only %s".formatted(expectedCount, ret.size())); + } + + synchronized int changeCount() { + return accumulatedChanges.size(); } } -- 2.36.6