Skip to content

Asserts on event-sender#3431

Merged
knative-prow-robot merged 8 commits into
knative:masterfrom
slinkydeveloper:sender_asserts
Jun 30, 2020
Merged

Asserts on event-sender#3431
knative-prow-robot merged 8 commits into
knative:masterfrom
slinkydeveloper:sender_asserts

Conversation

@slinkydeveloper
Copy link
Copy Markdown
Contributor

@slinkydeveloper slinkydeveloper commented Jun 29, 2020

Signed-off-by: Francesco Guardiani francescoguard@gmail.com

Part of #1845

Proposed Changes

  • This change introduces a way to perform asserts of event-sender result (eg status code)
  • Added 202 status code assert to channel data plane conformance test

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 29, 2020
@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 29, 2020
@knative-prow-robot knative-prow-robot added area/test-and-release Test infrastructure, tests or release approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 29, 2020
@slinkydeveloper
Asserts on event-sender
6172720 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@slinkydeveloper
Added todo
b196bb0 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@slinkydeveloper
Copyright
e65e71d 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@slinkydeveloper
More robust MatchStatusCode
ff1d2a4 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@slinkydeveloper
Update
429bc9c 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@slinkydeveloper
Set time
d3d5d62 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@knative-test-reporter-robot
Copy link
Copy Markdown

knative-test-reporter-robot commented Jun 30, 2020

The following jobs failed:

Test name Triggers Retries
pull-knative-eventing-integration-tests 0/3

Failed non-flaky tests preventing automatic retry of pull-knative-eventing-integration-tests:

test/conformance.TestBrokerTracing
test/conformance.TestChannelTracingWithReply
test/conformance.TestBrokerTracing/Channel-messaging.knative.dev/v1beta1
test/conformance.TestBrokerTracing/InMemoryChannel-messaging.knative.dev/v1beta1
test/conformance.TestChannelTracingWithReply/Channel-messaging.knative.dev/v1beta1
test/conformance.TestChannelTracingWithReply/InMemoryChannel-messaging.knative.dev/v1beta1

@slinkydeveloper
Updated to latest sdk-go, removed todo
18ab9cb 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
@devguyio
Copy link
Copy Markdown
Contributor

devguyio commented Jun 30, 2020

/assign

devguyio
devguyio reviewed Jun 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@devguyio devguyio left a comment

Choose a reason for hiding this comment

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

just a few comments, other than that lgtm

Comment thread test/lib/sender/matchers.go
)
}

func MatchInnerEvent(matchers ...cetest.EventMatcher) cetest.EventMatcher {
Copy link
Copy Markdown
Contributor

@devguyio devguyio Jun 30, 2020

Choose a reason for hiding this comment

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

Is this function used anywhere?

Copy link
Copy Markdown
Contributor Author

@slinkydeveloper slinkydeveloper Jun 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Not yet, but i suppose we'll need it for broker data-plane tests and other tests when sender actually expects an event back

Copy link
Copy Markdown
Member

@matzew matzew Jun 30, 2020

Choose a reason for hiding this comment

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

@lberk might want to take a look too ?

Comment thread test/lib/sender/matchers.go
Comment thread test/conformance/helpers/channel_message_modes_specversion_helper.go Outdated
Comment thread test/lib/sender/events.go
@slinkydeveloper
Applied suggestions
d20a74f 
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
devguyio
devguyio approved these changes Jun 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@devguyio devguyio left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 30, 2020
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Jun 30, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: devguyio, slinkydeveloper

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 merged commit 1ac1eb3 into knative:master Jun 30, 2020
@slinkydeveloper slinkydeveloper deleted the sender_asserts branch June 30, 2020 10:24
@slinkydeveloper slinkydeveloper mentioned this pull request Jun 30, 2020
Closed
@devguyio devguyio mentioned this pull request Jan 6, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matzew matzew matzew left review comments

@chaodaiG chaodaiG Awaiting requested review from chaodaiG

@steuhs steuhs Awaiting requested review from steuhs

+2 more reviewers

@mattmoor mattmoor mattmoor left review comments

@devguyio devguyio devguyio approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@devguyio devguyio

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@slinkydeveloper @knative-test-reporter-robot @devguyio @knative-prow-robot @matzew @mattmoor @googlebot