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

Equal, BeEquivalentTo, and BeComparableTo false positive on function types #570

Open
armsnyder opened this issue Aug 1, 2022 · 3 comments

Comments

@armsnyder
Copy link

Hi. 👋 I'm getting a false positive when I compare two structs that have a function-type field.

This is using gomega version v1.20.0.

Here's a minimal example:

func TestCompareFunctions_Minimal(t *testing.T) {
	fn := func() {}
	NewWithT(t).Expect(fn).To(Equal(fn))
}
=== RUN   TestCompareFunctions_Minimal
    builder_test.go:217: 
        Expected
            <func()>: 0x12cecc0
        to equal
            <func()>: 0x12cecc0
--- FAIL: TestCompareFunctions_Minimal (0.00s)

And a more complete example to illustrate something closer to a real use case:

Click to expand...
func TestCompareFunctions_Complete(t *testing.T) {
	type Strategy func()

	type Thing struct {
		Strategy Strategy
	}

	var NoOp Strategy = func() {}

	thingA := Thing{Strategy: NoOp}
	thingB := Thing{Strategy: NoOp}

	for _, matcher := range []types.GomegaMatcher{
		Equal(thingB),
		BeEquivalentTo(thingB),
		BeComparableTo(thingB),
	} {
		t.Run("", func(t *testing.T) {
			NewWithT(t).Expect(thingA).To(matcher)
		})
	}
}
=== RUN   TestCompareFunctions_Complete
=== RUN   TestCompareFunctions_Complete/#00
    builder_test.go:238: 
        Expected
            <saga_test.Thing>: {Strategy: 0x12cee40}
        to equal
            <saga_test.Thing>: {Strategy: 0x12cee40}
=== RUN   TestCompareFunctions_Complete/#01
    builder_test.go:238: 
        Expected
            <saga_test.Thing>: {Strategy: 0x12cee40}
        to be equivalent to
            <saga_test.Thing>: {Strategy: 0x12cee40}
=== RUN   TestCompareFunctions_Complete/#02
    builder_test.go:238: 
          saga_test.Thing{
        - 	Strategy: ⟪0x012cee40⟫,
        + 	Strategy: ⟪0x012cee40⟫,
          }
--- FAIL: TestCompareFunctions_Complete (0.00s)
    --- FAIL: TestCompareFunctions_Complete/#00 (0.00s)

    --- FAIL: TestCompareFunctions_Complete/#01 (0.00s)

    --- FAIL: TestCompareFunctions_Complete/#02 (0.00s)
@onsi
Copy link
Owner

onsi commented Aug 1, 2022

Hey there - Equal and BeEquivalentTo both use reflect.DeepEqual to assert equality and reflect.DeepEqual does not support testing for function equality (one could imagine dropping down to pointer-equality instead but that isn't what DeepEqual has opted to do). Gomega uses DeepEqual to avoid reimplementing something as foundational as deep equality (there are several nuances that go into Go's implementation DeepEqual).

BeComparableTo is a relatively new addition to Gomega that wraps go-cmp. You might be able to hack together so go-cmp options to support testing for function equality in some way (or at least ignoring it) but it's not something i've personally tried to do.

If your goal is to make assertions on the non-function parts of a struct you can use Gomega's gstruct and/or Gomega's HaveField and And/Or matchers to construct a quick custom matcher that for your struct that ignores the function fields.

Sorry there isn't a more straightforward answer :/

@armsnyder
Copy link
Author

Thanks for the details! I was able to get what I want using a go-cmp option. It's a little hacky, but it works.

func TestCompareFunctions_Complete(t *testing.T) {
	type Strategy func()

	type Thing struct {
		Strategy Strategy
	}

	var Strategy1 Strategy = func() {}
	var Strategy2 Strategy = func() {}

	thingA := Thing{Strategy: Strategy1}
	thingB := Thing{Strategy: Strategy1}
	thingC := Thing{Strategy: Strategy2}

	strategyCompareOption := cmp.Transformer("strategy", func(s Strategy) string {
		return fmt.Sprintf("%p", s)
	})

	g := NewWithT(t)

	g.Expect(thingA).To(BeComparableTo(thingB, strategyCompareOption))
	g.Expect(thingA).NotTo(BeComparableTo(thingC, strategyCompareOption))
}
=== RUN   TestCompareFunctions_Complete
--- PASS: TestCompareFunctions_Complete (0.00s)
PASS

@onsi
Copy link
Owner

onsi commented Aug 1, 2022

Makes sense! Glad you were able to find a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants