Skip to content

reflect: DeepEqual fails with matching key/value pairs in maps #44154

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

Closed
codeblooded opened this issue Feb 7, 2021 · 6 comments
Closed

reflect: DeepEqual fails with matching key/value pairs in maps #44154

codeblooded opened this issue Feb 7, 2021 · 6 comments

Comments

@codeblooded
Copy link

codeblooded commented Feb 7, 2021

What version of go are you using?

I noticed the issue when running code commit c8bd801, but I can confirm this problem exists on Go Playgrounds as well (see link in next section).

What did you do?

I created two maps with equal keys and values. Then, I compared them using reflect.DeepEqual.

https://play.golang.org/p/OjDV7PWt1SY

What did you expect to see?

Map values are deeply equal when all of the following are true: they are both nil or both non-nil, they have the same length, and either they are the same map object or their corresponding keys (matched using Go equality) map to deeply equal values. (https://golang.org/pkg/reflect/#DeepEqual).

The maps are structurally different, but they have equal keys/values and length. I would expect them to be equal.

What did you see instead?

The maps are not structurally identical, so reflect.DeepEqual returns false.

@codeblooded codeblooded changed the title reflect: DeepEqual fails with matching key/values pairs in maps reflect: DeepEqual fails with matching key/value pairs in maps Feb 7, 2021
@fraenkel
Copy link
Contributor

fraenkel commented Feb 8, 2021

If you change your code to either

expected := &MissingComponents{map[string]int{
		DefaultClientPool: 4,
		DefaultServerPool: 1,
		DefaultDriverPool: 1,
	}}

or
fmt.Printf("Do they match? %v\n", reflect.DeepEqual(actual.NodeCountByPool, expected))
you end up with true. Comparing a struct to a map will never be equal.

@ianlancetaylor
Copy link
Member

Closing because this is working as expected. Please comment if you disagree.

@codeblooded
Copy link
Author

Sorry, I extracted some of the logic into a playground and mistyped. Wasn't going to share the entire project. In the actual project, I'm not comparing a struct to a map. This is actually failing in a unit test using the ginkgo/gomega frameworks. According to their docs, gomega uses reflect.DeepEqual under the hood:

For the following test case:

It("sets the number of nodes missing from each pool", func() {
	actualReturn = CheckMissingPods(test, allRunningPods)
	Expect(actualReturn.NodeCountByPool).To(Equal(
		map[string]int{
			DefaultClientPool: 0,
			DefaultDriverPool: 0,
			DefaultServerPool: 0,
		},
	))
})

This is the failed expectation we are seeing:

• Failure [0.000 seconds]
CheckMissingPods some of pods from the current load test is running [It] sets the number of nodes missing from each pool 
~/benreed/grpc/test-infra/status/missing_pods_test.go:126

  Expected
      <map[string]int | len:4>: {
          "__default_pool (drivers)": 0,
          "__default_pool (servers)": 0,
          "workers-8core": 4,
          "__default_pool (clients)": 0,
      }
  to equal
      <map[string]int | len:4>: {
          "workers": 4,
          "__default_pool (clients)": 0,
          "__default_pool (drivers)": 0,
          "__default_pool (servers)": 0,
      }

 ~/benreed/grpc/test-infra/status/missing_pods_test.go:128

I will need to dive in to see if this is some sort of issue with gomega, since it did not replicate in the playground. I'll comment here if I cannot see a way gomega could cause this failure, and I'll update what I find regarding the framework in onsi/gomega#412.

@ianlancetaylor
Copy link
Member

In the failure message one of the maps has the key "workers-8core" and the other has the key "workers".

@codeblooded
Copy link
Author

@ianlancetaylor Thank you. I don't see how I missed that. I haven't felt well the last two days, and I knew I'm feeling crazy tired… guess I just thought I was still functional. Sorry to waste your time.

@ianlancetaylor
Copy link
Member

No worries.

@golang golang locked and limited conversation to collaborators Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants