fix: Fixing unit tests to work in Python 3 #445
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants.
In Py3, the function .iteritems() on a dict was removed and replaced with .items().
In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully.
In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list.
In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list.
Updated .iteritems() to items()
…oyment logicalid hash
eric-wieser
reviewed
May 25, 2018
tests/translator/test_translator.py
Outdated
| """ | ||
| if isinstance(obj1, dict) and isinstance(obj2, dict): | ||
| obj1 = json.dumps(obj1, sort_keys=True) | ||
| obj2 = json.dumps(obj2, sort_keys=True) |
There was a problem hiding this comment.
No way does this mimic the python 2 comparison order
There was a problem hiding this comment.
I think the code you're trying to emulate is:
https://github.com/python/cpython/blob/2.7/Objects/object.c#L767-L807
There was a problem hiding this comment.
@eric-wieser Rewriting the complete sort order is not the goal here. We wanted to get mimic the functionality of Py2 until we can make a deeper pass at the tests to assert not only the template but the order on which values are in lists (some lists in a template must be ordered). This is called out in the second bullet point in the description of the PR.
eric-wieser
reviewed
May 25, 2018
tests/translator/test_translator.py
Outdated
| @@ -139,8 +165,82 @@ def test_transform_success(self, testcase, partition_with_region): | |||
|
|
|||
| print(json.dumps(output_fragment, indent=2)) | |||
|
|
|||
| # Only update the deployment Logical Id hash in Py3. | |||
| if sys.version_info[0] >= (3): | |||
There was a problem hiding this comment.
Parens here do nothing. [0] can be better spelt .major
brettstack
approved these changes
May 25, 2018
…rator Also added lots of comments explaining why/how the deep sorting of lists work in unit tests
|
Pushed a change that addresses Eric's feedback |
eric-wieser
reviewed
May 31, 2018
| # data_str should always be unicode on python 3 | ||
| encoded_data_str = self.data_str.encode('utf-8') | ||
|
|
||
| data_hash = hashlib.sha1(bytes(encoded_data_str)).hexdigest() |
There was a problem hiding this comment.
There is no need for bytes here - you've already ensured the data is bytes by using encode above. - and if it's not, the correct thing to do is error.
eric-wieser
reviewed
May 31, 2018
tests/translator/test_translator.py
Outdated
| output_value["Ref"] = deployment_logical_id_dict[output_value.get("Ref")] | ||
|
|
||
| def _generate_new_deployment_hash(self, logical_id, dict_to_hash, rest_api_to_swagger_hash): | ||
| data_bytes = bytes(json.dumps(dict_to_hash, separators=(',', ':'), sort_keys=True).encode("utf8")) |
There was a problem hiding this comment.
bytes here is pointless too - encode always returns bytes
|
This is a blocker to enable "good" SAM template linting: aws-cloudformation/cfn-lint#81 |
brettstack
pushed a commit
that referenced
this pull request
Jun 28, 2018
* refactor: Updated imports to by Py3 compliant * refactor: Move class variable creation to constructor in globals.py Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants. * refactor: Update update_policy.py to be Py3 compliant In Py3, the function .iteritems() on a dict was removed and replaced with .items(). * refactor: Update deployment_preference_collection.py to be Py3 compliant In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully. * refactor: Update swagger.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * refactor: Update intrinsics.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * Staging translator.py changes Updated .iteritems() to items() * refactor: More updating to be Py3 compliant * refactor: Make hashing constisent between py2 and py3 * refactor: Make exceptions sortable to allow error case tests to pass in Py3 * feat: Run tox from Travis-CI * feat: Update tox to run in Py2 and Py3 * refactor: Force sorting behavior to be Py2 compatible and update Deployment logicalid hash * fix: Update tox to run tests against Py27 and Py36 * Update Travis config to run Py2 and Py3 tests in parallel * Setting region env var in tox file for Travis to pick up * Set AWS region in travis file * Pass AWS_* env vars to tox * Fixing ordering of resource types in Globals error message * Py2/3 compatible implementation of string encoding for logicalId generator Also added lots of comments explaining why/how the deep sorting of lists work in unit tests * Removing redundant usage of bytes
brettstack
added a commit
that referenced
this pull request
Jun 28, 2018
* feat: add support for Python 3 (#428) * refactor: Updated imports to by Py3 compliant * refactor: Move class variable creation to constructor in globals.py Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants. * refactor: Update update_policy.py to be Py3 compliant In Py3, the function .iteritems() on a dict was removed and replaced with .items(). * refactor: Update deployment_preference_collection.py to be Py3 compliant In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully. * refactor: Update swagger.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * refactor: Update intrinsics.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * Staging translator.py changes Updated .iteritems() to items() * refactor: More updating to be Py3 compliant * refactor: Make hashing constisent between py2 and py3 * refactor: Make exceptions sortable to allow error case tests to pass in Py3 * fix: add support for Python 3 (#445) * refactor: Updated imports to by Py3 compliant * refactor: Move class variable creation to constructor in globals.py Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants. * refactor: Update update_policy.py to be Py3 compliant In Py3, the function .iteritems() on a dict was removed and replaced with .items(). * refactor: Update deployment_preference_collection.py to be Py3 compliant In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully. * refactor: Update swagger.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * refactor: Update intrinsics.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * Staging translator.py changes Updated .iteritems() to items() * refactor: More updating to be Py3 compliant * refactor: Make hashing constisent between py2 and py3 * refactor: Make exceptions sortable to allow error case tests to pass in Py3 * feat: Run tox from Travis-CI * feat: Update tox to run in Py2 and Py3 * refactor: Force sorting behavior to be Py2 compatible and update Deployment logicalid hash * fix: Update tox to run tests against Py27 and Py36 * Update Travis config to run Py2 and Py3 tests in parallel * Setting region env var in tox file for Travis to pick up * Set AWS region in travis file * Pass AWS_* env vars to tox * Fixing ordering of resource types in Globals error message * Py2/3 compatible implementation of string encoding for logicalId generator Also added lots of comments explaining why/how the deep sorting of lists work in unit tests * Removing redundant usage of bytes
brettstack
added a commit
that referenced
this pull request
Jun 28, 2018
* feat: add support for Python 3 (#428) * refactor: Updated imports to by Py3 compliant * refactor: Move class variable creation to constructor in globals.py Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants. * refactor: Update update_policy.py to be Py3 compliant In Py3, the function .iteritems() on a dict was removed and replaced with .items(). * refactor: Update deployment_preference_collection.py to be Py3 compliant In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully. * refactor: Update swagger.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * refactor: Update intrinsics.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * Staging translator.py changes Updated .iteritems() to items() * refactor: More updating to be Py3 compliant * refactor: Make hashing constisent between py2 and py3 * refactor: Make exceptions sortable to allow error case tests to pass in Py3 * fix: add support for Python 3 (#445) * refactor: Updated imports to by Py3 compliant * refactor: Move class variable creation to constructor in globals.py Without moving this to the __init__, the globals.py file will not run in Py3 because it can't reference the constants. * refactor: Update update_policy.py to be Py3 compliant In Py3, the function .iteritems() on a dict was removed and replaced with .items(). * refactor: Update deployment_preference_collection.py to be Py3 compliant In Py3, .itervalues() and .iteritems() was replaced with .values() and .items(), respectfully. * refactor: Update swagger.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * refactor: Update intrinsics.py to be Py3 compliant In Py3, the .keys() method on a dictionary returns a dict_keys object that is a view into the dictionary. In Py2, it returns a list. To support Py3, we need to convert the .keys() to a list. * Staging translator.py changes Updated .iteritems() to items() * refactor: More updating to be Py3 compliant * refactor: Make hashing constisent between py2 and py3 * refactor: Make exceptions sortable to allow error case tests to pass in Py3 * feat: Run tox from Travis-CI * feat: Update tox to run in Py2 and Py3 * refactor: Force sorting behavior to be Py2 compatible and update Deployment logicalid hash * fix: Update tox to run tests against Py27 and Py36 * Update Travis config to run Py2 and Py3 tests in parallel * Setting region env var in tox file for Travis to pick up * Set AWS region in travis file * Pass AWS_* env vars to tox * Fixing ordering of resource types in Globals error message * Py2/3 compatible implementation of string encoding for logicalId generator Also added lots of comments explaining why/how the deep sorting of lists work in unit tests * Removing redundant usage of bytes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
No issue in this repo but related to aws/aws-sam-cli#387
Description of changes:
This change is complimentary to the earlier PR (#428) to make unit tests run in Python 3. There is a lot of interesting nuances in this PR that might not be obvious at start.
Translator computes a SHA hash of Swagger dictionary for API Gateway resources. This hash is appended to API Gateway Deployment resource's LogicalID to trigger a new APIGW deployment when Swagger changes. This hash is computed by stringifying swagger dictionary using
str(swagger_dict). The string value changes between Python2 and Python3 because of difference between the underlying dictionary implementation in the two versions. Hence the computed hash also changes. In this PR, we update the test expectation by re-computing the SHA within the test for Python3 only.The test used deepsort to sort the output CFN template dictionary and compare against the expectation. In Python2, this worked because you could sort a list with complex values just based on the data type of the value. This behavior is not there in Python3. Because the lists get sorted, they are by definition compared as an unordered list. This is wrong because lists like
Fn::Sub: []are ordered. To keep the PR focused on py3, we choose to ignore this problem. Instead, we used a custom comparator to mimic Py2's list sorting behavior for Py3.Updated Tox and Travis to run tests against both Python versions. All subsequent PRs must support both versions of Python before they can be merged.
Tox was setting PYTHONHASHSEED to random value. In Py2, this caused the order of elements within a dictionary to change resulting in APIGW Deployment Hash to change between every test run. Changed this to a fixed value in
.tox.inifileNOTE (only if you run Translator locally)
SAM Translator will work without any issues in Python 3. But if you already had a CloudFormation stack created using SAM Translator Py2, then updating your stack using Py3 translator will re-deploy your API Gateway endpoints. This is due to the first issue described above. If you never ran SAM Translator locally and always relied on CloudFormation, you don't have to worry about this problem.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.