-
Notifications
You must be signed in to change notification settings - Fork 261
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
Add event message verification to cronjob e2e test #698
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,10 @@ | |
package e2e | ||
|
||
import ( | ||
"fmt" | ||
"strings" | ||
"testing" | ||
"time" | ||
|
||
"gotest.tools/assert" | ||
|
||
|
@@ -56,7 +59,57 @@ func TestSourceCronJob(t *testing.T) { | |
jpSinkRefNameInSpec := "jsonpath={.spec.sink.ref.name}" | ||
out, err := test.getResourceFieldsWithJSONPath("cronjobsource", "testcronjobsource2", jpSinkRefNameInSpec) | ||
assert.NilError(t, err) | ||
assert.Equal(t, out, "testsvc1") | ||
//assert.Equal(t, out, "testsvc1") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Until test-infrastructure gets update, this PR will not pass the integration test. |
||
assert.Check(t, util.ContainsAllIgnoreCase(out, "testsvc1")) | ||
|
||
t.Log("verify event messages from cronjob source") | ||
mymsg := "This is a message from cronjob." | ||
test.serviceCreateEventDisplay(t, r, "displaysvc") | ||
test.cronJobSourceCreate(t, r, "testcronjobsource3", "*/1 * * * *", mymsg, "svc:displaysvc") | ||
results := test.kn.Run("source", "cronjob", "describe", "testcronjobsource3") | ||
r.AssertNoError(results) | ||
out, err = kubectl{test.kn.namespace}.Run("get", "pod", "-l", "sources.eventing.knative.dev/cronJobSource=testcronjobsource3", "-o", "yaml") | ||
t.Log(out) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this necessary ? Maybe better assert on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for debug purpose. will remove in the latest version. |
||
err = test.verifyEventDisplayLogs(t, "displaysvc", "This is a message from cronjob") | ||
assert.NilError(t, err) | ||
} | ||
|
||
func (test *e2eTest) verifyEventDisplayLogs(t *testing.T, svcname string, message string) error { | ||
var ( | ||
retries int | ||
err error | ||
out string | ||
) | ||
|
||
selectorStr := fmt.Sprintf("serving.knative.dev/service=%s", svcname) | ||
for retries < 5 { | ||
out, err = kubectl{test.kn.namespace}.Run("logs", "-l", selectorStr, "-c", "user-container") | ||
if err != nil { | ||
t.Logf("error happens at kubectl logs -l %s -c -n %s: %v", selectorStr, test.kn.namespace, err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should return here with the error. |
||
} else if err == nil && strings.Contains(out, message) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. err is nil for sure when you reach this (see condition before) |
||
break | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not return immediately ? then you don't have to check for nrRetries after the loop. |
||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can remove the else if you return early |
||
t.Logf("return from kubectl logs -l %s -n %s: %s", selectorStr, test.kn.namespace, out) | ||
out, err = kubectl{test.kn.namespace}.Run("get", "pods") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if err ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for debug purpose. will remove in the latest version. |
||
t.Log(out) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why log here ? |
||
out, err = kubectl{test.kn.namespace}.Run("logs", "-l", "sources.eventing.knative.dev/cronJobSource=testcronjobsource3") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if err ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for debug purpose. will remove in the latest version. |
||
t.Log(out) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why log ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for debug purpose. will remove in the latest version. |
||
} | ||
retries++ | ||
time.Sleep(2 * time.Minute) | ||
} | ||
|
||
if retries == 5 { | ||
return fmt.Errorf("Expected log incorrect after retry 5 times. Expecting to include:\n%s\n Instead found:\n%s\n", message, out) | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. else not needed |
||
return nil | ||
} | ||
} | ||
|
||
func (test *e2eTest) serviceCreateEventDisplay(t *testing.T, r *KnRunResultCollector, serviceName string) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this not a public There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's same as
Although it's not public, but you can use this function under any e2e tests. |
||
out := test.kn.Run("service", "create", serviceName, "--image", EventDisplayImage) | ||
r.AssertNoError(out) | ||
assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "service", serviceName, "creating", "namespace", test.kn.namespace, "ready")) | ||
} | ||
|
||
func (test *e2eTest) cronJobSourceCreate(t *testing.T, r *KnRunResultCollector, sourceName string, schedule string, data string, sink string) { | ||
|
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.
please remove
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.
for debug purpose. will remove in the latest version.