PyHamcrest icon indicating copy to clipboard operation
PyHamcrest copied to clipboard

contains_inanyorder fails although I would expect it to pass

Open redsox10 opened this issue 3 years ago • 5 comments

version 2.0.2

assert_that(['a',4], contains_inanyorder(*[greater_than(0), 'a']))
Expected: a sequence over [a value greater than <0>, 'a'] in any order
     but: was <['a', 4]>

I would expect is to pass as the definition of the matcher is: "This matcher iterates the evaluated sequence, seeing if each element satisfies any of the given matchers."

Reason for thie behavior is that:

class MatchInAnyOrder(object):
[...]
        def ismatched(self, item: T) -> bool:
        for index, matcher in enumerate(self.matchers):
            if matcher.matches(item):
                del self.matchers[index]
                return True

matcher.matches(item) throughs a TypeErrror when running a against greater_than(0 TypeError: '>' not supported between instances of 'str' and 'int'

Maybe it should be catched like
try:
           if matcher.matches(item):
               del self.matchers[index]
               return True
except:
           pass

What is curious though is that it is catched here:

class IsSequenceContainingInAnyOrder(BaseMatcher[Sequence[T]]):
    def __init__(self, matchers: Sequence[Matcher[T]]) -> None:
        self.matchers = matchers

    def matches(
        self, item: Sequence[T], mismatch_description: Optional[Description] = None
    ) -> bool:
        try:
            sequence = list(item)
            matchsequence = MatchInAnyOrder(self.matchers, mismatch_description)
            for element in sequence:
                if not matchsequence.matches(element):
                    return False
            return matchsequence.isfinished(sequence)
        except TypeError:
            if mismatch_description:
                super(IsSequenceContainingInAnyOrder, self).describe_mismatch(
                    item, mismatch_description
                )
            return False

But is this the correct and expected behaviour though?

redsox10 avatar Oct 22 '21 10:10 redsox10

I think you've identified a real bug; I would expect the matcher to handle mixed types with no issue.

offbyone avatar Oct 22 '21 14:10 offbyone

The comparison is taking place in OrderingComparison. I'm wondering if that's where we should be catching the TypeError?

brunns avatar Oct 25 '21 10:10 brunns

So, for example assert_that(greater_than(1, "a")) also throws a TypeError right now. Should that fail rather than throw an exception?

brunns avatar Oct 25 '21 10:10 brunns

@brunns I don't think having it return False is appropriate, as that leads to unexpected behavior with is_not.

assert_that("a", is_not(greater_than(1)))

Really I think ternary logic will be required to resolve this.

rittneje avatar Aug 11 '22 15:08 rittneje

@brunns I think the answer to your question (sorry, left it sitting) is cleared up in #215: I am okay with assert_that(greater_than(1, "a")) being resolved as not-matching, rather than TypeError-raising.

offbyone avatar Aug 13 '22 12:08 offbyone