smiroslav commented on code in PR #1280:
URL: https://github.com/apache/jackrabbit-oak/pull/1280#discussion_r1467648277


##########
oak-segment-azure/src/test/java/org/apache/jackrabbit/oak/segment/azure/tool/ToolUtilsTest.java:
##########
@@ -18,59 +18,95 @@
  */
 package org.apache.jackrabbit.oak.segment.azure.tool;
 
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.Appender;
+import ch.qos.logback.core.read.ListAppender;
 import com.microsoft.azure.storage.StorageCredentials;
 import com.microsoft.azure.storage.StorageCredentialsAccountAndKey;
 import com.microsoft.azure.storage.StorageCredentialsSharedAccessSignature;
+
+import java.net.URISyntaxException;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
+import com.microsoft.azure.storage.StorageException;
+import com.microsoft.azure.storage.blob.CloudBlobDirectory;
 import 
org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage.AzuriteDockerRule;
 import org.apache.jackrabbit.oak.segment.azure.AzureUtilities;
 import org.apache.jackrabbit.oak.segment.azure.util.Environment;
+import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.MockedStatic;
+import org.slf4j.LoggerFactory;
 
+import static 
org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_ACCOUNT_NAME;
+import static 
org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_CLIENT_ID;
+import static 
org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_CLIENT_SECRET;
+import static 
org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_SECRET_KEY;
+import static 
org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_TENANT_ID;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeNotNull;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mockStatic;
 
 public class ToolUtilsTest {
-    private static final String CONTAINER_URL = 
"https://myaccount.blob.core.windows.net/oak-test";;
-    private static final String REPO_DIR = "repository";
-    private static final String SEGMENT_STORE_PATH = CONTAINER_URL + '/' + 
REPO_DIR;
+    private static final Environment ENVIRONMENT = new Environment();
+
+    private static final String CONTAINER_URL_FORMAT = 
"https://%s.blob.core.windows.net/%s";;
+    private static final String SEGMENT_STORE_PATH_FORMAT = 
CONTAINER_URL_FORMAT + "/%s";
+
+    private static final String DEFAULT_ACCOUNT_NAME = "myaccount";
+    private static final String DEFAULT_CONTAINER_NAME = "oak";
+    private static final String DEFAULT_REPO_DIR = "repository";
+    private static final String DEFAULT_CONTAINER_URL = 
String.format(CONTAINER_URL_FORMAT, DEFAULT_ACCOUNT_NAME, 
DEFAULT_CONTAINER_NAME);
+    private static final String DEFAULT_SEGMENT_STORE_PATH = 
String.format(SEGMENT_STORE_PATH_FORMAT, DEFAULT_ACCOUNT_NAME, 
DEFAULT_CONTAINER_NAME, DEFAULT_REPO_DIR);
+    public static final String AZURE_SECRET_KEY_WARNING = "AZURE_CLIENT_ID, 
AZURE_CLIENT_SECRET and AZURE_TENANT_ID environment variables empty or missing. 
Switching to authentication with AZURE_SECRET_KEY.";
 
     private final TestEnvironment environment = new TestEnvironment();
 
     @Test
     public void createCloudBlobDirectoryWithAccessKey() {
-        environment.setVariable("AZURE_SECRET_KEY", 
AzuriteDockerRule.ACCOUNT_KEY);
+        environment.setVariable(AZURE_SECRET_KEY, 
AzuriteDockerRule.ACCOUNT_KEY);
+
+        final ListAppender<ILoggingEvent> logAppender = subscribeAppender();
 
         StorageCredentialsAccountAndKey credentials = expectCredentials(
             StorageCredentialsAccountAndKey.class, 
-            () -> ToolUtils.createCloudBlobDirectory(SEGMENT_STORE_PATH, 
environment)
+            () -> 
ToolUtils.createCloudBlobDirectory(DEFAULT_SEGMENT_STORE_PATH, environment),
+            DEFAULT_CONTAINER_URL
         );
-        
-        assertEquals("myaccount", credentials.getAccountName());
+
+        assertTrue(checkLogContainsMessage(AZURE_SECRET_KEY_WARNING, 
logAppender.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toList())));

Review Comment:
   I am not in favor of this type of check. Instead, I would prefer to examine 
the concrete class returned (subtype of StorageCredentials) or use another 
method for verification.



-- 
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: dev-unsubscr...@jackrabbit.apache.org

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

Reply via email to