-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Consolidate boilerplate in integration tests #1979
Conversation
LGTM |
LGTM |
integrations/integration_test.go
Outdated
) | ||
doc := NewHtmlParser(t, resp.Body) | ||
req = NewRequestWithValues(t, "POST", "/user/login", map[string]string{ | ||
"_csrf": doc.GetInputValueByName("_csrf"), |
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.
This should be changed to GetCSRF
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.
Good catch!
@@ -155,21 +156,19 @@ func (s *TestSession) MakeRequest(t *testing.T, req *http.Request) *TestResponse | |||
return resp | |||
} | |||
|
|||
func loginUser(t *testing.T, userName, password string) *TestSession { |
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.
No, I don't think we should do that. How should we test user login with different password?
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 having two functions?
func loginUserDefaultPassword(...) {
loginUser(..., "password")
}
func loginUser(...) {
// what we originally had
}
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.
@lunny Done
@ethantkoenig Could you rebase the master? something wrong with drone sig file. |
d008849
to
d1603d3
Compare
@appleboy Rebased |
d1603d3
to
a84e002
Compare
LGTM |
Move error-checking and other shared boilerplate to helper functions; makes tests less verbose.