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

Add mock tests for Openstack swift #75

Merged
merged 2 commits into from Dec 2, 2018
Merged

Add mock tests for Openstack swift #75

merged 2 commits into from Dec 2, 2018

Conversation

swapnilgm
Copy link
Contributor

@swapnilgm swapnilgm commented Nov 29, 2018 •
edited

Signed-off-by: Swapnil Mhamane swapnil.mhamane@sap.com

What this PR does / why we need it:
This PR implements the mock test for Openstack Swift snapstore.

Which issue(s) this PR fixes:
Fixes #

Partially completes #70

Special notes for your reviewer:
We will right negative test once we are done with mock client for all cloud provider. This is to get idea for writing generic negative test.

Release note:

Added mock test for Openstack Swift snapstore.

@swapnilgm swapnilgm added size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) area/dev-productivity Developer productivity related (how to improve development) exp/intermediate Issue that requires some project experience platform/openstack OpenStack platform/infrastructure area/ops-productivity Operator productivity related (how to improve operations) kind/test Test priority/normal needs/lgtm Needs approval for merging needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) status/new Issue is new and unprocessed labels Nov 29, 2018
@swapnilgm swapnilgm added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 30, 2018
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 30, 2018
shreyas-s-rao
shreyas-s-rao requested changes Nov 30, 2018
View reviewed changes
Copy link
Collaborator

@shreyas-s-rao shreyas-s-rao left a comment

Choose a reason for hiding this comment

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

Thanks for the well-written PR! I have a few suggestions that can ease debugging during testing. Please address them. Thanks.

pkg/snapstore/snapstore_test.go Outdated Show resolved Hide resolved
pkg/snapstore/snapstore_test.go Outdated Show resolved Hide resolved
pkg/snapstore/snapstore_test.go Outdated Show resolved Hide resolved
pkg/snapstore/snapstore_test.go Outdated Show resolved Hide resolved
pkg/snapstore/snapstore_test.go Outdated Show resolved Hide resolved
@swapnilgm swapnilgm force-pushed the swift-client branch 5 times, most recently from ee5326e to d0fc8a2 Compare November 30, 2018 11:02
@swapnilgm swapnilgm added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 30, 2018
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 30, 2018
Add mock client for openstak
0a5e579 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
Use ioutil.ReadAll for all read operations
d27b960 
Signed-off-by: Swapnil Mhamane <swapnil.mhamane@sap.com>
@swapnilgm swapnilgm added status/in-progress Issue is in progress/work reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed status/new Issue is new and unprocessed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 30, 2018
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 30, 2018
@swapnilgm
Copy link
Contributor Author

@shreyas-s-rao Thank for the review. I have updated the PR.The issue with logs message regarding ginkgo warning of DATA race condition has been resolved using appropriate. Also, broken pipe error logs issue has been resolved. PTAL.

shreyas-s-rao
shreyas-s-rao approved these changes Nov 30, 2018
View reviewed changes
Copy link
Collaborator

@shreyas-s-rao shreyas-s-rao left a comment •
edited

Choose a reason for hiding this comment

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

@swapnilgm Thanks for making the changes. Good to know the data race issue and the broken pipe error in the tests have now been resolved!
LGTM 👍🏼

@swapnilgm swapnilgm merged commit 1b25f0c into gardener:master Dec 2, 2018
@swapnilgm swapnilgm deleted the swift-client branch December 2, 2018 17:52
@PadmaB PadmaB modified the milestones: ---, 0.4.0 Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shreyas-s-rao shreyas-s-rao shreyas-s-rao approved these changes

@georgekuruvillak georgekuruvillak Awaiting requested review from georgekuruvillak

Assignees
No one assigned
Labels
area/dev-productivity Developer productivity related (how to improve development) area/ops-productivity Operator productivity related (how to improve operations) exp/intermediate Issue that requires some project experience kind/test Test needs/lgtm Needs approval for merging needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/openstack OpenStack platform/infrastructure size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/in-progress Issue is in progress/work
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@swapnilgm @shreyas-s-rao @PadmaB @gardener-robot-ci-1