gotestyourself / gotest.tools

A collection of packages to augment the go testing package and support common patterns.
https://gotest.tools/v3
Other
500 stars 50 forks source link

Issues implementing a "Not" comparison #147

Open cpuguy83 opened 5 years ago

cpuguy83 commented 5 years ago

Basically, I tried implementing a generic Not comparison like assert.Assert(t, Not(cmp.Equal(foo, bar)).

The interface here seems a bit limiting as I do not have access to variables, formatting or anything from my Not comparison. It seems right now I would have to implement a NotEqual comparison in order to gain access variables and the like.

I'm wondering if some additional interface could help here? Maybe something that a comparison could optionally implement to make it easier to wrap?

dnephin commented 5 years ago

https://godoc.org/gotest.tools/assert has some examples of how Not can be done with assert.Assert().

Since you are comparing against a fixed value, and the failure message will contain the literal source code used in the assertion, you should be able to use !foo or != value and have a failure print the literal.

If the expected value is a variable then I guess the literal is less useful. Is that the case?

rliebz commented 5 years ago

Negating a bool is trivial, but a cmp.Comparison is not.

As an example of a use case I had, if I had a string myText that should contain "foo" but should not contain "bar":

assert.Check(t, cmp.Contains(myText, "foo"))
assert.Check(t, !strings.Contains(myText, "bar"))

It's workable, but the error messaging on the Comparison functions tend to be better for these situations.

tiborvass commented 4 years ago

FWIW I implemented something like it here: https://github.com/tiborvass/docker/blob/c03078863fca6f37fe852b36c19d48e2ac025f44/integration-cli/checker/checker.go#L38-L50 which required the creation of another type whose Compare name I don't like because it conflicts with cmp.Comparison but oh well.

glenjamin commented 4 years ago

i recently tried to do this with the following code, it worked well when it passed, and produced an unhelpful message when it failed.

func not(c cmp.Comparison) cmp.Comparison {
    return func() cmp.Result {
        return InvertedResult{c()}
    }
}

type InvertedResult struct {
    cmp.Result
}

func (r InvertedResult) Success() bool {
    return !r.Result.Success()
}

If the FailureMessage interfaces were public, it might be possible to wrap generically enough to prefix the messages with NOT - but I didn't spend a lot of time on this

dnephin commented 4 years ago
func (r InvertedResult) FailureMessage(args []ast.Expr) string {
    if f, ok := r.Result.(interface{
                FailureMessage() string
    }); ok {
            return "NOT " + f.FailureMessage()
        }

    if f, ok := r.Result.(interface{
                FailureMessage(args []ast.Expr) string
    }); ok {
            return "NOT " + f.FailureMessage(args)
        }        
    return "failed, but I do not know why"
}

I think something along those lines should work.

dnephin commented 4 years ago
assert.Check(t, !strings.Contains(myText, "bar"))

It's workable, but the error messaging on the Comparison functions tend to be better for these situations.

Sorry for not replying to this comment!

I agree. In cases where the default message is not great I will add more context at the end:

assert.Assert(t, !strings.Contains(myText, "bar"), "got: %v", myText)
doer, ok := something.(Doer)
assert.Assert(t, ok, "got: %T", something)

Some examples of this in the intro docs would probably be a good addition.

It's a bit more typing, but hopefully negative assertions are relatively rare.

I think there are even advantages to this approach over of a Comparison. It encourages adding additional context to assertions, and keeps the interface small.

glenjamin commented 4 years ago

I think something along those lines should work.

Yep, that makes sense - thanks.

eloo commented 3 years ago

Hi,

i stumbled over this issue while trying to create something like a "NotDeepEqual" comparision. But sadly it looks like such a thing is not supported. So how should a check if two structs are not equal?

Thanks

dnephin commented 3 years ago

Hi @eloo , could you share more about your use case? What is the test trying to verify?

The reason I ask is because NotDeepEqual is a very weak check. It doesn't tell you much about the expected behaviour. Generally it is possible to write a test in a different way that gives you much better confidence by avoiding the use of Not comparisons.

eloo commented 3 years ago

Hi @dnephin my use case is that i want to try a replace function with random structs. And with a NotDeepEqual its pretty easy to check if the struct has been replaced or not.

RangelReale commented 8 months ago

Got this same problem while using is.Contains, I want "not contains", but there is no way of inverting the check. I'm converting from this testify code:

    require.NotContains(t, fields, "tag_id")
dkrieger commented 3 months ago

now that generics exist, could this be revisited? lacking a not modifier is a bizarre limitation, understandable based on language limitations and lib design decisions, but if it's possible to do generically now, I'd submit that there's no good reason not to.