757 - Integration test for Hot Reload of nodes #764
Conversation
cassandra-test-image/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.ericsson.bss.cassandra.ecchronos</groupId> | ||
| <artifactId>application</artifactId> |
cassandra-test-image/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.ericsson.bss.cassandra.ecchronos</groupId> | ||
| <artifactId>application</artifactId> | ||
| <version>1.0.0-SNAPSHOT</version> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
We like to put internal dependencies on the top of all, with a comment saying "Internal", check this pom.xml
|
|
||
| protected void decommissionNode ( String node) throws IOException, InterruptedException | ||
| { | ||
| String stdout = composeContainer.getContainerByServiceName(node).get() |
There was a problem hiding this comment.
If it's a void method, there's no reason to get the stdout and to create a variable to store this stdout
| { | ||
| private static final Logger LOG = LoggerFactory.getLogger(TestNodeAddition.class); | ||
| @BeforeClass | ||
| public static void setup() throws InterruptedException { |
There was a problem hiding this comment.
public static void setup() throws InterruptedException { -->
public static void setup() throws InterruptedException
{
...
}
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
|
|
||
| import static org.mockito.Mockito.*; |
There was a problem hiding this comment.
Avoid importing all methods, actually our pmd should be failing because of this, but for some reason this module is untouchable
| composeContainer.withScaledService("cassandra-seed-dc2-rack1-node1", 1 ); | ||
| composeContainer.start(); | ||
| LOG.info("Waiting for the new nodes to finish starting up."); | ||
| Thread.sleep(50000); |
There was a problem hiding this comment.
Isn't that too much time to sleep? Also, I think it'd be better to put this in a constant, like SLEEP_TIME_IN_MS
There was a problem hiding this comment.
I changed this, with a new method to wait until the cluster is all UP
| // scale up new nodes | ||
| composeContainer.withScaledService("cassandra-node-dc1-rack1-node2", 1 ); | ||
| composeContainer.withScaledService("cassandra-node-dc2-rack1-node2", 1 ); | ||
| composeContainer.withScaledService("cassandra-seed-dc2-rack1-node1", 1 ); |
There was a problem hiding this comment.
I think after scalling the nodes you should assert that the number of nodes increased, you can do it by checking stdout of nodetool status or using the driver
There was a problem hiding this comment.
Done with new method in AbstractCassandraCluster
| @Test | ||
| public void testAdditionalNodesAddedToCluster() throws InterruptedException | ||
| { | ||
| DefaultRepairConfigurationProvider listener = mock(DefaultRepairConfigurationProvider.class); |
There was a problem hiding this comment.
I'm not sure, but as it's an IT, I think it would be good to create the main objects and run it instead of using mock
There was a problem hiding this comment.
We have agreed not to do this
|
|
||
| import static org.mockito.Mockito.*; | ||
|
|
||
| public class TestNodeAddition extends AbstractCassandraCluster |
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.Mockito.*; | ||
|
|
||
| public class TestNodeRemoval extends AbstractCassandraCluster |
New classes to test eccChronos deals with nodes being added to the cluster or nodes being decommissioned.