Skip to content

refactor config tests to be in CI and use run_cmd instead of aws#7211

Merged
dlm6693 merged 2 commits intoaws:developfrom
dlm6693:test-config-fix
Aug 25, 2022
Merged

refactor config tests to be in CI and use run_cmd instead of aws#7211
dlm6693 merged 2 commits intoaws:developfrom
dlm6693:test-config-fix

Conversation

@dlm6693
Copy link
Copy Markdown
Contributor

@dlm6693 dlm6693 commented Aug 22, 2022

This PR moves the tests for the configure command to the functional suite. This was done for a few reasons. First and foremost, these tests silently failed or were not run at all in a recent release of the CLI v1 that allowed a bug to be introduced to customers. Additionally, this command doesn't actually make any HTTP requests over the wire so they can be run in the GitHub CI.

These tests were also refactored to use the run_cmd test function instead of aws, which provides a more robust mocking of CLI commands.

Some light amount of changes were made to custom configure subcommands to comply with the new test suite.

Moving to functional suite so that they run in GitHub CI. They can be
moved because no actual HTTP requests are made using the `configure`
command. These tests either silently failed or weren't run at all in a
release that allowed a bug to be introduced to customers using the
latest version of v1.
Comment on lines +37 to +42
def __init__(self, session, stream=None, error_stream=None):
super(ConfigureGetCommand, self).__init__(session)
if stream is None:
stream = sys.stdout
if error_stream is None:
error_stream = sys.stderr
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was updated because of the method default variable bug that every python programmer encounters from time to time. Because these defaults are evaluated before runtime, they are not properly mocked out by the test runner. Same issue for list.py.

section = profile_to_section(profile)
updated_config = {'__section__': section, varname: value}
self._config_writer.update_config(updated_config, config_filename)
return 0
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The new test class test_configure inherits from expects a return code for ALL commands.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 22, 2022

Codecov Report

Merging #7211 (8831d90) into develop (49df0c0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #7211   +/-   ##
========================================
  Coverage    92.91%   92.91%           
========================================
  Files          205      205           
  Lines        16416    16424    +8     
========================================
+ Hits         15253    15261    +8     
  Misses        1163     1163           
Impacted Files Coverage Δ
awscli/customizations/configure/get.py 98.46% <100.00%> (+0.10%) ⬆️
awscli/customizations/configure/list.py 92.30% <100.00%> (+0.47%) ⬆️
awscli/customizations/configure/set.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Copy Markdown
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for doing this. I realize this was likely tedious to port. I only had small comments and suggestions on improvements on the previous test content.

Copy link
Copy Markdown
Contributor

@kyleknap kyleknap left a comment

Choose a reason for hiding this comment

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

Looks good! 🚢

@dlm6693 dlm6693 merged commit d2d7dff into aws:develop Aug 25, 2022
@dlm6693 dlm6693 deleted the test-config-fix branch August 25, 2022 21:08
aws-sdk-python-automation added a commit that referenced this pull request Aug 26, 2022
* release-1.25.62:
  Bumping version to 1.25.62
  Update changelog based on model updates
  Add note for Tagged Unions
  refactor config tests to be in CI and use `run_cmd` instead of `aws` (#7211)
  Update sidebar AWS logo in refs
ConnorKirk pushed a commit to ConnorKirk/aws-cli that referenced this pull request Oct 28, 2022
…ws#7211)

* refactor config tests to be in CI and use `run_cmd` instead of `aws`

Moving to functional suite so that they run in GitHub CI. They can be
moved because no actual HTTP requests are made using the `configure`
command. These tests either silently failed or weren't run at all in a
release that allowed a bug to be introduced to customers using the
latest version of v1.
godd9170 pushed a commit to godd9170/aws-cli that referenced this pull request Mar 7, 2023
…ws#7211)

* refactor config tests to be in CI and use `run_cmd` instead of `aws`

Moving to functional suite so that they run in GitHub CI. They can be
moved because no actual HTTP requests are made using the `configure`
command. These tests either silently failed or weren't run at all in a
release that allowed a bug to be introduced to customers using the
latest version of v1.
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