Skip to content

Conversation

@devguyio
Copy link
Contributor

@devguyio devguyio commented Sep 7, 2021

This is a dummy PR to show case a failure in pkg/apis/eventing/v1/roundtip_test.go I faced while working on #5715 .

Root Causing

  • If I change the added field in TriggerSpec from Tags *[]string to Tags []string, the roundtrip tests succeed.
  • Root causing is initially clouded by the test crashing when the failure path is detected while trying to log the failure using cmp because of an unexported field in {*v1.Trigger}.Spec.Subscriber.URI.User.username .
    --- FAIL: TestEventingRoundTripTypesToJSON/eventing.knative.dev.v1.Trigger (0.01s)
panic: cannot handle unexported field at {*v1.Trigger}.Spec.Subscriber.URI.User.username:
	"net/url".Userinfo
consider using a custom Comparer; if you control the implementation of type, you can also consider using an Exporter, AllowUnexported, or cmpopts.IgnoreUnexported [recovered]
	panic: cannot handle unexported field at {*v1.Trigger}.Spec.Subscriber.URI.User.username:
	"net/url".Userinfo
consider using a custom Comparer; if you control the implementation of type, you can also consider using an Exporter, AllowUnexported, or cmpopts.IgnoreUnexported

goroutine 12 [running]:
testing.tRunner.func1.2(0xd71e80, 0xc0002b3390)
	/usr/lib/golang/src/testing/testing.go:1143 +0x332
testing.tRunner.func1(0xc0001b9680)
	/usr/lib/golang/src/testing/testing.go:1146 +0x4b6
panic(0xd71e80, 0xc0002b3390)
	/usr/lib/golang/src/runtime/panic.go:965 +0x1b9
github.com/google/go-cmp/cmp.validator.apply(0xc0000fa000, 0xd71e80, 0xc000481cb0, 0x1b8, 0xd71e80, 0xc00051a360, 0x1b8)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/options.go:244 +0x6b0
github.com/google/go-cmp/cmp.(*state).tryOptions(0xc0000fa000, 0xfda388, 0xd71e80, 0xd71e80, 0xc000481cb0, 0x1b8, 0xd71e80, 0xc00051a360, 0x1b8, 0x47b4a5)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:303 +0x132
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc92d0, 0xc000272d00)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:258 +0x2f2
github.com/google/go-cmp/cmp.(*state).compareStruct(0xc0000fa000, 0xfda388, 0xdf7c40, 0xdf7c40, 0xc000481cb0, 0x199, 0xdf7c40, 0xc00051a360, 0x199)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:427 +0x685
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc9240, 0xc0002b0800)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:286 +0xe38
github.com/google/go-cmp/cmp.(*state).comparePtr(0xc0000fa000, 0xfda388, 0xdc0100, 0xdc0100, 0xc000520da0, 0x196, 0xdc0100, 0xc000521100, 0x196)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:579 +0x333
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc92d0, 0xc000272c00)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:292 +0xcbe
github.com/google/go-cmp/cmp.(*state).compareStruct(0xc0000fa000, 0xfda388, 0xe54e20, 0xe54e20, 0xc000520d80, 0x199, 0xe54e20, 0xc0005210e0, 0x199)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:427 +0x685
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc9240, 0xc0002b07c0)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:286 +0xe38
github.com/google/go-cmp/cmp.(*state).comparePtr(0xc0000fa000, 0xfda388, 0xe15400, 0xe15400, 0xc0005a3178, 0x196, 0xe15400, 0xc0005a3cd8, 0x196)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:579 +0x333
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc92d0, 0xc000272a00)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:292 +0xcbe
github.com/google/go-cmp/cmp.(*state).compareStruct(0xc0000fa000, 0xfda388, 0xdddf20, 0xdddf20, 0xc0005a3170, 0x199, 0xdddf20, 0xc0005a3cd0, 0x199)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:427 +0x685
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc92d0, 0xc000272800)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:286 +0xe38
github.com/google/go-cmp/cmp.(*state).compareStruct(0xc0000fa000, 0xfda388, 0xe1fac0, 0xe1fac0, 0xc0005a3158, 0x199, 0xe1fac0, 0xc0005a3cb8, 0x199)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:427 +0x685
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc92d0, 0xc000272200)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:286 +0xe38
github.com/google/go-cmp/cmp.(*state).compareStruct(0xc0000fa000, 0xfda388, 0xe119c0, 0xe119c0, 0xc0005a3040, 0x199, 0xe119c0, 0xc0005a3ba0, 0x199)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:427 +0x685
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc9240, 0xc0002b0280)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:286 +0xe38
github.com/google/go-cmp/cmp.(*state).comparePtr(0xc0000fa000, 0xfda388, 0xe853a0, 0xe853a0, 0xc0005a3040, 0x16, 0xe853a0, 0xc0005a3ba0, 0x16)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:579 +0x333
github.com/google/go-cmp/cmp.(*state).compareAny(0xc0000fa000, 0xfc54c0, 0xc0002b0240)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:292 +0xcbe
github.com/google/go-cmp/cmp.Diff(0xe853a0, 0xc0005a3040, 0xe853a0, 0xc0005a3ba0, 0x0, 0x0, 0x0, 0xfb99c0, 0xc0005a3ba0)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/github.com/google/go-cmp/cmp/compare.go:119 +0xab
k8s.io/apimachinery/pkg/util/diff.legacyDiff(...)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/k8s.io/apimachinery/pkg/util/diff/diff.go:51
k8s.io/apimachinery/pkg/util/diff.ObjectReflectDiff(...)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/k8s.io/apimachinery/pkg/util/diff/diff.go:72
k8s.io/apimachinery/pkg/api/apitesting/roundtrip.roundTrip(0xc0001b9680, 0xc000241dc0, 0xfc8e20, 0xc0004b6c30, 0xfb99c0, 0xc0005a3040)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go:361 +0x1b0a
k8s.io/apimachinery/pkg/api/apitesting/roundtrip.roundTripOfExternalType(0xc0001b9680, 0xc000241dc0, 0xc000241dc0, 0xfb42a0, 0xc00000ee58, 0xc0001b8a80, 0x3, 0x4, 0xfc8e50, 0xc000033d60, ...)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go:282 +0x265
k8s.io/apimachinery/pkg/api/apitesting/roundtrip.roundTripSpecificKind(0xc0001b9680, 0xe9f6ab, 0x14, 0xe9406d, 0x2, 0xd0df25, 0x7, 0xc000241dc0, 0xc000241dc0, 0xfb42a0, ...)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go:199 +0x186
k8s.io/apimachinery/pkg/api/apitesting/roundtrip.RoundTripSpecificKindWithoutProtobuf(...)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go:181
knative.dev/pkg/apis/testing/roundtrip.ExternalTypesViaJSON.func1(0xc0001b9680)
	/home/abd4lla/Workspace/redhat/src/knative.dev/eventing/vendor/knative.dev/pkg/apis/testing/roundtrip/roundtrip.go:109 +0x11e
testing.tRunner(0xc0001b9680, 0xc0003dbf80)
	/usr/lib/golang/src/testing/testing.go:1193 +0xef
created by testing.(*T).Run
	/usr/lib/golang/src/testing/testing.go:1238 +0x2b3

Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
@knative-prow-robot knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2021
@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Sep 7, 2021
@knative-prow-robot knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 7, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: devguyio

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 7, 2021
@devguyio devguyio changed the title [WIP] [DUMMY] Why are roundtrip tests fail when adding this field? [WIP] [DUMMY] Why are roundtrip tests failing when adding this field? Sep 7, 2021
@devguyio
Copy link
Contributor Author

devguyio commented Sep 7, 2021

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 7, 2021
@devguyio devguyio changed the title [WIP] [DUMMY] Why are roundtrip tests failing when adding this field? [DUMMY] Why are roundtrip tests failing when adding this field? Sep 7, 2021
@knative-prow-robot knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 7, 2021
Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
@knative-prow-robot
Copy link
Contributor

@devguyio: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Rerun command
pull-knative-eventing-unit-tests 4366a56 link /test pull-knative-eventing-unit-tests
pull-knative-eventing-build-tests 4366a56 link /test pull-knative-eventing-build-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@devguyio devguyio closed this Sep 8, 2021
@devguyio devguyio deleted the roundtrip-error branch September 8, 2021 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants