Conversation
knative-prow-robot
added
the
size/S
knative-prow-robot
added
the
area/test-and-release
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: devguyio The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report
@@ Coverage Diff @@
## master #4353 +/- ##
=======================================
Coverage 80.81% 80.81%
=======================================
Files 288 288
Lines 8001 8001
=======================================
Hits 6466 6466
Misses 1147 1147
Partials 388 388 Continue to review full report at Codecov.
|
|
Awesome findings @devguyio. Could you also please remove those lines so the test will not skip if missing events are found: https://github.com/knative/eventing/blob/832cea7/test/upgrade/probe_test.go#L55-L56 And revert those to 10msec https://github.com/knative/eventing/blob/2a052db/test/upgrade/prober/prober.go#L31-L33 |
|
/test pull-knative-eventing-upgrade-tests |
|
#3260 can be revived after this PR lands or defaults for IMC are changed. |
|
@devguyio: PR needs rebase. DetailsInstructions 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. |
knative-prow-robot
added
the
needs-rebase
| configFilename = "config.toml" | ||
| watholaEventNs = "com.github.cardil.wathola" | ||
| healthEndpoint = "/healthz" | ||
| brokerClass = "MTChannelBasedBroker" |
There was a problem hiding this comment.
Do you think it makes sense to make this configurable through env var too? This way the file can be more flexible in terms of which broker class to test.
There was a problem hiding this comment.
We can change this to defaultBrokerCalss, then check an env var like UPGRADE_TEST_BROKER_CLASS in deployBroker.
|
/assign |
|
I am guessing this PR needs to be rebased or reapplied to the latest code since the recent PR about upgrade test change this file significantly. |
|
@devguyio do you plan to go ahead with this? |
| numRetries := int32(5) | ||
| backoff := eventingduckv1beta1.BackoffPolicyLinear | ||
| p.client.CreateBrokerV1Beta1OrFail(brokerName, | ||
| resources.WithBrokerClassForBrokerV1Beta1(brokerClass), |
There was a problem hiding this comment.
This can be moved to the upper level after #4616
|
@devguyio: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
Closing, seems to be no longer relevant |
5 participants
Proposed Changes