-
Notifications
You must be signed in to change notification settings - Fork 74
added or modified smoke test of four sources and gcp broker #1161
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: capri-xiyue The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
At first, I tried to avoid using methods like knative-gcp/test/e2e/lib/creation.go Line 75 in 53a1bd6
knative-gcp/test/e2e/lib/creation.go Line 83 in 53a1bd6
However, if I don't use that method, it will make the code kind of complex since I need to handle unhappy paths that the test fail and but we still need to delete the sources no matter what. Therefore, in the end, I still choose to use knative-gcp/test/e2e/lib/creation.go Line 75 in 53a1bd6
c.Tracker.AddObj(xxxx) . The side effect is that when the test passes, if you run go test -v, it will show similar logs that `tracker can't delete source, not found'
|
In addition, I found one bug of sources(source can still be ready even though the sink(k8s service) doesn't exist). Already filed another issue for that bug #1171 |
/test pull-google-knative-gcp-integration-tests |
lgtm from my side. |
/retest |
/lgtm |
/retest |
/unhold |
|
||
// WaitDeletionTime for deleting resources | ||
const ( | ||
WaitDeletionTime = 20 * time.Second |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, will put this one to 10, and then in the few places you need 20, just multiply?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you went with 20 everywhere. My only worry is that tests will last even longer... But I'm ok if you leave it as is
The following is the coverage report on the affected files.
|
/lgtm |
/test pull-google-knative-gcp-integration-tests |
1 similar comment
/test pull-google-knative-gcp-integration-tests |
The following jobs failed:
Automatically retrying due to test flakiness... |
Fixes #909 #1165
Proposed Changes
Release Note
Docs