Skip to content
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

fix: add missing tests, correct calls to DictWrapper constructor and improve metrics type hints #168

Merged
merged 4 commits into from
Sep 21, 2020

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Sep 20, 2020

Issue #, if available:

Description of changes:

Changes:

  • validator - Add test for the validator decator with an envelope
  • data_classes - Call DictWrapper constructor with the nested dict
  • metrics - Some minor type hint improvements

Checklist


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Changes:
* validator - Add test for the validator decator with an envelope
* data_classes - Call DictWrapper constructor with the nested dict
@michaelbrewer michaelbrewer changed the title tests(validator): add missing tests for validator test(validator): add missing tests for validator Sep 20, 2020
@michaelbrewer michaelbrewer changed the title test(validator): add missing tests for validator test(validator): add missing test for @validator with an envelope Sep 20, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2020

Codecov Report

Merging #168 into develop will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop      #168      +/-   ##
============================================
+ Coverage    99.90%   100.00%   +0.09%     
============================================
  Files           52        52              
  Lines         2151      2151              
  Branches        96        96              
============================================
+ Hits          2149      2151       +2     
+ Misses           1         0       -1     
+ Partials         1         0       -1     
Impacted Files Coverage Δ
aws_lambda_powertools/metrics/base.py 100.00% <100.00%> (ø)
...bda_powertools/utilities/data_classes/alb_event.py 100.00% <100.00%> (ø)
.../utilities/data_classes/api_gateway_proxy_event.py 100.00% <100.00%> (ø)
.../utilities/data_classes/cognito_user_pool_event.py 100.00% <100.00%> (ø)
...ambda_powertools/utilities/validation/validator.py 100.00% <0.00%> (+9.52%) ⬆️

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 b654ca6...4c97eae. Read the comment docs.

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

Thanks for those changes @michaelbrewer, and for catching an issue with the data_classes utilities.

Could you update description to reflect what changed here? Title reflects validator test only.

One minor issue with type hint as it should've been float over int due to value, and numeric tower also catches int as part of float

@michaelbrewer michaelbrewer changed the title test(validator): add missing test for @validator with an envelope fix: add missing tests, correct calls to DictWrapper constructor and improve metrics type hints Sep 21, 2020
Co-authored-by: Heitor Lessa <[email protected]>
Copy link
Contributor Author

@michaelbrewer michaelbrewer left a comment

Choose a reason for hiding this comment

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

Corrected

aws_lambda_powertools/metrics/base.py Outdated Show resolved Hide resolved
@heitorlessa heitorlessa merged commit 805f23d into aws-powertools:develop Sep 21, 2020
@heitorlessa heitorlessa added the bug Something isn't working label Sep 22, 2020
@michaelbrewer michaelbrewer deleted the tests-missing branch February 22, 2021 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants