Skip to content

modify ProcessCredentialsProviderTest to interpret session tokens #1528

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

Merged

Conversation

cenedhryn
Copy link
Contributor

Description

The unit tests sometimes provide a session token when they create AWS credentials. Currently, the scripts that generate the credentials do not honor this parameter. This commit changes that while preserving the ability to randomize tokens, which is necessary in expiration tests.

Testing

Unit tested

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • [x ] I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed
  • A short description of the change has been added to the CHANGELOG
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@cenedhryn cenedhryn requested a review from bmaizels November 26, 2019 17:57
Copy link
Contributor

@bmaizels bmaizels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed offline I don't like the pattern of passing in an arbitrary string as a fixed argument to trigger optional behavior, I would prefer proper command line processing, but given this is a windows batch file and given its scope (test helper script), I think it's fine to just acknowledge and continue with this version.

@@ -6,7 +6,11 @@ ECHO "AccessKeyId": "%1",
ECHO "SecretAccessKey": "%2"
IF NOT "%3"=="" (
ECHO ,
ECHO "SessionToken": "%RANDOM"
IF "%3"=="RANDOM_TOKEN" (
ECHO "SessionToken": "%RANDOM"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to %RANDOM%

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it is not the optimal way to structure the script.

@codecov-io
Copy link

Codecov Report

Merging #1528 into master will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1528      +/-   ##
============================================
+ Coverage     74.78%   74.87%   +0.08%     
  Complexity      759      759              
============================================
  Files           867      871       +4     
  Lines         26715    26838     +123     
  Branches       2048     2065      +17     
============================================
+ Hits          19979    20095     +116     
- Misses         5820     5822       +2     
- Partials        916      921       +5
Flag Coverage Δ Complexity Δ
#unittests 74.87% <ø> (+0.08%) 759 <ø> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
...tomization/CodegenCustomizationProcessorChain.java 81.81% <0%> (-8.19%) 0% <0%> (ø)
.../http/nio/netty/internal/NettyRequestExecutor.java 66.32% <0%> (-5.47%) 0% <0%> (ø)
...on/awssdk/protocols/ion/AwsIonProtocolFactory.java 72.72% <0%> (-5.06%) 0% <0%> (ø)
.../software/amazon/awssdk/core/sync/RequestBody.java 89.28% <0%> (-3.31%) 0% <0%> (ø)
...ssdk/core/internal/async/FileAsyncRequestBody.java 80.95% <0%> (-2.86%) 0% <0%> (ø)
...ftware/amazon/awssdk/codegen/internal/Jackson.java 59.09% <0%> (-2.82%) 0% <0%> (ø)
...on/awssdk/codegen/internal/DocumentationUtils.java 74.19% <0%> (-2.48%) 0% <0%> (ø)
...db/mappingclient/extensions/WriteModification.java 52% <0%> (-2.17%) 5% <0%> (ø)
...etty/internal/http2/Http2ToHttpInboundAdapter.java 54.54% <0%> (-1.98%) 0% <0%> (ø)
...amodb/mappingclient/operations/ConditionCheck.java 41.66% <0%> (-1.2%) 3% <0%> (ø)
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea6938c...45e928c. Read the comment docs.

…tokens in script

fix RANDOM function in bat script for unit test
@cenedhryn cenedhryn force-pushed the salande/processcredentialsprovider-update-test-sessiontoken branch from 4cd9cc9 to d79ffe0 Compare December 2, 2019 22:10
@cenedhryn cenedhryn merged commit be557e7 into master Dec 3, 2019
@cenedhryn cenedhryn deleted the salande/processcredentialsprovider-update-test-sessiontoken branch December 19, 2019 23:40
aws-sdk-java-automation added a commit that referenced this pull request Jul 9, 2021
…a18bedd06

Pull request: release <- staging/a59b358e-c6e3-4945-9b19-efaa18bedd06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants