artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 17:11:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore: Remove client construction from SignUp/In funcs (#62789)

Browse Source 
These functions are used in test code, which makes it tricker to
pass in loggers for checking requests and responses. Instead, make
the API method-based. This introduces slightly more boilerplate
since Client construction is fallible, but it allows calling code
to pass in loggers more easily for debugging test failures.
This commit is contained in:
Varun Gandhi 2024-05-21 21:18:58 +08:00 committed by GitHub
parent 7347853bba
commit e4bb0b5ce6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 90 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
dev/authtest/code_intel_test.go
View File

@ -21,10 +21,9 @@ func TestCodeIntelEndpoints(t *testing.T) {
// user should receive access denied for LSIF endpoints of repositories the user
// does not have access to.
const testUsername = "authtest-user-code-intel"
userClient, err := gqltestutil.SignUp(*baseURL, testUsername+"@sourcegraph.com", testUsername, "mysecurepassword")
if err != nil {
t.Fatal(err)
}
userClient, err := gqltestutil.NewClient(*baseURL)
require.NoError(t, err)
require.NoError(t, userClient.SignUp(testUsername+"@sourcegraph.com", testUsername, "mysecurepassword"))
defer func() {
err := client.DeleteUser(userClient.AuthenticatedUserID(), true)
if err != nil {

10
dev/authtest/main_test.go
View File

@ -54,15 +54,17 @@ func TestMain(m *testing.M) {
log.Fatal("Failed to check if site needs init: ", err)
}
client, err = gqltestutil.NewClient(*baseURL)
if err != nil {
log.Fatal("Failed to create gql client: ", err)
}
if needsSiteInit {
client, err = gqltestutil.SiteAdminInit(*baseURL, *email, *username, *password)
if err != nil {
if err := client.SiteAdminInit(*email, *username, *password); err != nil {
log.Fatal("Failed to create site admin: ", err)
}
log.Println("Site admin has been created:", *username)
} else {
client, err = gqltestutil.SignIn(*baseURL, *email, *password)
if err != nil {
if err := client.SignIn(*email, *password); err != nil {
log.Fatal("Failed to sign in:", err)
}
log.Println("Site admin authenticated:", *username)

8
dev/authtest/organization_test.go
View File

@ -6,6 +6,7 @@ import (
"testing"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"github.com/sourcegraph/sourcegraph/internal/gqltestutil"
)
@ -27,10 +28,9 @@ func TestOrganization(t *testing.T) {
// "authtest-organization", the user should not have access to any of the
// organization's resources.
const testUsername = "authtest-user-organization"
userClient, err := gqltestutil.SignUp(*baseURL, testUsername+"@sourcegraph.com", testUsername, "mysecurepassword")
if err != nil {
t.Fatal(err)
}
userClient, err := gqltestutil.NewClient(*baseURL)
require.NoError(t, err)
require.NoError(t, userClient.SignUp(testUsername+"@sourcegraph.com", testUsername, "mysecurepassword"))
defer func() {
err := client.DeleteUser(userClient.AuthenticatedUserID(), true)
if err != nil {

8
dev/authtest/repository_test.go
View File

@ -5,6 +5,7 @@ import (
"time"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"github.com/sourcegraph/sourcegraph/internal/extsvc"
"github.com/sourcegraph/sourcegraph/internal/gqltestutil"
@ -84,10 +85,9 @@ func TestRepository(t *testing.T) {
// Create a test user (authtest-user-repository) which is not a site admin, the
// user should only have access to non-private repositories.
const testUsername = "authtest-user-repository"
userClient, err := gqltestutil.SignUp(*baseURL, testUsername+"@sourcegraph.com", testUsername, "mysecurepassword")
if err != nil {
t.Fatal(err)
}
userClient, err := gqltestutil.NewClient(*baseURL)
require.NoError(t, err)
require.NoError(t, userClient.SignUp(testUsername+"@sourcegraph.com", testUsername, "mysecurepassword"))
defer func() {
err := client.DeleteUser(userClient.AuthenticatedUserID(), true)
if err != nil {

9
dev/authtest/site_admin_test.go
View File

@ -7,6 +7,8 @@ import (
"strings"
"testing"
"github.com/stretchr/testify/require"
"github.com/sourcegraph/sourcegraph/internal/auth"
"github.com/sourcegraph/sourcegraph/internal/gqltestutil"
)
@ -15,10 +17,9 @@ func TestSiteAdminEndpoints(t *testing.T) {
// Create a test user (authtest-user-1) which is not a site admin, the user
// should receive access denied for site admin endpoints.
const testUsername = "authtest-user-1"
userClient, err := gqltestutil.SignUp(*baseURL, testUsername+"@sourcegraph.com", testUsername, "mysecurepassword")
if err != nil {
t.Fatal(err)
}
userClient, err := gqltestutil.NewClient(*baseURL)
require.NoError(t, err)
require.NoError(t, userClient.SignUp(testUsername+"@sourcegraph.com", testUsername, "mysecurepassword"))
defer func() {
err := client.DeleteUser(userClient.AuthenticatedUserID(), true)
if err != nil {

11
dev/ci/integration/executors/tester/init.go
View File

@ -27,16 +27,17 @@ func initAndAuthenticate() (*gqltestutil.Client, error) {
return nil, errors.Wrap(err, "failed to check if site needs init")
}
var client *gqltestutil.Client
client, err := gqltestutil.NewClient(SourcegraphEndpoint)
if err != nil {
return nil, errors.Wrap(err, "failed to create gql client")
}
if needsSiteInit {
client, err = gqltestutil.SiteAdminInit(SourcegraphEndpoint, adminEmail, adminUsername, adminPassword)
if err != nil {
if err := client.SiteAdminInit(adminEmail, adminUsername, adminPassword); err != nil {
return nil, errors.Wrap(err, "failed to create site admin")
}
log.Println("Site admin has been created:", adminUsername)
} else {
client, err = gqltestutil.SignIn(SourcegraphEndpoint, adminEmail, adminPassword)
if err != nil {
if err = client.SignIn(adminEmail, adminPassword); err != nil {
return nil, errors.Wrap(err, "failed to sign in")
}
log.Println("Site admin authenticated:", adminUsername)

5
dev/codeintel-qa/internal/graphql.go
View File

@ -11,7 +11,10 @@ var (
)
func InitializeGraphQLClient() (err error) {
client, err = gqltestutil.NewClient(SourcegraphEndpoint, requestWriter.Write, responseWriter.Write)
client, err = gqltestutil.NewClient(SourcegraphEndpoint, gqltestutil.ClientOption{
GraphQLRequestLogger: requestWriter.Write,
GraphQLResponseLogger: responseWriter.Write,
})
return err
}

12
dev/gqltest/main_test.go
View File

@ -66,15 +66,17 @@ func TestMain(m *testing.M) {
log.Fatal("Failed to check if site needs init:", err)
}
client, err = gqltestutil.NewClient(*baseURL)
if err != nil {
log.Fatal("Failed to create gql client: ", err)
}
if needsSiteInit {
client, err = gqltestutil.SiteAdminInit(*baseURL, *email, *username, *password)
if err != nil {
log.Fatal("Failed to create site admin:", err)
if err := client.SiteAdminInit(*email, *username, *password); err != nil {
log.Fatal("Failed to create site admin: ", err)
}
log.Println("Site admin has been created:", *username)
} else {
client, err = gqltestutil.SignIn(*baseURL, *email, *password)
if err != nil {
if err := client.SignIn(*email, *password); err != nil {
log.Fatal("Failed to sign in:", err)
}
log.Println("Site admin authenticated:", *username)

14
dev/gqltest/site_config_test.go
View File

@ -6,6 +6,8 @@ import (
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/sourcegraph/sourcegraph/internal/gqltestutil"
"github.com/sourcegraph/sourcegraph/schema"
)
@ -14,10 +16,9 @@ func TestSiteConfig(t *testing.T) {
t.Run("builtin auth provider: allowSignup", func(t *testing.T) {
// Sign up a new user is allowed by default.
const testUsername1 = "gqltest-auth-user-1"
testClient1, err := gqltestutil.SignUp(*baseURL, testUsername1+"@sourcegraph.com", testUsername1, "mysecurepassword")
if err != nil {
t.Fatal(err)
}
testClient1, err := gqltestutil.NewClient(*baseURL)
require.NoError(t, err)
require.NoError(t, testClient1.SignUp(testUsername1+"@sourcegraph.com", testUsername1, "mysecurepassword"))
removeTestUserAfterTest(t, testClient1.AuthenticatedUserID())
// Update site configuration to not allow sign up for builtin auth provider.
@ -42,8 +43,9 @@ func TestSiteConfig(t *testing.T) {
err = gqltestutil.Retry(5*time.Second, func() error {
// Sign up a new user should fail.
const testUsername2 = "gqltest-auth-user-2"
testClient2, err := gqltestutil.SignUp(*baseURL, testUsername2+"@sourcegraph.com", testUsername2, "mysecurepassword")
if err != nil {
testClient2, err := gqltestutil.NewClient(*baseURL)
require.NoError(t, err)
if err := testClient2.SignUp(testUsername2+"@sourcegraph.com", testUsername2, "mysecurepassword"); err != nil {
if strings.Contains(err.Error(), "Signup is not enabled") {
return nil
}

7
dev/gqltest/sub_repo_permissions_test.go
View File

@ -338,10 +338,9 @@ func createTestUserAndWaitForRepo(t *testing.T) (*gqltestutil.Client, string, er
// Alice doesn't have access to Security directory. (there is a .sh file)
alicePassword := "alicessupersecurepassword"
t.Log("Creating Alice")
userClient, err := gqltestutil.SignUpOrSignIn(*baseURL, aliceEmail, aliceUsername, alicePassword)
if err != nil {
t.Fatal(err)
}
userClient, err := gqltestutil.NewClient(*baseURL)
require.NoError(t, err)
require.NoError(t, userClient.SignUp(aliceEmail, aliceUsername, alicePassword))
aliceID := userClient.AuthenticatedUserID()
removeTestUserAfterTest(t, aliceID)

20
internal/cmd/init-sg/main.go
View File

@ -75,15 +75,17 @@ func initSourcegraph() {
log.Fatal("Failed to check if site needs init: ", err)
}
client, err = gqltestutil.NewClient(*baseURL)
if err != nil {
log.Fatal("Failed to create gql client: ", err)
}
if needsSiteInit {
client, err = gqltestutil.SiteAdminInit(*baseURL, *email, *username, *password)
if err != nil {
if err := client.SiteAdminInit(*email, *username, *password); err != nil {
log.Fatal("Failed to create site admin: ", err)
}
log.Println("Site admin has been created:", *username)
} else {
client, err = gqltestutil.SignIn(*baseURL, *email, *password)
if err != nil {
if err := client.SignIn(*email, *password); err != nil {
log.Fatal("Failed to sign in:", err)
}
log.Println("Site admin authenticated:", *username)
@ -140,8 +142,11 @@ func addReposCommand() {
log.Fatal("Environment variable GITHUB_TOKEN is not set")
}
client, err := gqltestutil.SignIn(*baseURL, *email, *password)
client, err := gqltestutil.NewClient(*baseURL)
if err != nil {
log.Fatal("Failed to create gql client: ", err)
}
if err := client.SignIn(*email, *password); err != nil {
log.Fatal("Failed to sign in:", err)
}
log.Println("Site admin authenticated:", *username)
@ -214,8 +219,11 @@ func oobmigrationCommand() {
id := *migrationID
up := !*migrationDownFlag
client, err := gqltestutil.SignIn(*baseURL, *email, *password)
client, err := gqltestutil.NewClient(*baseURL)
if err != nil {
log.Fatal("Failed to create gql client: ", err)
}
if err := client.SignIn(*email, *password); err != nil {
log.Fatal("Failed to sign in:", err)
}
log.Println("Site admin authenticated:", *username)

59
internal/gqltestutil/client.go
View File

@ -30,8 +30,8 @@ func NeedsSiteInit(baseURL string) (bool, string, error) {
// SiteAdminInit initializes the instance with given admin account.
// It returns an authenticated client as the admin for doing testing.
func SiteAdminInit(baseURL, email, username, password string) (*Client, error) {
return authenticate(baseURL, "/-/site-init", map[string]string{
func (c *Client) SiteAdminInit(email, username, password string) error {
return c.authenticate("/-/site-init", map[string]string{
"email": email,
"username": username,
"password": password,
@ -40,46 +40,30 @@ func SiteAdminInit(baseURL, email, username, password string) (*Client, error) {
// SignUp signs up a new user with given credentials.
// It returns an authenticated client as the user for doing testing.
func SignUp(baseURL, email, username, password string) (*Client, error) {
return authenticate(baseURL, "/-/sign-up", map[string]string{
func (c *Client) SignUp(email, username, password string) error {
return c.authenticate("/-/sign-up", map[string]string{
"email": email,
"username": username,
"password": password,
})
}
func SignUpOrSignIn(baseURL, email, username, password string) (*Client, error) {
client, err := SignUp(baseURL, email, username, password)
if err != nil {
return SignIn(baseURL, email, password)
func (c *Client) SignUpOrSignIn(email, username, password string) error {
if err := c.SignUp(email, username, password); err != nil {
return c.SignIn(email, password)
}
return client, err
return nil
}
// SignIn performs the sign in with given user credentials.
// It returns an authenticated client as the user for doing testing.
func SignIn(baseURL, email, password string) (*Client, error) {
return authenticate(baseURL, "/-/sign-in", map[string]string{
func (c *Client) SignIn(email, password string) error {
return c.authenticate("/-/sign-in", map[string]string{
"email": email,
"password": password,
})
}
// authenticate initializes an authenticated client with given request body.
func authenticate(baseURL, path string, body any) (*Client, error) {
client, err := NewClient(baseURL, nil, nil)
if err != nil {
return nil, errors.Wrap(err, "new client")
}
err = client.authenticate(path, body)
if err != nil {
return nil, errors.Wrap(err, "authenticate")
}
return client, nil
}
// extractCSRFToken extracts CSRF token from HTML response body.
func extractCSRFToken(body string) string {
anchor := `X-Csrf-Token":"`
@ -114,16 +98,25 @@ type LogFunc func(payload []byte)
func noopLog(payload []byte) {}
type ClientOption struct {
GraphQLRequestLogger LogFunc
GraphQLResponseLogger LogFunc
}
// NewClient instantiates a new client by performing a GET request then obtains the
// CSRF token and cookie from its response, if there is one (old versions of Sourcegraph only).
// If request- or responseLogger are provided, the request and response bodies, respectively,
// If loggers are provided via options, the request and response bodies, respectively,
// will be written to them for any GraphQL requests only.
func NewClient(baseURL string, requestLogger, responseLogger LogFunc) (*Client, error) {
if requestLogger == nil {
requestLogger = noopLog
}
if responseLogger == nil {
responseLogger = noopLog
func NewClient(baseURL string, options ...ClientOption) (*Client, error) {
requestLogger := noopLog
responseLogger := noopLog
for _, opt := range options {
if opt.GraphQLRequestLogger != nil {
requestLogger = opt.GraphQLRequestLogger
}
if opt.GraphQLResponseLogger != nil {
responseLogger = opt.GraphQLResponseLogger
}
}
resp, err := http.Get(baseURL)