utapyngo/pytest-unordered

0.4.0 breaks 1-item sequence calls with type checking

Closed this issue · 5 comments

rcoup commented

Type checking was added in #3 (fixing #2) (v0.4.0) and enabled based on the number of arguments passed to unordered()

This breaks some code that was previously working:

def pb(message):
    """
    Helper for unordered comparisons of protobuf messages
    Usage:
        assert pb(message_obj) == pb(other_message_obj)
    """
    if isinstance(message, Message):
        message = MessageToDict(message)

    if isinstance(message, list):
        return unordered(*[pb(x) for x in message])
    elif isinstance(message, dict):
        return {k: pb(v) for k, v in message.items()}
    else:
        # primitive
        return message

Using the above, something like this causes an error (RefList & Ref are both Messages) because the list is only one-item long, so check_type ends up as True:

    assert pb(resp) == pb(
        RefList(
            refs=[
                Ref(
                    name="refs/heads/master",
                    shorthand="master",
                    target="0c64d8211c072a08d5fc6e6fe898cbb59fc83d16",
                )
            ]
        )
    )

>>> TypeError: cannot make unordered comparisons to mapping: {'name': 'refs/heads/master', 'target': '0c64d8211c072a08d5fc6e6fe898cbb59fc83d16', 'shorthand': 'master'}

I can (have) worked around it by checking for a 1-item list, but perhaps check_type could be explicitly settable via a parameter to unordered(), rather than automatically setting it based on the sequence length?

Since 0.3, you can use the following syntax:

unordered(pb(x) for x in message)

Since 0.4, you can also use

unordered([pb(x) for x in message])

to enforce type checking.

I'll add the examples to the README.

@rcoup, does this work for you?

👋 (same team as @rcoup )

I think we're trying to avoid type checking. Currently our workaround for this issue looks like:

    if isinstance(message, list):
        # unordered does type-checking on 1-length lists
        # https://github.com/utapyngo/pytest-unordered/issues/5
        ms = [pb(x) for x in message]
        if len(message) > 1:
            return unordered(*ms)
        else:
            return ms

This is pretty ugly though. A better solution might be to add a kwarg:

  • check_types=False: Don't check types. Don't guess based on number of args
  • check_types=True: Check types. Don't guess based on number of args
  • check_types=None: (default) guess based on number of args

👋
I see. You can use the UnorderedList class directly, it already has the check_type argument:

unordered(*[1]) == {1}  # breaks with TypeError
unordered([1]) == {1}  # returns False
UnorderedList([1]) == {1}  # returns False
UnorderedList([1], check_type=False) == {1}  # returns True

@rcoup, @craigds, could you please review #6? Does it work for you?