adelapena commented on code in PR #2903:
URL: https://github.com/apache/cassandra/pull/2903#discussion_r1395873141
##########
test/unit/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDCTest.java:
##########
@@ -91,7 +90,7 @@ public void testCDCWriteFailure() throws Throwable
// Simulate a CDC consumer reading files then deleting them
for (File f : new
File(DatabaseDescriptor.getCDCLogLocation()).tryList())
- FileUtils.deleteWithConfirm(f);
+ f.deleteIfExists();
Review Comment:
There are multiple repetitions of this loop and of the creation of the array
of CDC raw files across the test. We could encapsulate them in reusable methods:
```java
private static File[] getCDCRawFiles()
{
return new File(DatabaseDescriptor.getCDCLogLocation()).tryList();
}
private static void deleteCDCRawFiles()
{
for (File f : getCDCRawFiles())
{
f.deleteIfExists();
}
}
```
##########
test/unit/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDCTest.java:
##########
@@ -382,7 +380,7 @@ public String toString()
public boolean equals(Object other)
{
CDCIndexData cid = (CDCIndexData)other;
- return fileName.equals(cid.fileName) && offset == cid.offset;
+ return fileName.equals(cid.fileName) && offset == cid.offset &&
this.getClass().equals(other.getClass());
Review Comment:
Do we need to add this class check? That check happens after a cast, so we
know that both objects are instances of `CDCIndexData`. Since we never extend
the class, the compared objects always have the same class. In fact,
`CDCIndexData` can be a final class. I think the class check that the IDE
warning is asking for should be done before casting. For example:
```java
@Override
public boolean equals(Object other)
{
if (!(other instanceof CDCIndexData))
return false;
CDCIndexData cid = (CDCIndexData)other;
return fileName.equals(cid.fileName) && offset == cid.offset;
}
```
##########
test/unit/org/apache/cassandra/db/commitlog/CommitLogSegmentManagerCDCTest.java:
##########
@@ -327,7 +326,10 @@ public void testReplayLogic() throws Throwable
for (CDCIndexData cid : oldData)
{
if (cid.fileName.equals(ncid.fileName))
+ {
found = true;
+ break;
Review Comment:
Nice, we can add an identical `break` in the similar exists loop a few lines
above (line 308)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
