amaechler commented on code in PR #16521:
URL: https://github.com/apache/druid/pull/16521#discussion_r1621482281
##########
extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureDataSegmentKillerTest.java:
##########
@@ -70,7 +73,7 @@ public class AzureDataSegmentKillerTest extends
EasyMockSupport
ImmutableMap.of("containerName", CONTAINER_NAME, "blobPath", BLOB_PATH),
null,
null,
- NoneShardSpec.instance(),
+ new LinearShardSpec(0),
Review Comment:
`NoneShardSpec` is marked deprecated, and it doesn't really matter for these
tests.
##########
extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureTestUtils.java:
##########
@@ -25,34 +25,14 @@
import org.easymock.EasyMockSupport;
import org.easymock.IExpectationSetters;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
import java.net.URI;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
public class AzureTestUtils extends EasyMockSupport
{
- public static File createZipTempFile(final String segmentFileName, final
String content) throws IOException
Review Comment:
Moved to the one test class that was using this.
##########
extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureDataSegmentPusherTest.java:
##########
@@ -315,39 +310,23 @@ public void getPathForHadoopWithPrefixTest()
{
AzureDataSegmentPusher pusher = new AzureDataSegmentPusher(azureStorage,
azureAccountConfig, segmentConfigWithPrefix);
String hadoopPath = pusher.getPathForHadoop();
-
Assert.assertEquals("wasbs://[email protected]/prefix/",
hadoopPath);
+ assertEquals("wasbs://[email protected]/prefix/",
hadoopPath);
}
@Test
public void getPathForHadoopWithoutPrefixTest()
{
AzureDataSegmentPusher pusher = new AzureDataSegmentPusher(azureStorage,
azureAccountConfig, segmentConfigWithoutPrefix);
String hadoopPath = pusher.getPathForHadoop();
- Assert.assertEquals("wasbs://[email protected]/",
hadoopPath);
- }
-
- @Test
- public void test_getPathForHadoop_noArgsWithoutPrefix_succeeds()
- {
- AzureDataSegmentPusher pusher = new AzureDataSegmentPusher(azureStorage,
azureAccountConfig, segmentConfigWithoutPrefix);
- String hadoopPath = pusher.getPathForHadoop("");
Review Comment:
`getPathForHadoop(String)` is deprecated, and `AzureDataSegmentPusher` calls
`getPathForHadoop()` either way.
##########
extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureDataSegmentKillerTest.java:
##########
@@ -121,27 +124,35 @@ public void killTest() throws SegmentLoadingException,
BlobStorageException
verifyAll();
}
- @Test(expected = SegmentLoadingException.class)
+ @Test
public void
test_kill_StorageExceptionExtendedErrorInformationNull_throwsException()
- throws SegmentLoadingException, BlobStorageException
{
+ String dirPath = Paths.get(BLOB_PATH).getParent().toString();
Review Comment:
Removed duplicate test, and inlined the helper method.
##########
extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureStorageDruidModuleTest.java:
##########
@@ -163,120 +164,116 @@ public void
testGetAzureEntityFactoryCanCreateAzureEntity()
EasyMock.expect(cloudObjectLocation2.getPath()).andReturn(PATH);
replayAll();
- injector = makeInjectorWithProperties(PROPERTIES);
+ final Injector injector = makeInjectorWithProperties(PROPERTIES);
AzureEntityFactory factory =
injector.getInstance(AzureEntityFactory.class);
Object object1 = factory.create(cloudObjectLocation1, azureStorage,
AzureInputSource.SCHEME);
Object object2 = factory.create(cloudObjectLocation2, azureStorage,
AzureInputSource.SCHEME);
Object object3 = factory.create(cloudObjectLocation1, azureStorage,
AzureStorageAccountInputSource.SCHEME);
- Assert.assertNotNull(object1);
- Assert.assertNotNull(object2);
- Assert.assertNotNull(object3);
- Assert.assertNotSame(object1, object2);
- Assert.assertNotSame(object1, object3);
+ assertNotNull(object1);
+ assertNotNull(object2);
+ assertNotNull(object3);
+ assertNotSame(object1, object2);
+ assertNotSame(object1, object3);
}
@Test
public void
testGetAzureCloudBlobIteratorFactoryCanCreateAzureCloudBlobIterator()
{
- injector = makeInjectorWithProperties(PROPERTIES);
+ final Injector injector = makeInjectorWithProperties(PROPERTIES);
AzureCloudBlobIteratorFactory factory =
injector.getInstance(AzureCloudBlobIteratorFactory.class);
Object object1 = factory.create(EMPTY_PREFIXES_ITERABLE, 10, azureStorage);
Object object2 = factory.create(EMPTY_PREFIXES_ITERABLE, 10, azureStorage);
- Assert.assertNotNull(object1);
- Assert.assertNotNull(object2);
- Assert.assertNotSame(object1, object2);
+ assertNotNull(object1);
+ assertNotNull(object2);
+ assertNotSame(object1, object2);
}
@Test
public void
testGetAzureCloudBlobIterableFactoryCanCreateAzureCloudBlobIterable()
{
- injector = makeInjectorWithProperties(PROPERTIES);
+ final Injector injector = makeInjectorWithProperties(PROPERTIES);
AzureCloudBlobIterableFactory factory =
injector.getInstance(AzureCloudBlobIterableFactory.class);
AzureCloudBlobIterable object1 = factory.create(EMPTY_PREFIXES_ITERABLE,
10, azureStorage);
AzureCloudBlobIterable object2 = factory.create(EMPTY_PREFIXES_ITERABLE,
10, azureStorage);
- Assert.assertNotNull(object1);
- Assert.assertNotNull(object2);
- Assert.assertNotSame(object1, object2);
+ assertNotNull(object1);
+ assertNotNull(object2);
+ assertNotSame(object1, object2);
}
@Test
public void testSegmentKillerBoundSingleton()
{
Injector injector = makeInjectorWithProperties(PROPERTIES);
OmniDataSegmentKiller killer =
injector.getInstance(OmniDataSegmentKiller.class);
-
Assert.assertTrue(killer.getKillers().containsKey(AzureStorageDruidModule.SCHEME));
- Assert.assertSame(
+
assertTrue(killer.getKillers().containsKey(AzureStorageDruidModule.SCHEME));
+ assertSame(
AzureDataSegmentKiller.class,
killer.getKillers().get(AzureStorageDruidModule.SCHEME).get().getClass()
);
- Assert.assertSame(
+ assertSame(
killer.getKillers().get(AzureStorageDruidModule.SCHEME).get(),
killer.getKillers().get(AzureStorageDruidModule.SCHEME).get()
);
}
- @Test
- public void testMultipleCredentialsSet()
+ @ParameterizedTest
+ @MethodSource("propertiesWithMultipleCredentials")
Review Comment:
Rewrote this as a parametrized test. The `expectedException` usage must also
have had a bug, because this change highlighted a mistake in the test;
`useAzureCredentialsChain` is what is checked in code for the config, not
`managedIdentityClientId`.
--
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]
