onsi / gomega

Ginkgo's Preferred Matcher Library
http://onsi.github.io/gomega/
MIT License
2.19k stars 284 forks source link

HaveField buffers stale fields #787

Closed pohly closed 1 month ago

pohly commented 1 month ago

I am using the following code to ensure that list.Items[] has exactly one element and that element has an empty Spec.Devices:

            emptySlice := gomega.HaveField("Spec.Devices", gomega.BeEmpty())
            gomega.Eventually(ctx, listSlices).WithTimeout(time.Minute).Should(gomega.HaveField("Items", gomega.ContainElements(emptySlice)))

My code under test incorrectly deleted all items (= list.Items[] empty). But the failure message then contained information about some previous item:

  [FAILED] Timed out after 60.000s.
  Value for field 'Items' failed to satisfy matcher.
  Expected
      <[]v1alpha3.ResourceSlice | len:0, cap:0>: nil
  to contain elements
      <[]*matchers.HaveFieldMatcher | len:1, cap:1>: [
          {
              Field: "Spec.Devices",
              Expected: 
                  {},
              extractedField: <[]v1alpha3.Device | len:128, cap:170>[
                  {
                      Name: "dev-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx0000",
                      Basic: {
                          Attributes: {
                              "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.example.com/xxxxxxxxxxxxxxxxxxxxxxxxxxxx0010": {
                                  IntValue: nil,
                                  BoolValue: nil,
                                  StringValue: "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv",
                                  VersionValue: nil,
                              },
...

  Gomega truncated this representation as it exceeds 'format.MaxLength'.
  Consider having the object provide a custom 'GomegaStringer' representation
  or adjust the parameters in Gomega's 'format' package.
onsi commented 1 month ago

ah. subtle. so the outer HaveField matcher is being polled and when it successfully fins .Items it passes the value of that field to ContainElements which then passes it to the inner HaveField. when the code deleted the items the out HaveField matcher failed early and did nto pass nil in to ContainElements.

The problem is that HaveField, the inner HaveField in particular, memoizes the extractedField and expectedMatcher so that it can render failure messages without having to redo the expensive (?) logic of extracting the field. I can remove this optimization, though, and then you won't get this stale and confusing detail in the output.

onsi commented 1 month ago

done. will cut a release soon. i expect this change to be stable and the performance impact to be limited but please let me know if you see anything suspicious!