Print Remediation Time in E2E Tests #111
Conversation
Checking remediation time in e2e could help us identify slow remediations and identify if the pr changes impact the remediations time. In addition there is no need to test pod creation time
|
Skipping CI for Draft Pull Request. |
razo7
force-pushed
the
print-remediation-time
branch
from
7e98cb7
to
d58949c
Compare
|
/test 4.14-openshift-e2e |
test/e2e/far_e2e_test.go
Outdated
| AfterEach(func() { | ||
| if len(remediationTimes) == 2 { |
There was a problem hiding this comment.
I didn't find a better way to check that both tests are over. AfterAll didn't work for me (or I didn't use it properly).
There was a problem hiding this comment.
If I read [1] correctly, AfterAll can only be used in Ordered containers, which cannot be randomized though, so considering the latest changes, it doesn't seem a viable option.
[1] https://onsi.github.io/ginkgo/#setup-in-ordered-containers-beforeall-and-afterall
| DeferCleanup(cleanupTestedResources, pod) | ||
|
|
||
| // set the node as "unhealthy" by disabling kubelet | ||
| makeNodeUnready(selectedNode) | ||
|
|
||
| startTime = time.Now() |
There was a problem hiding this comment.
I wonder if this is accurate enough. It is taking into account the time to create the object (which is variable) other than the actual initial instant of the remediation.
There was a problem hiding this comment.
It is taking into account the time to create the object (which is variable) other than the actual initial instant of the remediation.
Are you suggesting just "storing" the time after creating far CR (after line 131) instead of before the creation? I believe it is negligible, but I don't mind doing so
There was a problem hiding this comment.
I was thinking of getting the cr creation time, but you're right that the difference should be minimal
|
/test 4.15-openshift-e2e |
|
/lgtm |
test/e2e/far_e2e_test.go
Outdated
| AfterEach(func() { | ||
| if len(remediationTimes) == 2 { |
test/e2e/far_e2e_test.go
Outdated
| averageTimeDuration := 0.0 | ||
| for index, remTime := range remediationTimes { | ||
| averageTimeDuration += remTime.Seconds() | ||
| fmt.Printf("\nRemediation time #%d: %s\n", index+1, remTime) |
There was a problem hiding this comment.
please use the logger log instead of fmt.Print
There was a problem hiding this comment.
I see that I have used fmt.Print in more places in the e2e test file, so I will update them all.
razo7
force-pushed
the
print-remediation-time
branch
2 times, most recently
from
f6886d8
to
0e20157
Compare
AfterSuite is run after the whole suite is finished
razo7
force-pushed
the
print-remediation-time
branch
from
0e20157
to
2d428da
Compare
|
/retest |
test/e2e/far_e2e_test.go
Outdated
| @@ -134,27 +135,28 @@ var _ = Describe("FAR E2e", func() { | |||
| When("running FAR to reboot two nodes", func() { | |||
| It("should successfully remediate the first node", func() { | |||
| checkRemediation(nodeName, nodeBootTimeBefore, pod) | |||
| remediationTimes = append(remediationTimes, time.Since(startTime)) | |||
| remediationsTime = append(remediationsTime, time.Since(startTime)) | |||
There was a problem hiding this comment.
hm, I think the old name was correct... 🤔
multiple times of a single remediation
this now sounds like overall time of all remediations
s/remediationsTime/remediationTimes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: razo7, slintes 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 |
Checking remediation time in e2e could help us identify slow remediations and identify if the PR changes impact the remediation time. We also calculate their average time for better understanding.