Simplify test and improve test logging #76
Conversation
kubevirt-bot
added
the
dco-signoff: yes
|
/test pull-kvp-functional-test |
| err = StartVirtualMachine(*kvClient, namespace.Name, vm.Name) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| err = WaitForDataVolumePhase(*kvClient, namespace.Name, cdiv1.Succeeded, vmSpec.Spec.DataVolumeTemplates[0].Name) |
There was a problem hiding this comment.
With GC can you be sure the DV is still around? Maybe wait for the VM to have started, the DV will be guaranteed to be completed at that point.
There was a problem hiding this comment.
well, that means I need to verify the whole test suite! and that is a bigger task. Maybe I can even move the verification to StartVirtualMachine
There was a problem hiding this comment.
but this is a task for another PR.
BTW: is GC enabled by default in all tests?
There was a problem hiding this comment.
I currently don't think the default is GC, but likely it will be soon.
| @@ -45,138 +65,121 @@ var _ = Describe("[smoke] VM Backup", func() { | |||
| r.Dump(CurrentGinkgoTestDescription().Duration) | |||
| } | |||
|
|
|||
| // Deleting the backup also deletes all restores, volume snapshots etc. | |||
| err := DeleteBackup(timeout, backupName, r.BackupNamespace) | |||
| if err != nil { | |||
There was a problem hiding this comment.
Maybe better expressed as Expect(err).ToNot(HaveOccurred()) that will print the error message if an error occurred.
There was a problem hiding this comment.
Then, wen it fails it does not try to remove VM. After one or two such failures other tests may fail due too not enough resources.
Maybe I can change it to new gingko Defer(), but this is also a task for another PR.
There was a problem hiding this comment.
Gotcha, yeah if you are worried about the cleanup make sure you either clean it up in an AfterEach or use a defer.
There was a problem hiding this comment.
this is AfterEach(), and I think failing an Expect() would not run next cleanups.
Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>
brybacki
force-pushed
the
vm-bkup-test-log
branch
from
d3122e5
to
87d799a
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brybacki 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 |
kubevirt-bot
added
the
approved
Signed-off-by: Bartosz Rybacki brybacki@redhat.com
What this PR does / why we need it:
Test failed:
https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_kubevirt-velero-plugin/75/pull-kvp-functional-test/1562787451645202432
This PR improves logging for troubleshooting.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
Release note: