-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tests for SnowflakeFileTransferAgent (part 1) #1288
Conversation
@@ -315,6 +320,11 @@ private static InputStreamWithMetadata compressStreamWithGZIP( | |||
|
|||
countingStream.flush(); | |||
|
|||
// Normal flow will never hit here. This is only for testing purposes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you write a utility method as same stuff is there multiple time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a method to check if the exception injection was enabled. The other parts are checking specific exceptions that the method could throw. Extracting this to a utility method may result in having to change the signatures of the methods to handle additional exceptions they wouldn't throw in normal flow.
081a14b
to
332124d
Compare
remove file callable injection put command fix tests
c2cec6d
to
9b9c1b1
Compare
@@ -315,6 +324,12 @@ private static InputStreamWithMetadata compressStreamWithGZIP( | |||
|
|||
countingStream.flush(); | |||
|
|||
// Normal flow will never hit here. This is only for testing purposes | |||
if (isInjectedFileTransferExceptionEnabled() | |||
&& injectedFileTransferException instanceof NoSuchAlgorithmException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why can't we make new method like void checkForInjectedTransferException() throws NoSuchAlgorithmException
{
if (isInjectedFileTransferExceptionEnabled()
&& injectedFileTransferException instanceof NoSuchAlgorithmException) {
throw (NoSuchAlgorithmException) SnowflakeFileTransferAgent.injectedFileTransferException;
}
}
then calll this method everywhere.
SonarQube Quality Gate |
fcae45f
to
c095f40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
try to remove some duplication and add more tests remove duplication fix test
c095f40
to
0584199
Compare
Overview
SNOW-XXXXX
External contributors - please answer these questions before submitting a pull request. Thanks!
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes #NNNN
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Please write a short description of how your code change solves the related issue.
Pre-review checklist