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.

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

Upload and Download tests against vfkit #453

Merged

Conversation

vyasgun
Copy link
Member

@vyasgun vyasgun commented Jan 10, 2025

This pull request adds two tests:

  1. Upload files to the VM This test uploads three files of different sizes (10M, 100M, 1G) to the running VM and verifies the sha256sum of the uploaded files.

  2. Download the above files from the VM This test downloads the three files uploaded in the previous test and verifies their sha256sum values.

@vyasgun vyasgun changed the title This commit adds two tests: Upload and Download tests against vfkit Jan 10, 2025
@vyasgun vyasgun force-pushed the pr/upload-download-tests branch 5 times, most recently from 62f231d to 730f708 Compare January 10, 2025 12:19
Copy link
Collaborator

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just a suggestion to reduce duplicate code

test-vfkit/vfkit_suite_test.go Show resolved Hide resolved
@vyasgun vyasgun force-pushed the pr/upload-download-tests branch from 730f708 to 8e45dda Compare January 13, 2025 08:07
Copy link
Collaborator

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

@vyasgun the test never deletes the temp files/dirs that it creates

@vyasgun vyasgun force-pushed the pr/upload-download-tests branch from 09652f1 to 8e45dda Compare January 16, 2025 03:13
Copy link
Collaborator

@evidolob evidolob left a comment

Choose a reason for hiding this comment

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

In current state test not testing what it intend, see comment for for loop

I prefer to have this test wrapped with Describe with some cleanup code in AfterEach. As multiple test runs will easily overflow laptop ssd.
If you are looking on samples how to doit, see https://onsi.github.io/ginkgo/#spec-cleanup-aftereach-and-defercleanup

FYI: I try to run test with fixed loop and it fail, VM crushed, not sure why
But this test is doing good job to indicate problems

test-vfkit/basic_test.go Outdated Show resolved Hide resolved
test-vfkit/basic_test.go Outdated Show resolved Hide resolved
@vyasgun vyasgun force-pushed the pr/upload-download-tests branch from 8e45dda to 909b24b Compare January 21, 2025 07:32
This commit adds two tests:

1. Upload files to the VM This test uploads three files of different sizes (10M, 100M, 1G) to the running VM and verifies the sha256sum of the uploaded files.

2. Download the above files from the VM This test downloads the three files uploaded in the previous test and verifies their sha256sum values.

Signed-off-by: Gunjan Vyas <[email protected]>
@vyasgun vyasgun force-pushed the pr/upload-download-tests branch from 909b24b to 0b09d74 Compare January 23, 2025 08:00
@vyasgun vyasgun requested a review from evidolob January 23, 2025 08:01
@vyasgun
Copy link
Member Author

vyasgun commented Jan 23, 2025

Thank you @lstocchi and @evidolob -- addressed the comments and updated the PR

Copy link
Collaborator

@lstocchi lstocchi 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 @vyasgun 🚀

@lstocchi
Copy link
Collaborator

/approve

Copy link
Contributor

openshift-ci bot commented Jan 29, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evidolob, lstocchi, vyasgun

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cfergeau
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 30, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit c30fc5c into containers:main Jan 30, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants