adoroszlai commented on code in PR #7264:
URL: https://github.com/apache/ozone/pull/7264#discussion_r1795086975
##########
hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/TestFileChecksumHelper.java:
##########
@@ -296,12 +282,19 @@ private XceiverClientReply buildValidResponse() {
.addChecksums(byteString)
.build();
- ContainerProtos.ChunkInfo chunkInfo =
+ ContainerProtos.ChunkInfo chunkInfo = type.equals("RATIS") ?
ContainerProtos.ChunkInfo.newBuilder()
.setChunkName("dummy_chunk")
.setOffset(1)
.setLen(10)
.setChecksumData(checksumData)
+ .build()
+ : ContainerProtos.ChunkInfo.newBuilder()
+ .setChunkName("dummy_chunk")
+ .setOffset(1)
+ .setLen(10)
+ .setChecksumData(checksumData)
+ .setStripeChecksum(byteString)
.build();
Review Comment:
No need to duplicate the builder for different types:
```suggestion
ContainerProtos.ChunkInfo.Builder chunkInfo =
ContainerProtos.ChunkInfo.newBuilder()
.setChunkName("dummy_chunk")
.setOffset(1)
.setLen(10)
.setChecksumData(checksumData);
if (!"RATIS".equals(type)) {
chunkInfo.setStripeChecksum(byteString);
}
```
##########
hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/checksum/TestFileChecksumHelper.java:
##########
@@ -119,128 +123,126 @@ public void close() throws IOException {
client.close();
}
+ private OmKeyInfo omKeyInfo(String type, FileChecksum cachedChecksum,
List<OmKeyLocationInfo> locationInfo) {
+ ReplicationConfig config = type.equals("EC") ? new ECReplicationConfig(6,
3)
+ : RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE);
- @Test
- public void testEmptyBlock() throws IOException {
- // test the file checksum of a file with an empty block.
- RpcClient mockRpcClient = mock(RpcClient.class);
-
- OzoneManagerProtocol om = mock(OzoneManagerProtocol.class);
- when(mockRpcClient.getOzoneManagerClient()).thenReturn(om);
-
- OmKeyInfo omKeyInfo = new OmKeyInfo.Builder()
+ return new OmKeyInfo.Builder()
.setVolumeName(null)
.setBucketName(null)
.setKeyName(null)
.setOmKeyLocationInfos(Collections.singletonList(
- new OmKeyLocationInfoGroup(0, new ArrayList<>())))
+ new OmKeyLocationInfoGroup(0, locationInfo)))
.setCreationTime(Time.now())
.setModificationTime(Time.now())
.setDataSize(0)
- .setReplicationConfig(RatisReplicationConfig.getInstance(
- HddsProtos.ReplicationFactor.ONE))
+ .setReplicationConfig(config)
.setFileEncryptionInfo(null)
+ .setFileChecksum(cachedChecksum)
.setAcls(null)
.build();
+ }
- when(om.lookupKey(any())).thenReturn(omKeyInfo);
+ private BaseFileChecksumHelper checksumHelper(String type, OzoneVolume
mockVolume, OzoneBucket mockBucket,
+ int length, OzoneClientConfig.ChecksumCombineMode combineMode, RpcClient
mockRpcClient, OmKeyInfo keyInfo)
+ throws IOException {
+ return type.equals("RATIS") ? new ReplicatedFileChecksumHelper(
+ mockVolume, mockBucket, "dummy", length, combineMode, mockRpcClient)
+ : new ECFileChecksumHelper(
+ mockVolume, mockBucket, "dummy", length, combineMode, mockRpcClient,
keyInfo);
+ }
- OzoneVolume mockVolume = mock(OzoneVolume.class);
- when(mockVolume.getName()).thenReturn("vol1");
- OzoneBucket bucket = mock(OzoneBucket.class);
- when(bucket.getName()).thenReturn("bucket1");
+ private Pipeline pipeline(String type, List<DatanodeDetails>
datanodeDetails) {
+ ReplicationConfig config = type.equals("RATIS") ? RatisReplicationConfig
+ .getInstance(HddsProtos.ReplicationFactor.THREE)
+ : new ECReplicationConfig(6, 3);
+
+ return Pipeline.newBuilder()
+ .setId(PipelineID.randomId())
+ .setReplicationConfig(config)
+ .setState(Pipeline.PipelineState.CLOSED)
+ .setNodes(datanodeDetails)
+ .build();
+ }
+ @ParameterizedTest
+ @ValueSource(strings = {"EC", "RATIS"})
+ public void testEmptyBlock(String helperType) throws IOException {
Review Comment:
It would be nice to make the parameter strongly typed by using
`ReplicationType` and `@EnumSource`.
```java
@EnumSource(names = {"EC", "RATIS"})
public void testEmptyBlock(ReplicationType helperType) throws IOException {
```
--
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]
