From 1a56a7637405e71045e0720aaa7e0120d4e267e0 Mon Sep 17 00:00:00 2001 From: Jelena Furundzic Date: Thu, 19 Dec 2024 02:33:36 -0800 Subject: [PATCH 1/2] Adding testing for better code coverage --- .../cloud/storage/SnowflakeGCSClient.java | 18 +++++++ .../snowflake/client/jdbc/ConnectionIT.java | 23 +++++++++ .../client/jdbc/FileUploaderLatestIT.java | 28 +++++++++++ .../client/jdbc/SnowflakeDriverLatestIT.java | 49 ------------------- .../jdbc/telemetryOOB/TelemetryServiceIT.java | 3 -- 5 files changed, 69 insertions(+), 52 deletions(-) diff --git a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java index ccf76388d..ca85340a0 100644 --- a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java +++ b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java @@ -101,6 +101,8 @@ public class SnowflakeGCSClient implements SnowflakeStorageClient { private static final SFLogger logger = SFLoggerFactory.getLogger(SnowflakeGCSClient.class); + private static Throwable injectedException = null; // for testing purpose + private SnowflakeGCSClient() {} /* @@ -198,6 +200,13 @@ public StorageObjectSummaryCollection listObjects(String remoteStorageLocation, logger.debug( "Listing objects in the bucket {} with prefix {}", remoteStorageLocation, prefix); Page blobs = this.gcsClient.list(remoteStorageLocation, BlobListOption.prefix(prefix)); + // Normal flow will never hit here. This is only for testing purposes + if (isInjectedExceptionEnabled() + && SnowflakeGCSClient.injectedException + instanceof StorageProviderException) { + throw (StorageProviderException) + SnowflakeGCSClient.injectedException; + } return new StorageObjectSummaryCollection(blobs); } catch (Exception e) { logger.debug("Failed to list objects", false); @@ -1387,4 +1396,13 @@ public String getStreamingIngestClientName(StorageObjectMetadata meta) { public String getStreamingIngestClientKey(StorageObjectMetadata meta) { return meta.getUserMetadata().get(GCS_STREAMING_INGEST_CLIENT_KEY); } + + // This function should only be used for testing purpose + public static void setInjectedException(Throwable th) { + injectedException = th; + } + + public static boolean isInjectedExceptionEnabled() { + return injectedException != null; + } } diff --git a/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java b/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java index 7066e31c7..3f6acbe00 100644 --- a/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java +++ b/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java @@ -1014,6 +1014,29 @@ public void testFailOverOrgAccount() throws SQLException { } } + @Test + public void testDiagnosticCheckFailsWithNoAllowlistFileProvided() throws SQLException { + Properties props = new Properties(); + props.put("ENABLE_DIAGNOSTICS", true); + try(Connection con = getConnection(props)) { + fail(); + } catch (SnowflakeSQLException e) { + assertTrue(e.getMessage().contains("Diagnostics was enabled but an allowlist file was not provided")); + } + } + + @Test + public void testDiagnosticCheckWithFakeAllowlistFileProvided() throws SQLException { + Properties props = new Properties(); + props.put("ENABLE_DIAGNOSTICS", true); + props.put("DIAGNOSTICS_ALLOWLIST_FILE", "/some/path/allowlist.json"); + try(Connection con = getConnection(props)) { + fail(); + } catch (Exception e) { + assertTrue(e.getMessage().contains("A connection was not created because the driver is running in diagnostics mode.")); + } + } + private class ConcurrentConnections implements Runnable { ConcurrentConnections() {} diff --git a/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java b/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java index a116a794b..7a2ac8611 100644 --- a/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java @@ -871,4 +871,32 @@ public void testUploadWithTildeInPath() throws SQLException, IOException { FileUtils.deleteDirectory(subDir.toFile()); } } + + @DontRunOnGithubActions + @Test + public void testListObjectsException() throws Exception { + Properties paramProperties = new Properties(); + paramProperties.put("GCS_USE_DOWNSCOPED_CREDENTIAL", true); + SnowflakeGCSClient.setInjectedException( + new StorageProviderException(new Exception("could not list objects"))); + + try (Connection con = getConnection("gcpaccount", paramProperties); + Statement statement = con.createStatement()) { + try { + statement.execute("create or replace stage testStage"); + SFSession sfSession = con.unwrap(SnowflakeConnectionV1.class).getSfSession(); + String command = "PUT file://" + getFullPathFileInResource(TEST_DATA_FILE) + " @testStage"; + SnowflakeFileTransferAgent sfAgent = + new SnowflakeFileTransferAgent(command, sfSession, new SFStatement(sfSession)); + + sfAgent.execute(); + } catch (SnowflakeSQLException err) { + assertEquals(200016, err.getErrorCode()); + assertTrue(err.getMessage().contains("Failed to list objects")); + } finally { + statement.execute("DROP STAGE if exists testStage"); + } + } + SnowflakeGCSClient.setInjectedException(null); + } } diff --git a/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverLatestIT.java b/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverLatestIT.java index fde744f15..6c01ecb84 100644 --- a/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/SnowflakeDriverLatestIT.java @@ -1489,55 +1489,6 @@ public void testNoSpaceLeftOnDeviceException() throws SQLException { } } - @Test - @Disabled // TODO: ignored until SNOW-1616480 is resolved - public void testUploadWithGCSPresignedUrlWithoutConnection() throws Throwable { - File destFolder = new File(tmpFolder, "dest"); - destFolder.mkdirs(); - String destFolderCanonicalPath = destFolder.getCanonicalPath(); - // set parameter for presignedUrl upload instead of downscoped token - Properties paramProperties = new Properties(); - paramProperties.put("GCS_USE_DOWNSCOPED_CREDENTIAL", false); - try (Connection connection = getConnection("gcpaccount", paramProperties); - Statement statement = connection.createStatement()) { - try { - // create a stage to put the file in - statement.execute("CREATE OR REPLACE STAGE " + testStageName); - - SFSession sfSession = connection.unwrap(SnowflakeConnectionV1.class).getSfSession(); - - // Test put file with internal compression - String putCommand = "put file:///dummy/path/file1.gz @" + testStageName; - SnowflakeFileTransferAgent sfAgent = - new SnowflakeFileTransferAgent(putCommand, sfSession, new SFStatement(sfSession)); - List metadata = sfAgent.getFileTransferMetadatas(); - - String srcPath = getFullPathFileInResource(TEST_DATA_FILE); - for (SnowflakeFileTransferMetadata oneMetadata : metadata) { - InputStream inputStream = new FileInputStream(srcPath); - - assertTrue(oneMetadata.isForOneFile()); - SnowflakeFileTransferAgent.uploadWithoutConnection( - SnowflakeFileTransferConfig.Builder.newInstance() - .setSnowflakeFileTransferMetadata(oneMetadata) - .setUploadStream(inputStream) - .setRequireCompress(true) - .setNetworkTimeoutInMilli(0) - .setOcspMode(OCSPMode.FAIL_OPEN) - .build()); - } - - assertTrue( - statement.execute( - "GET @" + testStageName + " 'file://" + destFolderCanonicalPath + "/' parallel=8"), - "Failed to get files"); - assertTrue(isFileContentEqual(srcPath, false, destFolderCanonicalPath + "/file1.gz", true)); - } finally { - statement.execute("DROP STAGE if exists " + testStageName); - } - } - } - @Test @DontRunOnGithubActions public void testUploadWithGCSDownscopedCredentialWithoutConnection() throws Throwable { diff --git a/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java b/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java index 347bc97e3..76e660bc1 100644 --- a/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java +++ b/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java @@ -58,7 +58,6 @@ public void tearDown() throws InterruptedException { } @SuppressWarnings("divzero") - @Disabled @Test public void testCreateException() { TelemetryService service = TelemetryService.getInstance(); @@ -102,7 +101,6 @@ public void testWrongServerURL() throws InterruptedException { assertThat("WrongServerURL do not block.", service.getEventCount() > count); } - @Disabled @Test public void testCreateLog() { // this log will be delivered to snowflake @@ -161,7 +159,6 @@ public void stressTestCreateLog() { sw.stop(); } - @Disabled @Test public void testCreateLogInBlackList() { // this log will be delivered to snowflake From 17648827b606c95d2b86ad08a99c4a19a49bbe1e Mon Sep 17 00:00:00 2001 From: Jelena Furundzic Date: Thu, 19 Dec 2024 02:34:21 -0800 Subject: [PATCH 2/2] check style --- .../jdbc/cloud/storage/SnowflakeGCSClient.java | 6 ++---- .../net/snowflake/client/jdbc/ConnectionIT.java | 13 +++++++++---- .../snowflake/client/jdbc/FileUploaderLatestIT.java | 6 +++--- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java index ca85340a0..5735e1c2b 100644 --- a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java +++ b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java @@ -202,10 +202,8 @@ public StorageObjectSummaryCollection listObjects(String remoteStorageLocation, Page blobs = this.gcsClient.list(remoteStorageLocation, BlobListOption.prefix(prefix)); // Normal flow will never hit here. This is only for testing purposes if (isInjectedExceptionEnabled() - && SnowflakeGCSClient.injectedException - instanceof StorageProviderException) { - throw (StorageProviderException) - SnowflakeGCSClient.injectedException; + && SnowflakeGCSClient.injectedException instanceof StorageProviderException) { + throw (StorageProviderException) SnowflakeGCSClient.injectedException; } return new StorageObjectSummaryCollection(blobs); } catch (Exception e) { diff --git a/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java b/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java index 3f6acbe00..59a8a85cf 100644 --- a/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java +++ b/src/test/java/net/snowflake/client/jdbc/ConnectionIT.java @@ -1018,10 +1018,12 @@ public void testFailOverOrgAccount() throws SQLException { public void testDiagnosticCheckFailsWithNoAllowlistFileProvided() throws SQLException { Properties props = new Properties(); props.put("ENABLE_DIAGNOSTICS", true); - try(Connection con = getConnection(props)) { + try (Connection con = getConnection(props)) { fail(); } catch (SnowflakeSQLException e) { - assertTrue(e.getMessage().contains("Diagnostics was enabled but an allowlist file was not provided")); + assertTrue( + e.getMessage() + .contains("Diagnostics was enabled but an allowlist file was not provided")); } } @@ -1030,10 +1032,13 @@ public void testDiagnosticCheckWithFakeAllowlistFileProvided() throws SQLExcepti Properties props = new Properties(); props.put("ENABLE_DIAGNOSTICS", true); props.put("DIAGNOSTICS_ALLOWLIST_FILE", "/some/path/allowlist.json"); - try(Connection con = getConnection(props)) { + try (Connection con = getConnection(props)) { fail(); } catch (Exception e) { - assertTrue(e.getMessage().contains("A connection was not created because the driver is running in diagnostics mode.")); + assertTrue( + e.getMessage() + .contains( + "A connection was not created because the driver is running in diagnostics mode.")); } } diff --git a/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java b/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java index 7a2ac8611..4022a4743 100644 --- a/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java @@ -878,16 +878,16 @@ public void testListObjectsException() throws Exception { Properties paramProperties = new Properties(); paramProperties.put("GCS_USE_DOWNSCOPED_CREDENTIAL", true); SnowflakeGCSClient.setInjectedException( - new StorageProviderException(new Exception("could not list objects"))); + new StorageProviderException(new Exception("could not list objects"))); try (Connection con = getConnection("gcpaccount", paramProperties); - Statement statement = con.createStatement()) { + Statement statement = con.createStatement()) { try { statement.execute("create or replace stage testStage"); SFSession sfSession = con.unwrap(SnowflakeConnectionV1.class).getSfSession(); String command = "PUT file://" + getFullPathFileInResource(TEST_DATA_FILE) + " @testStage"; SnowflakeFileTransferAgent sfAgent = - new SnowflakeFileTransferAgent(command, sfSession, new SFStatement(sfSession)); + new SnowflakeFileTransferAgent(command, sfSession, new SFStatement(sfSession)); sfAgent.execute(); } catch (SnowflakeSQLException err) {