Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contains_inanyorder fails although I would expect it to pass #185

Closed
redsox10 opened this issue Oct 22, 2021 · 5 comments
Closed

contains_inanyorder fails although I would expect it to pass #185

redsox10 opened this issue Oct 22, 2021 · 5 comments

Comments

@redsox10
Copy link

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?

@offbyone
Copy link
Member

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

@brunns
Copy link
Member

brunns commented Oct 25, 2021

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

@brunns
Copy link
Member

brunns commented Oct 25, 2021

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

@rittneje
Copy link

rittneje commented Aug 11, 2022

@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.

@offbyone
Copy link
Member

@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.

@brunns brunns closed this as completed Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants