-
Notifications
You must be signed in to change notification settings - Fork 0
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 Unit Tests #26
Add Unit Tests #26
Conversation
} | ||
|
||
@Test | ||
void testGetEnvironmentVariableEmpty() { |
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.
Are we testing anything different than the testGetEnvironmentVariableNotSet method above?
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.
This tests when the environment variable is set to the empty string instead of when it's not set at all as that's the other case when the default variable is used. I've consolidated these two cases into one test.
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.
pom.xml
Outdated
@@ -10,6 +10,8 @@ | |||
<properties> | |||
<maven.compiler.source>21</maven.compiler.source> | |||
<maven.compiler.target>21</maven.compiler.target> | |||
<jmockit.version>1.49</jmockit.version> | |||
<powermock.version>2.0.2</powermock.version> |
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.
Is this powermock
dependency used anywhere?
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.
It is not - this has been removed
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.
Tests pass but they don't work great in VSCode. I am going to approve this with the comment that this might be something to look into.
These issues with running tests in VSCode should be fixed now. |
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.
Awesome, they do indeed work in VSCode now!
This PR adds unit tests for the S3-depositor methods.
Additionally, there is some refactoring within the depositor to allow its methods to be tested.
These changes were tested with Maven and verified to run correctly.