Update assertInstanceEqual to fix JSON comparison of 'data_schema' keys. #3568
Conversation
This fixes the issue where assertInstanceEqual was relying on JSON columns to have the same formatting as the incoming data, which is typically not the case.
nautobot/extras/tests/test_views.py
Outdated
| @@ -284,7 +284,7 @@ def setUpTestData(cls): | |||
| cls.form_data = { | |||
| "name": "Schema X", | |||
| "slug": "schema-x", | |||
| "data_schema": '{"type": "object", "properties": {"baz": {"type": "string"}}}', | |||
| "data_schema": '{"type": "object","properties": {"baz": {"type": "string"}}}', | |||
There was a problem hiding this comment.
Please revert this line, or is the expectation here that this is what comes back from MySQL? If so, can you please add a comment that explains why this is so?
Or... Could we instead rely on the mixin code to sort this? I'm asking because if someone like myself see the lack of a space between commas and tries to "fix" it without context, we might have a bad time.
There was a problem hiding this comment.
Please revert this line, or is the expectation here that this is what comes back from MySQL? If so, can you please add a comment that explains why this is so?
The change is actually to intentionally introduce a difference between input and MySQL output.
If you revert the other changes in this PR but keep this line, nautobot tests will start to fail, because they are testing that the input string (this one) is identical to the JSON returned by MySQL. I wanted to include this not-quite-formatted JSON to ensure that the correct JSON formatting is happening in the test.
I can keep this line and add the above explanation, or revert this change. Up to you. I get that it's weird to have poorly-formatted JSON as a test input.
There was a problem hiding this comment.
Please keep the line and add the comment to explain for posterity. Thank you!
There was a problem hiding this comment.
Looks like @bryanculver got there first and added the comment about formatting, no action required on my part. Thanks!
nautobot/utilities/testing/mixins.py
Outdated
| newJsonText = json.dumps(obj, sort_keys=True) | ||
| return newJsonText |
There was a problem hiding this comment.
| newJsonText = json.dumps(obj, sort_keys=True) | |
| return newJsonText | |
| return json.dumps(obj, sort_keys=True) |
There was a problem hiding this comment.
Will make this change.
nautobot/utilities/testing/mixins.py
Outdated
| def standardize_json(self, jsonText): | ||
| obj = json.loads(jsonText) |
There was a problem hiding this comment.
I know I'm being pedantic, but we don't use camelCase in Python and our linters will scream.
| def standardize_json(self, jsonText): | |
| obj = json.loads(jsonText) | |
| def standardize_json(self, data): | |
| obj = json.loads(data) |
There was a problem hiding this comment.
No problem, thanks for clarifying, fixing! This is my first time with Python, so I'm happy to learn the conventions.
- avoid unnecessary variable - avoid camelCase in parameters
This fixes the issue where assertInstanceEqual test code was relying on JSON columns to have the same formatting as the incoming data, which is typically not the case.
Closes: #3116
What's Changed
Update assertInstanceEqual to fix JSON comparison of 'data_schema' keys.
This fixes the issue where assertInstanceEqual test code was relying on JSON columns to have the same formatting as the incoming data, which is typically not the case.
This only impacts test code.