fix: unit tests for KIX and remove regional calls to boto #2640
Conversation
| algo = "lda" | ||
| accounts = _accounts_for_algo(algo) | ||
|
|
||
| for region in regions.regions(): |
There was a problem hiding this comment.
Refactoring this test since it is a duplicacy and the LDA param can be instead pulled into the config and tested from above
| uri = image_uris.retrieve(algo, region) | ||
| assert expected_uris.algo_uri(algo, accounts[region], region) == uri | ||
| else: | ||
| with pytest.raises(ValueError) as e: |
There was a problem hiding this comment.
Removing these if/else blocks because 1. The if is really filtering the regions and can be combined with the loop above, and since we are removing use of boto regions anyways this makes no sense.
The ValueError raised in else is not something that should be done in a test, especially if we are checking only supported regions, and seems redundant.
ahsan-z-khan
changed the title
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
shreyapandit
changed the title
| @@ -78,7 +77,7 @@ | |||
|
|
|||
| @pytest.mark.parametrize("xgboost_framework_version", XGBOOST_FRAMEWORK_CPU_GPU_VERSIONS) | |||
| def test_xgboost_framework(xgboost_framework_version): | |||
| for region in regions.regions(): | |||
| for region in FRAMEWORK_REGISTRIES.keys(): | |||
There was a problem hiding this comment.
If we are removing regions.region what are we testing here now? Seems to me we are testing image_uris.retrieve() to expected_uris.framework_uri() which is unnecessary.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
shreyapandit
changed the title
* remove boto region fetch call and use locals where applicable
Issue #, if available:
https://tiny.amazon.com/1eo2abrqk
Description of changes:
Need to add KIX regional account ids for unit tests that are missing the same, and use local config for unit tests
Testing done:
Ran all unit tests
================== test session starts ===================================
platform darwin -- Python 3.8.3, pytest-6.0.2, py-1.10.0, pluggy-0.13.1
cachedir: .tox/py38/.pytest_cache
rootdir: /Users/pandishr/MLFrameworks/sagemaker-python-sdk, configfile: tox.ini
plugins: cov-2.12.1, xdist-2.3.0, rerunfailures-10.1, timeout-1.4.2, forked-1.3.0
==========5003 passed, 447 skipped, 1 xfailed, 1 xpassed, 3046 warnings in ==================
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
unique_name_from_baseto create resource names in integ tests (if appropriate)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.