Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Cypress: Add Volume cases #331

Merged
merged 10 commits into from
Nov 9, 2022
Merged

Cypress: Add Volume cases #331

merged 10 commits into from
Nov 9, 2022

Conversation

DaiYuzeng
Copy link
Contributor

WuJun2016
WuJun2016 reviewed May 9, 2022
View reviewed changes
cypress/testcases/volumes.spec.ts Outdated
* 3. Volume should still show in Volume list
* 4. Volume should delete
*/
describe.only('Delete volume that was attached to VM but now is not', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete '.only'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@DaiYuzeng DaiYuzeng marked this pull request as draft May 9, 2022 08:28
@noahgildersleeve
Copy link
Contributor

@DaiYuzeng This is marked as a draft PR. Is it ready for merging?

@DaiYuzeng DaiYuzeng force-pushed the volume branch 2 times, most recently from dddab4e to c0d416c Compare June 6, 2022 06:03
@DaiYuzeng DaiYuzeng marked this pull request as ready for review June 6, 2022 06:05
@DaiYuzeng DaiYuzeng requested a review from WuJun2016 June 6, 2022 06:05
@DaiYuzeng
Copy link
Contributor Author

DaiYuzeng commented Jun 6, 2022 •
edited
Loading

@noahgildersleeve Please take a look, thanks!

This PR seems to be conflict with #359 and I am going to switch it to be a draft PR.

@DaiYuzeng DaiYuzeng marked this pull request as draft June 8, 2022 01:40
TachunLin
TachunLin requested changes Jun 9, 2022
View reviewed changes
Copy link
Contributor

@TachunLin TachunLin left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified running volume.spec.ts test suite, some of the cases failed
Not sure it's related to network latency or dependency from other test

  • FAILED Create image from Volume
  • PASS Create volume root disk VM Image Form
  • FAILED Delete volume that was attached to VM but now is not

image

@n313893254
Copy link
Contributor

Hi, @DaiYuzeng. The VM PR has merged, Please let me know if it caused any issues.

@DaiYuzeng
Copy link
Contributor Author

Hi, @DaiYuzeng. The VM PR has merged, Please let me know if it caused any issues.

Thanks for the changes!
I am going to resolve the conflicts and then do some tests.
But I am working on an issue of Harvester Node Driver, I'll work on this issue 2/3 days later.

WuJun2016
WuJun2016 suggested changes Nov 2, 2022
View reviewed changes
cypress/constants/constants.ts Show resolved Hide resolved
cypress/constants/constants.ts Show resolved Hide resolved
cypress/pageobjects/image.po.ts Show resolved Hide resolved
@DaiYuzeng DaiYuzeng marked this pull request as ready for review November 3, 2022 09:24
@DaiYuzeng DaiYuzeng requested a review from WuJun2016 November 3, 2022 09:24
@DaiYuzeng DaiYuzeng marked this pull request as draft November 3, 2022 09:27
@DaiYuzeng DaiYuzeng marked this pull request as ready for review November 4, 2022 07:30
@DaiYuzeng
Copy link
Contributor Author

@lanfon72 @TachunLin Hi! This PR is ready for review, please take a look, thanks!
The last case in image, the last case in volume and the first case in host can't be past, we'll fix these cases at incoming commit.

@TachunLin TachunLin mentioned this pull request Nov 7, 2022
Closed
10 tasks
@TachunLin
Copy link
Contributor

TachunLin commented Nov 7, 2022 •
edited
Loading

  • Tested can finish all Image test cases on Harvester v1.1.0, it takes 6 minutes 13 seconds execution time
    image

  • Tested can finish all Volume test cases on Harvester v1.1.0, it takes 3 minutes 15 seconds execution time
    image

  • Tested can finish all Network test cases on Harvester v1.1.0, it takes 1 minutes 6 seconds execution time
    image

  • Tested can finish all Settings test cases on Harvester v1.1.0, it takes 1 minutes 8 seconds execution time
    image

  • Tested can finish ssh-keys test cases in VM settings on Harvester v1.1.0
    image

@TachunLin
Copy link
Contributor

  1. To make the Image/Create a ISO image via upload works, we may need to change /cypress/cypress/fixtures/cirros-0.5.2-aarch64-disk.img to executable
    image

  2. We can use minio to setup as NFS backup target for the settings test cases

TachunLin
TachunLin requested changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@TachunLin TachunLin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While running the hosts.spec.ts, the 3rd test plan Check Add disk would failed at Line 70 testcases/dashboard/hosts.spec.ts:70:8

image

@DaiYuzeng DaiYuzeng mentioned this pull request Nov 8, 2022
Open
@DaiYuzeng
Copy link
Contributor Author

While running the hosts.spec.ts, the 3rd test plan Check Add disk would failed at Line 70 testcases/dashboard/hosts.spec.ts:70:8

image

Hi @TachunLin ! We would like to file a new issue to track this failed case. Thanks!

@TachunLin TachunLin self-requested a review November 8, 2022 02:25
TachunLin
TachunLin approved these changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@TachunLin TachunLin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to above test result.
Checked the related test cases can run correctly on Harvester v1.1.0
For the hosts.spec.ts related issue would be track separately at test issue #599

@DaiYuzeng
Copy link
Contributor Author

DaiYuzeng commented Nov 8, 2022 •
edited
Loading

Hi @TachunLin ! The newest commit has been merged into this PR. It will affect two cases:

  1. network
  2. virtualmachine/network

image

Please take a look! If these cases are passed, I think this PR may be ready to merge, thanks! (cc @n313893254 @WuJun2016 )

@TachunLin TachunLin self-requested a review November 8, 2022 07:40
TachunLin
TachunLin requested changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@TachunLin TachunLin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@n313893254, @DaiYuzeng
When tested the network/network.spec.ts, the Create Vlan1 and Vlan2 cases encounter failure.
It would probably related to the column id changes. Please help to take a look. Thanks

  • Create Vlan1 failed
    image

  • Create Vlan2 failed
    image

@TachunLin TachunLin self-requested a review November 8, 2022 10:18
@TachunLin
Copy link
Contributor

TachunLin commented Nov 8, 2022 •
edited
Loading

Checked can pass the Create Vlan1 and failed the Create Vlan2, it's correct behavior since Vlan 2 can't be routed on local kvm environment.
image

Can pass all virtualmachine/network test cases
image

TachunLin
TachunLin approved these changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@TachunLin TachunLin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the update

@noahgildersleeve
Copy link
Contributor

@WuJun2016 Could you check if your changes are still needed/fixed when you get a chance so we can get this merged?

@DaiYuzeng
Copy link
Contributor Author

Hi @noahgildersleeve @WuJun2016 ! I think this PR has been ready to merge, thank you!

@WuJun2016
Copy link
Contributor

WuJun2016 commented Nov 9, 2022 •
edited
Loading

@noahgildersleeve I think it can be merged. If there is any problem, we can submit a new pr to solve it.

@WuJun2016 Could you check if your changes are still needed/fixed when you get a chance so we can get this merged?

WuJun2016
WuJun2016 approved these changes Nov 9, 2022
View reviewed changes
Copy link
Contributor

@WuJun2016 WuJun2016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@noahgildersleeve noahgildersleeve merged commit c23d3f3 into harvester:main Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TachunLin TachunLin TachunLin approved these changes

@WuJun2016 WuJun2016 WuJun2016 approved these changes

@n313893254 n313893254 n313893254 approved these changes

@noahgildersleeve noahgildersleeve Awaiting requested review from noahgildersleeve

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@DaiYuzeng @noahgildersleeve @n313893254 @TachunLin @WuJun2016