XComp commented on code in PR #20267:
URL: https://github.com/apache/flink/pull/20267#discussion_r1004484209
##########
flink-filesystems/flink-azure-fs-hadoop/src/test/java/org/apache/flink/fs/azurefs/AzureFileSystemBehaviorITCase.java:
##########
@@ -70,25 +60,43 @@ public class AzureFileSystemBehaviorITCase extends
FileSystemBehaviorTestSuite {
private static final String TEST_DATA_DIR = "tests-" + UUID.randomUUID();
- // Azure Blob Storage defaults to https only storage accounts. We check if
http support has been
- // enabled on a best effort basis and test http if so.
- @Parameterized.Parameters(name = "Scheme = {0}")
- public static List<String> parameters() throws IOException {
- boolean httpsOnly = isHttpsTrafficOnly();
- return httpsOnly ? Arrays.asList("wasbs") : Arrays.asList("wasb",
"wasbs");
- }
-
- private static boolean isHttpsTrafficOnly() throws IOException {
- if (StringUtils.isNullOrWhitespaceOnly(RESOURCE_GROUP)
- || StringUtils.isNullOrWhitespaceOnly(TOKEN_CREDENTIALS_FILE))
{
+ /**
+ * Azure Blob Storage defaults to https only storage accounts, tested in
the base class.
+ *
+ * <p>This nested class repeats the tests with http support, but only if a
best effort check on
+ * https support succeeds.
+ */
+ static class HttpSupportAzureFileSystemBehaviorITCase extends
AzureFileSystemBehaviorITCase {
+ @BeforeAll
+ static void onlyRunIfHttps() throws IOException {
// default to https only, as some fields are missing
- return true;
+ assumeThat(RESOURCE_GROUP)
+ .describedAs("Azure resource group not configured,
skipping test...")
+ .isNotBlank();
+ assumeThat(TOKEN_CREDENTIALS_FILE)
+ .describedAs("Azure token credentials not configured,
skipping test...")
+ .isNotBlank();
+ assumeThat(ACCOUNT)
Review Comment:
Shouldn't we also call assume on `CONTAINER` since it's used for the URL
generation? :thinking:
##########
flink-filesystems/flink-azure-fs-hadoop/src/test/java/org/apache/flink/fs/azurefs/AzureFileSystemBehaviorITCase.java:
##########
@@ -102,49 +110,48 @@ private static boolean isHttpsTrafficOnly() throws
IOException {
.enableHttpsTrafficOnly();
}
- @BeforeClass
- public static void checkCredentialsAndSetup() throws IOException {
+ @BeforeAll
+ static void checkCredentialsAndSetup() {
// check whether credentials and container details exist
- Assume.assumeTrue(
- "Azure container not configured, skipping test...",
- !StringUtils.isNullOrWhitespaceOnly(CONTAINER));
- Assume.assumeTrue(
- "Azure access key not configured, skipping test...",
- !StringUtils.isNullOrWhitespaceOnly(ACCESS_KEY));
+ assumeThat(CONTAINER)
+ .describedAs("Azure container not configured, skipping
test...")
+ .isNotBlank();
+ assumeThat(ACCESS_KEY)
+ .describedAs("Azure access key not configured, skipping
test...")
+ .isNotBlank();
// initialize configuration with valid credentials
final Configuration conf = new Configuration();
// fs.azure.account.key.youraccount.blob.core.windows.net = ACCESS_KEY
conf.setString("fs.azure.account.key." + ACCOUNT +
".blob.core.windows.net", ACCESS_KEY);
Review Comment:
Shouldn't we also add an assume on `ACCOUNT` since we're using it in the
parameter name and when generating the base URL? :thinking:
--
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]
