-
Notifications
You must be signed in to change notification settings - Fork 534
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
test: e2e test chash upstream with key (query_string, arg_xxx) #932
Conversation
Codecov Report
@@ Coverage Diff @@
## master #932 +/- ##
==========================================
- Coverage 43.10% 43.02% -0.08%
==========================================
Files 18 18
Lines 1290 1290
==========================================
- Hits 556 555 -1
- Misses 642 643 +1
Partials 92 92
Continue to review full report at Codecov.
|
} | ||
resp.Body.Close() | ||
} | ||
//the results will be distributed among the 3 upstream |
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.
please add a comment: todo: need to find a better way to confirm the result
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.
current way is unstable
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.
fixed.
testCaseCheck(tc) | ||
} | ||
|
||
//hit routes |
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.
bad comment style, please fix other similar points
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.
fixed.
{ | ||
caseDesc: "create route using the upstream just created", | ||
Object: ManagerApiExpect(t), | ||
Method: http.MethodPut, | ||
Path: "/apisix/admin/routes/1", | ||
Body: `{ | ||
"uri": "/server_port", | ||
"upstream_id": "1" | ||
}`, | ||
Headers: map[string]string{"Authorization": token}, | ||
ExpectStatus: http.StatusOK, | ||
Sleep: sleepTime, | ||
}, |
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.
fixed
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.
fixed. @membphis
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.
LGTM, except minor things
counts = append(counts, value) | ||
} | ||
sort.Ints(counts) | ||
assert.True(t, float64(counts[2]-counts[0]) / float64(counts[1]) < 0.4) // the result is unstable, fix it later |
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.
add a todo
keyword
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.
fixed.
counts = append(counts, value) | ||
} | ||
sort.Ints(counts) | ||
assert.True(t, float64(counts[2]-counts[0]) / float64(counts[1]) < 0.4) // the result is unstable, fix it later |
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.
ditto
ping @idbeta |
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.
LGTM Except for some code format issues
Please answer these questions before submitting a pull request
related issue: #908