[GitHub] [ozone] adoroszlai commented on a diff in pull request #3302: HDDS-6529. Adding Unit-Test cases for S3-Gateway Object-Endpoint Metrics

[GitBox <gi...@apache.org>]

adoroszlai commented on code in PR #3302:
URL: https://github.com/apache/ozone/pull/3302#discussion_r862650168


##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -272,11 +273,367 @@ public void testPutAclFailure() throws Exception {
     assertEquals(1L, curMetric - oriMetric);
   }
 
+
+  /**
+   * Object Level Endpoints.
+   */
+
+  @Test
+  public void testHeadKeySuccess() throws Exception {
+    String value = RandomStringUtils.randomAlphanumeric(32);
+    OzoneOutputStream out = bucket.createKey(keyName,
+        value.getBytes(UTF_8).length, ReplicationType.RATIS,
+        ReplicationFactor.ONE, new HashMap<>());
+    out.write(value.getBytes(UTF_8));
+    out.close();
+
+    // Test for Success of HeadKeySuccess Metric
+    long oriMetric = metrics.getHeadKeySuccess();
+
+    keyEndpoint.head(bucketName, keyName);
+
+    long curMetric = metrics.getHeadKeySuccess();
+    assertEquals(1L, curMetric - oriMetric);
+  }
+
+  @Test
+  public void testHeadKeyFailure() throws Exception {
+    // Test for Failure of HeadKeyFailure Metric

Review Comment:
   These comments are unnecessary.  Test method names describe the same.



##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -272,11 +273,367 @@ public void testPutAclFailure() throws Exception {
     assertEquals(1L, curMetric - oriMetric);
   }
 
+
+  /**
+   * Object Level Endpoints.
+   */
+
+  @Test
+  public void testHeadKeySuccess() throws Exception {
+    String value = RandomStringUtils.randomAlphanumeric(32);
+    OzoneOutputStream out = bucket.createKey(keyName,
+        value.getBytes(UTF_8).length, ReplicationType.RATIS,
+        ReplicationFactor.ONE, new HashMap<>());
+    out.write(value.getBytes(UTF_8));
+    out.close();

Review Comment:
   `head()` is not affected by the contents of the key.  I think content could be fix, and key name should be random instead.
   
   Also, it would be nice to extract a helper method for creating a key to keep the tests more concise.



##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -32,36 +32,48 @@
 import org.apache.hadoop.ozone.s3.endpoint.ObjectEndpoint;
 import org.apache.hadoop.ozone.s3.endpoint.RootEndpoint;
 import org.apache.hadoop.ozone.s3.endpoint.TestBucketAcl;
+import org.apache.hadoop.ozone.s3.endpoint.MultipartUploadInitiateResponse;
+import org.apache.hadoop.ozone.s3.endpoint.CompleteMultipartUploadRequest;
 import org.apache.hadoop.ozone.s3.exception.OS3Exception;
 import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+
+
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mockito;
 
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.HashMap;
 
 import static java.net.HttpURLConnection.HTTP_OK;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.hadoop.ozone.s3.util.S3Consts.COPY_SOURCE_HEADER;
+import static org.apache.hadoop.ozone.s3.util.S3Consts.STORAGE_CLASS_HEADER;
+import static org.apache.hadoop.ozone.s3.util.S3Utils.urlEncode;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.when;
 
 /**
  * Tests for {@link S3GatewayMetrics}.
  */
 public class TestS3GatewayMetrics {
 
   private String bucketName = OzoneConsts.BUCKET;
+  private String keyName =  OzoneConsts.KEY;

Review Comment:
   I think each test case should use a different key name, so that any failures do not affect other test cases.



##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java:
##########
@@ -272,11 +273,367 @@ public void testPutAclFailure() throws Exception {
     assertEquals(1L, curMetric - oriMetric);
   }
 
+
+  /**
+   * Object Level Endpoints.
+   */
+
+  @Test
+  public void testHeadKeySuccess() throws Exception {
+    String value = RandomStringUtils.randomAlphanumeric(32);
+    OzoneOutputStream out = bucket.createKey(keyName,
+        value.getBytes(UTF_8).length, ReplicationType.RATIS,
+        ReplicationFactor.ONE, new HashMap<>());
+    out.write(value.getBytes(UTF_8));
+    out.close();
+
+    // Test for Success of HeadKeySuccess Metric
+    long oriMetric = metrics.getHeadKeySuccess();
+
+    keyEndpoint.head(bucketName, keyName);
+
+    long curMetric = metrics.getHeadKeySuccess();
+    assertEquals(1L, curMetric - oriMetric);
+  }
+
+  @Test
+  public void testHeadKeyFailure() throws Exception {
+    // Test for Failure of HeadKeyFailure Metric
+    long oriMetric = metrics.getHeadKeyFailure();
+
+    keyEndpoint.head(bucketName, "unknownKey");
+
+    long curMetric = metrics.getHeadKeyFailure();
+    assertEquals(1L, curMetric - oriMetric);
+  }
+
+  @Test
+  public void testCreateKeySuccess() throws Exception {
+
+    // Test for Success of CreateKeySuccess Metric
+    long oriMetric = metrics.getCreateKeySuccess();
+    // Create an input stream
+    ByteArrayInputStream body =
+        new ByteArrayInputStream(CONTENT.getBytes(UTF_8));
+    keyEndpoint.setHeaders(headers);

Review Comment:
   I think it would be better to call `setHeaders` in `setup()`, rather than in each test method.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org