Compat cmp dict fix

[11chrisadams11]

What does this PR do?

When passed 2 dicts, the cmp function would fail with a TypeError of "'>' not supported between instances of 'dict' and 'dict'\n". This change adds a check for dict types, compares them with ==, and returns a 0 or -1.

What issues does this PR fix or reference?

Using the boto_cfn state failed with Python 3 as it was trying to compare 2 dicts using compat.cmp

Previous Behavior

The boto_cfn state would fail with the following stack trace:

An exception occurred in this state: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/state.py", line 2154, in call
       *cdata["args"], **cdata["kwargs"]
  File "/usr/lib/python3/dist-packages/salt/loader.py",
    line 2087, in wrapper
    return f(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/states/boto_cfn.py",
    line 199, in present
    compare = salt.utils.compat.cmp(template, _template_body)

  File "/usr/lib/python3/dist-packages/salt/utils/compat.py", line 66, in cmp
     return (x > y) - (x < y)
TypeError: '>' not supported between instances of 'dict' and 'dict'

New Behavior

The state completes successfully

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

[waynew]

Hey @11chrisadams11 thanks for the PR! This looks mostly great!

However - even (or especially?) for a change this small we are requiring tests. Should be just a couple of simple cases:

• - if x and y are equivalent (or copy) dicts, cmp should return 0
• - if x and y are not equivalent dicts, cmp should return -1 (maybe just do cmp(x, y), cmp(y, x), and expected_result = -1; assert result_one == result_two == expected_result
• - if x is a dict and y is not, or x is not and y is, ValueError (or TypeError?) should be raised.

Also, this will need a changelog entry, probably Fixed salt.utils.compat.cmp to work with dictionaries or something to that effect.

HTH!

[11chrisadams11]

Sounds good. I wasn't sure if a test was required since there was not an existing one. I will get it added.

[11chrisadams11]

I have added tests for my changes to compat.cmp, and added a changelog file. Let me know if there is anything else needed.

Thanks

[waynew]

Looks like a couple of test failures @11chrisadams11, mind looking into those? For pre-commit if you click on that and then check the console output it will tell you what's wrong. The freebsd122 isn't required so can probably be ignored - doesn't look like it's even able to find any image (but merging in the latest changes from the master branch will probably sort things)

[11chrisadams11]

I made the changes required by the pre-commit check, and everything has passed now.

[11chrisadams11]

All checks have passed now. Is there anything else needed from me for this?

[waynew]

🚀

[twangboy]

Moved tests to pytests and rebased... looks good