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

Improve error message for test assertions #380

Merged
merged 3 commits into from
Aug 25, 2022

Conversation

porridge
Copy link
Member

@porridge porridge commented Aug 22, 2022

What this PR does / why we need it:

With a test case such as this:

[mowsiany@mowsiany kuttl]$ cat ns.yaml 
apiVersion: v1
kind: Namespace
status:
  phase: Active
[mowsiany@mowsiany kuttl]$ kubectl-kuttl errors --timeout 1 ns.yaml 

The result is:

Before this change:

resource matched of kind: /v1, Kind=Namespace
Error: error asserts not valid

which is not terribly useful for debugging..

After this change:

resource /v1, Kind=Namespace default (and 65 other resources) matched error assertion
Error: error asserts not valid

Getting information about the name of the matched resource was instrumental for solving the issue described in https://github.com/porridge/config-race-repro

See descriptions of individual commits for details.

Fixes: #371

We need [this commit](kubernetes-sigs/controller-runtime@3a037ef)
for the subsequent changes which invoke List() on the fake client.

Signed-off-by: Marcin Owsiany <porridge@redhat.com>
In the test cases for multiple objects we're using NewV1Pod, which returns an
actual Pod struct, rather than NewPod (which returns an Unstructured). This is
because the fake client is unable to List() pods which were supplied to builder
as Unstructureds. Instead it errors out with:

  item[0]: can't assign or convert unstructured.Unstructured into v1.Pod

Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Copy link
Member

@kensipe kensipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. thanks @porridge!

@kensipe
Copy link
Member

kensipe commented Aug 25, 2022

apologies @porridge. my delay in getting to PRs had me merge an earlier PR which had collisions on the dependency in go.mod. I will handle the conflict if you don't have time. Ready to merge after all is resolved

Signed-off-by: Ken Sipe <kensipe@gmail.com>
@kensipe kensipe merged commit 5f709d5 into kudobuilder:main Aug 25, 2022
@porridge porridge deleted the verbose-error-nice branch September 13, 2022 06:41
iblancasa pushed a commit to iblancasa/kuttl that referenced this pull request Nov 17, 2022
* Bump controller-runtime to pick up a fake client fix.

We need [this commit](kubernetes-sigs/controller-runtime@3a037ef)
for the subsequent changes which invoke List() on the fake client.

* Improve error message for error assertions and add test.

In the test cases for multiple objects we're using NewV1Pod, which returns an
actual Pod struct, rather than NewPod (which returns an Unstructured). This is
because the fake client is unable to List() pods which were supplied to builder
as Unstructureds. Instead it errors out with:

  item[0]: can't assign or convert unstructured.Unstructured into v1.Pod

Co-authored-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Signed-off-by: Israel Blancas <iblancasa@gmail.com>
This pull request was closed.
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

Successfully merging this pull request may close these issues.

Show content of unexpected resources
3 participants