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
Bug 1882268: Add snapshots integration test #6314
Conversation
/retest |
2 similar comments
/retest |
/retest |
}); | ||
|
||
afterAll(() => { | ||
deleteResource(testVM); |
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.
shouldn't we ensure that the snapshot gets deleted, for example when the status doe not get ready in time? We could use the leakableResources construct like other tests do
|
||
// TODO: add restore testing (right now not available in the API) | ||
|
||
await click(editVMSnapshotsView.kebab); |
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.
can we use listViewAction instead?
/retest |
1 similar comment
/retest |
6fa1779
to
781c276
Compare
fixed suggestions @suomiy |
}); | ||
|
||
it( | ||
'ID() create, restore and delete a snapshot', |
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.
@yaacov do we have test ID
for this PR?
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.
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.
It needs to create the test case on polarion, if you can't do it in polarion, then I can help creating it.
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.
@gouyang I'm not sure how to do it.. we need cases for CDI Upload, and Snapshots
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.
https://polarion.engineering.redhat.com/polarion/#/project/CNV/workitem?id=CNV-4717.
Please use ID(CNV-4717)
.
import { $, $$ } from 'protractor'; | ||
|
||
export const addSnapshotBtn = $('#add-snapshot'); | ||
export const saveSnapshotBtn = $('#confirm-action'); |
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.
these 2 are not used anymore
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.
addSnapshotBtn
is used
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.
but snapshotKebab is not
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.
removed
// wait for snapshot to be ready | ||
await browser.wait( | ||
until.textToBePresentInElement( | ||
editVMSnapshotsView.getStatusElement('test-snapshot'), |
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.
we could make a constant out of the name
9782bec
to
f4ca9e1
Compare
/retest |
f4ca9e1
to
54b0cf4
Compare
added test ID. |
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/bugzilla refresh |
@glekner: This pull request references Bugzilla bug 1882268, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions 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. |
/hold - #6736 removed them for 4.6 |
@glekner: The following test failed, say
Full PR test history. Your PR dashboard. Instructions 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. |
54b0cf4
to
607b806
Compare
607b806
to
fc6784c
Compare
Added Restoring a snapshot to the Test. @suomiy can you lgtm |
can you please include this test in the CI? |
ping @glekner |
fc6784c
to
4d98ef1
Compare
/retest |
28ae687
to
f3a0e6a
Compare
f3a0e6a
to
9072bf0
Compare
9072bf0
to
29ed70d
Compare
/hold cancel |
/test analyze |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: glekner, suomiy, yaacov 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 |
@glekner: All pull requests linked via external trackers have merged: Bugzilla bug 1882268 has been moved to the MODIFIED state. In response to this:
Instructions 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. |
No description provided.