HDDS-12884.unit tests for Ozone reconciler #8320
Conversation
|
@babluraul please add jira id and update the PR description according to the template, thanks! |
@jojochuang I've added the JIRA ID and updated the PR description according to the template. Let me know if there's anything else needed. Thanks! |
...c/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java
Outdated
Show resolved
Hide resolved
...c/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java
Show resolved
Hide resolved
...c/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java
Show resolved
Hide resolved
...c/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java
Show resolved
Hide resolved
| @Test | ||
| void testGetContainerChecksumInfoSuccess() throws IOException { | ||
| String content = "mock-checksum"; | ||
| Files.write(checksumFile.toPath(), content.getBytes()); |
There was a problem hiding this comment.
Do not write dummy content to the class object i.e container or checksumFile as it may break the other test
| Files.write(checksumFile.toPath(), "dummy content".getBytes()); | ||
|
|
||
| Container mockContainer = mock(Container.class); | ||
| when(mockContainer.getContainerData()).thenReturn(container); |
There was a problem hiding this comment.
Why are we returning container instead of mockContainer?
There was a problem hiding this comment.
in the implemention
public KeyValueContainerData getContainerData() {
return containerData;
}
its returning a KeyValueContainerData type
so in my case container which i return is KeyValueContainer type
| void testChecksumFileExistFalse() { | ||
|
|
||
| Container mockContainer = mock(Container.class); | ||
| when(mockContainer.getContainerData()).thenReturn(container); |
There was a problem hiding this comment.
Why are we returning container instead of mockContainer?
There was a problem hiding this comment.
When the getContainerData() method of the mockContainer is called, it returns the real container object
|
|
||
| @Test | ||
| void testChecksumFileExistTrue() throws IOException { | ||
| Files.write(checksumFile.toPath(), "dummy content".getBytes()); |
There was a problem hiding this comment.
Do not update the content of class object
There was a problem hiding this comment.
We shouldn't be creating checksumFile manually
| public void testReconcileFailsWithNotEligibleReplicas() throws Exception { | ||
| addContainer(RATIS_THREE_REP, LifeCycleState.CLOSED); | ||
|
|
||
| addReplicasToContainer(State.CLOSED, State.DELETED, State.DELETED); |
There was a problem hiding this comment.
Can we parametrize this with other states as well
There was a problem hiding this comment.
The current test is already a duplicate, so adding more states likely won’t add coverage. I’ll go ahead and remove it
|
|
||
| @Test | ||
| public void testReconcileFailsDueToInvalidContainerState() throws Exception { | ||
| addContainer(RATIS_THREE_REP, LifeCycleState.DELETING); |
There was a problem hiding this comment.
The current test is already a duplicate, so adding more states likely won’t add coverage. I’ll go ahead and remove it
| } | ||
|
|
||
| @Test | ||
| public void testScmNotLeader() { |
|
closing since most of the changes are duplicates |
-Added unit test for
ContainerChecksumTreeManager,
ReconcileContainerEventHandler
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12884