-
Notifications
You must be signed in to change notification settings - Fork 101
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
[WIP] Negative tests for network interruption #106
[WIP] Negative tests for network interruption #106
Conversation
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.
I have done a preliminary review. Please consider if the suggestions make sense.
@@ -136,6 +136,12 @@ func (p *fakePolicy) handleContainerGetOperation(w *http.Response) error { | |||
|
|||
// handleListObjectNames responds with a blob `List` response. | |||
func (p *fakePolicy) handleListObjects(w *http.Response) error { | |||
if networkTimeoutFlag { |
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.
How about a separate delegating errorPolicy implementation that delegates to fakePolicy if the flag is set? This way the fakePolicy implementation is not littered with if conditions.
Besides, the if conditions might proliferate for simulating latency or some other response error etc.This would apply to all the methods of fakePolicy that are modified below.
WDYT?
swiftStore snapstore.SnapStore | ||
testObj *testing.T | ||
swiftStore snapstore.SnapStore | ||
networkTimeoutFlag bool = false |
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 will need eventually simulate other errors as well such as different HTTP errors, latency etc.
This flag is better kept in the fakePolicy or the errorPolicy I proposed rather than as a global flag.
What is the status here? if it is matching with #173 can we close this in favor of other PR.? |
@swapnilgm We can close this PR for now and reopen when necessary. Would be easier to run such negative test scenarios from infra perspective once we have etcd controller. WDYT? |
What this PR does / why we need it:
Negative tests for network interruption scenarios.
Which issue(s) this PR fixes:
fixes partially #73 and #104
Special notes for your reviewer:
This is only a first-cut version of setting up negative tests, and includes only network interruption cases, which might occur during calls to the object store.
Release note: