e2e tests: use HaveField() for better error checking#14040
e2e tests: use HaveField() for better error checking#14040openshift-merge-robot merged 2 commits intocontainers:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Two commits, for ease of review (see commit message of 2nd commit for reason). Tested by hand-mucking with New: And, should it ever happen that the struct being tested does not even have an element with that name: |
3a8f870 to
4018b9b
Compare
test/e2e/generate_kube_test.go
Outdated
There was a problem hiding this comment.
This is really annoying:
Value for field 'Value' failed to satisfy matcher.
Expected
<*string | 0xc000504380>: blue
to equal
<string>: blue
I've tried BeEquivalent() and BeEqual() and a variety of other dead ends. Does anyone know if there's an incantation for dereferencing Value, or for telling HaveField() to just shut up and Do What I Mean?
There was a problem hiding this comment.
The below diff turned it green on my end:
diff --git a/test/e2e/generate_kube_test.go b/test/e2e/generate_kube_test.go
index f58b121f3070..31f2ea16e927 100644
--- a/test/e2e/generate_kube_test.go
+++ b/test/e2e/generate_kube_test.go
@@ -777,7 +777,8 @@ var _ = Describe("Podman generate kube", func() {
Expect(pod.Spec.DNSConfig.Searches).To(ContainElement("foobar.com"))
Expect(len(pod.Spec.DNSConfig.Options)).To(BeNumerically(">", 0))
Expect(pod.Spec.DNSConfig.Options[0]).To(HaveField("Name", "color"))
- Expect(pod.Spec.DNSConfig.Options[0]).To(HaveField("Value", "blue"))
+ s := "blue"
+ Expect(pod.Spec.DNSConfig.Options[0]).To(HaveField("Value", &s))
})
It("podman generate kube multiple container dns servers and options are cumulative", func() {Probably worth adding a helper function somewhere (utils package?) for such cases:
func StringToPointer(s string) *string {
return &s
}
Luap99
left a comment
There was a problem hiding this comment.
LGTM but there is one downside with this function.
When I change the struct field the compiler/IDE would complain that the test is using a field which no longer exists. Now I will run the tests to get a failure much later.
This is a very late followup to my ginkgo-improving work of 2021.
It has been stuck since December because it requires gomega 1.17,
which we've just enabled.
This commit is simply a copy-paste of a command I saved in
my TODO list many months ago:
sed -i -e 's/Expect(\([^ ]\+\)\.\([a-zA-Z0-9]\+\))\.To(Equal(/Expect(\1).To(HaveField(\"\2\", /' test/e2e/*_test.go
Signed-off-by: Ed Santiago <santiago@redhat.com>
Two for this error:
invalid indirect of pod.Spec.DNSConfig.Options[0]
...and one for a gofmt error (spaces).
Signed-off-by: Ed Santiago <santiago@redhat.com>
4018b9b to
a5aea8e
Compare
|
The Bad: new registry flake. The Good: the only files touched in this PR are in The Neutral: I have no opinion on the struct-change issue brought up by @Luap99, and will leave it for the team to weigh the tradeoffs. If struct renames are that common, please just close this without merging. Thanks @vrothberg for the pointer fix! |
|
@edsantiago This not a problem for me I just wanted to point that out. |
|
/lgtm |
This is a very late followup to my ginkgo-improving work of 2021.
It has been stuck since December because it requires gomega 1.17,
which we've just enabled.
This commit is simply a copy-paste of a command I saved in
my TODO list many months ago:
Signed-off-by: Ed Santiago santiago@redhat.com