NodeItemTest: use val and Assert.assertThat()

* Lombok's val simplfies the local variable declarations
* Assert.assertThat() gives better error messages
* javadoc tidy

Signed-off-by: Paul Campbell <pcampbell@kemitix.net>

src/test/java/net/kemitix/node/NodeItemTest.java

@@ -1,18 +1,17 @@
 package net.kemitix.node;
 
+import lombok.val;
+import org.junit.Assert;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
+
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Optional;
 
-import static org.hamcrest.CoreMatchers.hasItem;
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-
 /**
  * Test for {@link NodeItem}.
  *
@@ -26,22 +25,21 @@ public class NodeItemTest {
     private Node<String> node;
 
     /**
-     * Test {@link NodeItem#Node(java.lang.Object) } that node data is
+     * Test that node data is recoverable.
-     * recoverable.
      */
     @Test
     public void shouldReturnNodeData() {
         //given
-        final String data = "this node data";
+        val data = "this node data";
         //when
         node = new NodeItem<>(data);
         //then
-        assertThat(node.getData(), is(data));
+        Assert.assertThat("can get the data from a node", node.getData(),
+                is(data));
     }
 
     /**
-     * Test {@link NodeItem#Node(java.lang.Object) } that passing null as node
+     * Test that passing null as node data throws exception.
-     * data throws exception.
      */
     @Test(expected = NullPointerException.class)
     public void shouldThrowNPEWhenDataIsNull() {
@@ -50,78 +48,79 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#Node(java.lang.Object) } that default node parent is
+     * Test that default node parent is null.
-     * null.
      */
     @Test
-    public void shouldHaveNullForDefaulParent() {
+    public void shouldHaveNullForDefaultParent() {
         //given
         node = new NodeItem<>("data");
         //then
-        assertNull(node.getParent());
+        Assert.assertThat("node created without a parent has null as parent",
+                node.getParent(), nullValue());
     }
 
     /**
-     * Test {@link NodeItem#Node(java.lang.Object, net.kemitix.node.Node) } that
+     * Test that provided node parent is returned.
-     * provided node parent is returned.
      */
     @Test
     public void shouldReturnNodeParent() {
         //given
-        Node<String> parent = new NodeItem<>("parent");
+        val parent = new NodeItem<String>("parent");
         //when
         node = new NodeItem<>("subject", parent);
         //then
-        assertThat(node.getParent(), is(parent));
+        Assert.assertThat("node created with a parent can return the parent",
+                node.getParent(), is(parent));
     }
 
     /**
-     * Test {@link NodeItem#Node(java.lang.Object, net.kemitix.node.Node) } that
+     * Test that setting the parent on a node where the proposed parent is a
-     * setting the parent on a node where the proposed parent is a child of the
+     * child of the node throws an exception.
-     * node throws an exception.
      */
     @Test(expected = NodeException.class)
     public void shouldThrowNEWhenSettingParentToAChild() {
         //given
         node = new NodeItem<>("subject");
-        Node<String> child = new NodeItem<>("child", node);
+        val child = new NodeItem<String>("child", node);
         //when
         node.setParent(child);
     }
 
     /**
-     * Test {@link NodeItem#Node(java.lang.Object, net.kemitix.node.Node) } that
+     * Test that when parent is added to created node, the created node is now a
-     * when parent is added to created node, the created node is now a child of
+     * child of the parent.
-     * the parent.
      */
     @Test
     public void shouldAddNewNodeAsChildToParent() {
         //given
-        Node<String> parent = new NodeItem<>("parent");
+        val parent = new NodeItem<String>("parent");
         //when
         node = new NodeItem<>("subject", parent);
         //then
-        assertThat(parent.getChildren(), hasItem(node));
+        Assert.assertThat(
+                "when a node is created with a parent, the parent has the new"
+                        + " node among it's children", parent.getChildren(),
+                hasItem(node));
     }
 
     /**
-     * Test {@link NodeItem#setParent(net.kemitix.node.Node) } that we return
+     * Test that we return the same parent when set.
-     * the same parent when set.
      */
     @Test
     public void shouldReturnSetParent() {
         //given
         node = new NodeItem<>("subject");
-        Node<String> parent = new NodeItem<>("parent");
+        val parent = new NodeItem<String>("parent");
         //when
         node.setParent(parent);
         //then
-        assertThat(node.getParent(), is(parent));
+        Assert.assertThat(
+                "when a node is assigned a new parent that parent can be "
+                        + "returned", node.getParent(), is(parent));
     }
 
     /**
-     * Test {@link NodeItem#setParent(net.kemitix.node.Node) } that we throw an
+     * Test that we throw an exception when passed null.
-     * exception when passed null.
      */
     @Test(expected = NullPointerException.class)
     public void shouldThrowNPEWhenSetParentNull() {
@@ -132,8 +131,8 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#setParent(net.kemitix.node.Node) } that we throw an
+     * Test that we throw an exceptions when attempting to node as its own
-     * exceptions when attempting to node as its own parent.
+     * parent.
      */
     @Test(expected = NodeException.class)
     public void shouldThrowNEWhenSetParentSelf() {
@@ -144,44 +143,53 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#setParent(net.kemitix.node.Node) } that when a node
+     * Test that when a node with an existing parent is assigned a new parent,
-     * with an existing parent is assigned a new parent, that the old parent no
+     * that the old parent no longer sees it as one of its children.
-     * longer sees it as one of its children.
      */
     @Test
     public void shouldUpdateOldParentWhenNodeSetToNewParent() {
         //given
         node = new NodeItem<>("subject");
-        Node<String> child = node.createChild("child");
+        val child = node.createChild("child");
-        Node<String> newParent = new NodeItem<>("newParent");
+        val newParent = new NodeItem<String>("newParent");
         //when
         child.setParent(newParent);
         //then
-        assertThat(child.getParent(), is(newParent));
+        Assert.assertThat(
-        assertFalse(node.getChild("child").isPresent());
+                "when a node is assigned a new parent, the old parent is "
+                        + "replaced", child.getParent(), is(newParent));
+        Assert.assertThat(
+                "when a node is assigned a new parent, the old parent no "
+                        + "longer has the node among it's children",
+                node.getChild("child").isPresent(), is(false));
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that when a node
+     * Test that when a node is added as a child to another node, that it's
-     * is added as a child to another node, that it's previous parent no longer
+     * previous parent no longer has it as a child.
-     * has it as a child.
      */
     @Test
     public void shouldRemoveNodeFromOldParentWhenAddedAsChildToNewParent() {
         //given
         node = new NodeItem<>("subject");
-        Node<String> child = node.createChild("child");
+        val child = node.createChild("child");
-        Node<String> newParent = new NodeItem<>("newParent");
+        val newParent = new NodeItem<String>("newParent");
         //when
         newParent.addChild(child);
         //then
-        assertThat(child.getParent(), is(newParent));
+        Assert.assertThat(
-        assertFalse(node.getChild("child").isPresent());
+                "when a node with an existing parent is added as a child "
+                        + "to another node, then the old parent is replaced",
+                child.getParent(), is(newParent));
+        Assert.assertThat(
+                "when a node with an existing parent is added as a child to "
+                        + "another node, then the old parent no longer has "
+                        + "the node among it's children",
+                node.getChild("child").isPresent(), is(false));
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that adding null
+     * Test that adding null as a child throws an exception.
-     * as a child throws an exception.
      */
     @Test(expected = NullPointerException.class)
     public void shouldThrowNPEWhenAddingNullAsChild() {
@@ -192,23 +200,23 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that adding a
+     * Test that adding a child is returned.
-     * child is returned.
      */
     @Test
     public void shouldReturnAddedChild() {
         //given
-        Node<String> child = new NodeItem<>("child");
         node = new NodeItem<>("subject");
+        val child = new NodeItem<String>("child");
         //when
         node.addChild(child);
         //then
-        assertThat(node.getChildren(), hasItem(child));
+        Assert.assertThat(
+                "when a node is added as a child, the node is among the "
+                        + "children", node.getChildren(), hasItem(child));
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that adding a node
+     * Test that adding a node as it's own child throws an exception.
-     * as it's own child throws an exception.
      */
     @Test(expected = NodeException.class)
     public void shouldThrowNEWhenAddingANodeAsOwnChild() {
@@ -219,8 +227,7 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that adding a node
+     * Test that adding a node to itself as a child causes an exception.
-     * to itself as a child causes an exception.
      */
     @Test(expected = NodeException.class)
     public void shouldThrowWhenAddingSelfAsChild() {
@@ -231,89 +238,93 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that adding the
+     * Test that adding the parent of a node to the node as a child causes an
-     * parent to node causes an exception.
+     * exception.
      */
     @Test(expected = NodeException.class)
     public void shouldThrowWhenAddingParentAsChild() {
         //given
-        Node<String> parent = new NodeItem<>("parent");
+        val parent = new NodeItem<String>("parent");
         node = new NodeItem<>("subject", parent);
         //when
         node.addChild(parent);
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that adding the
+     * Test that adding the grandparent to a node as a child causes an
-     * grandparent to node causes an exception.
+     * exception.
      */
     @Test(expected = NodeException.class)
     public void shouldThrowWhenAddingGrandParentAsChild() {
         //given
-        Node<String> grandParent = new NodeItem<>("grandparent");
+        val grandParent = new NodeItem<String>("grandparent");
-        Node<String> parent = new NodeItem<>("parent", grandParent);
+        val parent = new NodeItem<String>("parent", grandParent);
         node = new NodeItem<>("subject", parent);
         //when
         node.addChild(grandParent);
     }
 
     /**
-     * Test {@link NodeItem#addChild(net.kemitix.node.Node) } that adding a
+     * Test that adding a child to a node, sets the child's parent node.
-     * child to a node, sets the child's parent node.
      */
     @Test
     public void shouldSetParentOnChildWhenAddedAsChild() {
         //given
-        Node<String> child = new NodeItem<>("child");
+        val child = new NodeItem<String>("child");
         node = new NodeItem<>("subject");
         //when
         node.addChild(child);
         //then
-        assertThat(child.getParent(), is(node));
+        Assert.assertThat(
+                "when a node is added as a child, the child has the node as "
+                        + "its parent", child.getParent(), is(node));
     }
 
     /**
-     * Test {@link NodeItem#walkTree(java.util.List) } that we can walk a tree
+     * Test that we can walk a tree to the target node.
-     * to the target node.
      */
     @Test
     public void shouldWalkTreeToNode() {
         //given
-        final String grandparent = "grandparent";
+        val grandparent = "grandparent";
-        Node<String> grandParentNode = new NodeItem<>(grandparent);
+        val grandParentNode = new NodeItem<String>(grandparent);
-        final String parent = "parent";
+        val parent = "parent";
-        Node<String> parentNode = new NodeItem<>(parent, grandParentNode);
+        val parentNode = new NodeItem<String>(parent, grandParentNode);
-        final String subject = "subject";
+        val subject = "subject";
         node = new NodeItem<>(subject, parentNode);
         //when
-        Optional<Node<String>> result = grandParentNode.walkTree(Arrays.asList(
+        val result = grandParentNode.walkTree(Arrays.asList(parent, subject));
-                parent, subject));
         //then
-        assertTrue(result.isPresent());
+        Assert.assertThat("when we walk the tree to a node it is found",
-        assertThat(result.get(), is(node));
+                result.isPresent(), is(true));
+        if (result.isPresent()) {
+            Assert.assertThat(
+                    "when we walk the tree to a node the correct node is found",
+                    result.get(), is(node));
+        }
     }
 
     /**
-     * Test {@link NodeItem#walkTree(java.util.List) } that we get an empty
+     * Test that we get an empty {@link Optional} when walking a path that
-     * {@link Optional} when walking a path that doesn't exist.
+     * doesn't exist.
      */
     @Test
-    public void shouldNotFindNonExistantChildNode() {
+    public void shouldNotFindNonExistentChildNode() {
         //given
-        final String parent = "parent";
+        val parent = "parent";
-        Node<String> parentNode = new NodeItem<>(parent);
+        val parentNode = new NodeItem<String>(parent);
-        final String subject = "subject";
+        val subject = "subject";
         node = new NodeItem<>(subject, parentNode);
         //when
-        Optional<Node<String>> result = parentNode.walkTree(Arrays.asList(
+        val result = parentNode.walkTree(Arrays.asList(subject, "no child"));
-                subject, "no child"));
         //then
-        assertFalse(result.isPresent());
+        Assert.assertThat(
+                "when we walk the tree to a node that doesn't exists, nothing"
+                        + " is found", result.isPresent(), is(false));
     }
 
     /**
-     * Test {@link NodeItem#walkTree(java.util.List) } that when we pass null we
+     * Test that when we pass null we get an exception.
-     * get an exception.
      */
     @Test(expected = NullPointerException.class)
     public void shouldThrowNEWhenWalkTreeNull() {
@@ -324,8 +335,8 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#walkTree(java.util.List) } that when we pass an
+     * Test that when we pass an empty path we get and empty {@link Optional} as
-     * empty path we get and empty {@link Optional} as a result.
+     * a result.
      */
     @Test
     public void shouldReturnEmptyForEmptyWalkTreePath() {
@@ -336,37 +347,58 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#createDescendantLine(java.util.List) } that we can
+     * Test that we can create a chain of descendant nodes.
-     * create a chain of descendant nodes.
      */
     @Test
     public void shouldCreateDescendantNodes() {
         //given
         node = new NodeItem<>("subject");
-        final String alphaData = "alpha";
+        val alphaData = "alpha";
-        final String betaData = "beta";
+        val betaData = "beta";
-        final String gammaData = "gamma";
+        val gammaData = "gamma";
         //when
         node.createDescendantLine(
                 Arrays.asList(alphaData, betaData, gammaData));
         //then
-        final Optional<Node<String>> alphaOptional = node.getChild(alphaData);
+        val alphaOptional = node.getChild(alphaData);
-        assertTrue(alphaOptional.isPresent());
+        Assert.assertThat(
-        Node<String> alpha = alphaOptional.get();
+                "when creating a descendant line, the first element is found",
-        assertThat(alpha.getParent(), is(node));
+                alphaOptional.isPresent(), is(true));
-        final Optional<Node<String>> betaOptional = alpha.getChild(betaData);
+        if (alphaOptional.isPresent()) {
-        assertTrue(betaOptional.isPresent());
+            val alpha = alphaOptional.get();
-        Node<String> beta = betaOptional.get();
+            Assert.assertThat(
-        assertThat(beta.getParent(), is(alpha));
+                    "when creating a descendant line, the first element has "
-        final Optional<Node<String>> gammaOptional = beta.getChild(gammaData);
+                            + "the current node as its parent",
-        assertTrue(gammaOptional.isPresent());
+                    alpha.getParent(), is(node));
-        Node<String> gamma = gammaOptional.get();
+            val betaOptional = alpha.getChild(betaData);
-        assertThat(gamma.getParent(), is(beta));
+            Assert.assertThat(
+                    "when creating a descendant line, the second element is "
+                            + "found", betaOptional.isPresent(), is(true));
+            if (betaOptional.isPresent()) {
+                val beta = betaOptional.get();
+                Assert.assertThat(
+                        "when creating a descendant line, the second element "
+                                + "has the first as its parent",
+                        beta.getParent(), is(alpha));
+                val gammaOptional = beta.getChild(gammaData);
+                Assert.assertThat(
+                        "when creating a descendant line, the third element "
+                                + "is found", gammaOptional.isPresent(),
+                        is(true));
+                if (gammaOptional.isPresent()) {
+                    val gamma = gammaOptional.get();
+                    Assert.assertThat(
+                            "when creating a descendant line, the third "
+                                    + "element has the second as its parent",
+                            gamma.getParent(), is(beta));
+                }
+            }
+        }
     }
 
     /**
-     * Test {@link NodeItem#createDescendantLine(java.util.List) } that if we
+     * Test that if we pass null to create a chain of descendant nodes we get an
-     * pass null to create a chain of descendant nodes we get an exception.
+     * exception.
      */
     @Test(expected = NullPointerException.class)
     public void shouldThrowNPEWhenCreateDescendantNull() {
@@ -377,8 +409,7 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#createDescendantLine(java.util.List) } that if we
+     * Test that if we pass an empty list nothing is changed.
-     * pass an empty list nothing is changed.
      */
     @Test
     public void shouldChangeNothingWhenCreateDescendantEmpty() {
@@ -387,43 +418,46 @@ public class NodeItemTest {
         //when
         node.createDescendantLine(Collections.emptyList());
         //then
-        assertThat(node.getChildren().size(), is(0));
+        Assert.assertThat(
+                "when creating a descendant line from an empty list, nothing "
+                        + "is created", node.getChildren().size(), is(0));
     }
 
     /**
-     * Test {@link NodeItem#findOrCreateChild(java.lang.Object) } that we can
+     * Test that we can find a child of a node.
-     * find a child of a node.
      */
     @Test
     public void shouldFindExistingChildNode() {
         //given
         node = new NodeItem<>("subject");
-        final String childData = "child";
+        val childData = "child";
-        Node<String> child = new NodeItem<>(childData, node);
+        val child = new NodeItem<String>(childData, node);
         //when
-        Node<String> found = node.findOrCreateChild(childData);
+        val found = node.findOrCreateChild(childData);
         //then
-        assertThat(found, is(child));
+        Assert.assertThat(
+                "when searching for a child by data, the matching child is "
+                        + "found", found, is(child));
     }
 
     /**
-     * Test {@link NodeItem#findOrCreateChild(java.lang.Object) } that we create
+     * Test that we create a missing child of a node.
-     * a missing child of a node.
      */
     @Test
     public void shouldFindCreateNewChildNode() {
         //given
         node = new NodeItem<>("subject");
-        final String childData = "child";
+        val childData = "child";
         //when
-        Node<String> found = node.findOrCreateChild(childData);
+        val found = node.findOrCreateChild(childData);
         //then
-        assertThat(found.getData(), is(childData));
+        Assert.assertThat(
+                "when searching for a child by data, a new node is created",
+                found.getData(), is(childData));
     }
 
     /**
-     * Test {@link NodeItem#findOrCreateChild(java.lang.Object) } that if we
+     * Test that if we pass null we get an exception.
-     * pass null we get an exception.
      */
     @Test(expected = NullPointerException.class)
     public void shouldThrowNPEFWhenFindOrCreateChildNull() {
@@ -434,26 +468,29 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#getChild(java.lang.Object) } that we can get the
+     * Test that we can get the node for a child.
-     * node for a child.
      */
     @Test
     public void shouldGetChild() {
         //given
         node = new NodeItem<>("subject");
-        final String childData = "child";
+        val childData = "child";
-        Node<String> child = new NodeItem<>(childData);
+        val child = new NodeItem<String>(childData);
         node.addChild(child);
         //when
-        Optional<Node<String>> found = node.getChild(childData);
+        val found = node.getChild(childData);
         //then
-        assertTrue(found.isPresent());
+        Assert.assertThat("when retrieving a child by its data, it is found",
-        assertThat(found.get(), is(child));
+                found.isPresent(), is(true));
+        if (found.isPresent()) {
+            Assert.assertThat(
+                    "when retrieving a child by its data, it is the expected "
+                            + "node", found.get(), is(child));
+        }
     }
 
     /**
-     * Test {@link NodeItem#getChild(java.lang.Object) } that we throw an
+     * Test that we throw an exception when passed null.
-     * exception when passed null.
      */
     @Test(expected = NullPointerException.class)
     public void shouldThrowNPEWhenGetChildNull() {
@@ -464,22 +501,29 @@ public class NodeItemTest {
     }
 
     /**
-     * Test {@link NodeItem#createChild(java.lang.Object) } that we create a
+     * Test that we create a child as a child of the current node and with the
-     * child as a child of the current node and with the current node as its
+     * current node as its parent.
-     * parent.
      */
     @Test
-    public void shoudCreateChild() {
+    public void shouldCreateChild() {
         //given
         node = new NodeItem<>("subject");
-        final String childData = "child";
+        val childData = "child";
         //when
-        Node<String> child = node.createChild(childData);
+        val child = node.createChild(childData);
         //then
-        assertThat(child.getParent(), is(node));
+        Assert.assertThat(
-        final Optional<Node<String>> foundChild = node.getChild(childData);
+                "when creating a child node, the child has the current node "
-        assertTrue(foundChild.isPresent());
+                        + "as its parent", child.getParent(), is(node));
-        assertThat(foundChild.get(), is(child));
+        val foundChild = node.getChild(childData);
+        Assert.assertThat(
+                "when creating a child node, the child can be found by its "
+                        + "data", foundChild.isPresent(), is(true));
+        if (foundChild.isPresent()) {
+            Assert.assertThat(
+                    "when creating a child node, the correct child can be "
+                            + "found by its data", foundChild.get(), is(child));
+        }
     }
 
     /**