Skip to content

fix: use database for user creation to prevent flake #10992

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

Merged
merged 5 commits into from
Dec 4, 2023
Merged

fix: use database for user creation to prevent flake #10992

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b862d50
fix: prevent flake by increasing context deadline
f0ssel Dec 1, 2023
859b6b6
use db over API to speed up user creation
f0ssel Dec 1, 2023
37a6c10
remove comments
f0ssel Dec 1, 2023
39cd0f7
remove comment
f0ssel Dec 1, 2023
d4d2317
use generics
f0ssel Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 44 additions & 29 deletions coderd/users_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/util/slice"
Expand Down Expand Up @@ -1699,7 +1700,7 @@ func TestSuspendedPagination(t *testing.T) {
// them using different page sizes.
func TestPaginatedUsers(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
client, db := coderdtest.NewWithDatabase(t, nil)
coderdtest.CreateFirstUser(t, client)

// This test takes longer than a long time.
Expand All @@ -1708,15 +1709,17 @@ func TestPaginatedUsers(t *testing.T) {

me, err := client.User(ctx, codersdk.Me)
require.NoError(t, err)
orgID := me.OrganizationIDs[0]

// When 50 users exist
total := 50
allUsers := make([]codersdk.User, total+1) // +1 forme
allUsers[0] = me
specialUsers := make([]codersdk.User, total/2)
allUsers := make([]database.User, total+1)
allUsers[0] = database.User{
Email: me.Email,
Username: me.Username,
}
specialUsers := make([]database.User, total/2)

eg, egCtx := errgroup.WithContext(ctx)
eg, _ := errgroup.WithContext(ctx)
// Create users
for i := 0; i < total; i++ {
i := i
Expand All @@ -1730,21 +1733,14 @@ func TestPaginatedUsers(t *testing.T) {
if i%3 == 0 {
username = strings.ToUpper(username)
}
// One side effect of having to use the api vs the db calls directly, is you cannot
// mock time. Ideally I could pass in mocked times and space these users out.
//
// But this also serves as a good test. Postgres has microsecond precision on its timestamps.
// If 2 users share the same created_at, that could cause an issue if you are strictly paginating via
// timestamps. The pagination goes by timestamps and uuids.
newUser, err := client.CreateUser(egCtx, codersdk.CreateUserRequest{
Email: email,
Username: username,
Password: "MySecurePassword!",
OrganizationID: orgID,

// We used to use the API to ceate users, but that is slow.
// Instead, we create them directly in the database now
// to prevent timeout flakes.
newUser := dbgen.User(t, db, database.User{
Email: email,
Username: username,
})
if err != nil {
return err
}
allUsers[i+1] = newUser
if i%2 == 0 {
specialUsers[i/2] = newUser
Expand All @@ -1757,8 +1753,8 @@ func TestPaginatedUsers(t *testing.T) {
require.NoError(t, err, "create users failed")

// Sorting the users will sort by username.
sortUsers(allUsers)
sortUsers(specialUsers)
sortDatabaseUsers(allUsers)
sortDatabaseUsers(specialUsers)

gmailSearch := func(request codersdk.UsersRequest) codersdk.UsersRequest {
request.Search = "gmail"
Expand All @@ -1772,7 +1768,7 @@ func TestPaginatedUsers(t *testing.T) {
tests := []struct {
name string
limit int
allUsers []codersdk.User
allUsers []database.User
opt func(request codersdk.UsersRequest) codersdk.UsersRequest
}{
{name: "all users", limit: 10, allUsers: allUsers},
Expand Down Expand Up @@ -1800,7 +1796,7 @@ func TestPaginatedUsers(t *testing.T) {
// Assert pagination will page through the list of all users using the given
// limit for each page. The 'allUsers' is the expected full list to compare
// against.
func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client, limit int, allUsers []codersdk.User,
func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client, limit int, allUsers []database.User,
opt func(request codersdk.UsersRequest) codersdk.UsersRequest,
) {
var count int
Expand All @@ -1817,7 +1813,7 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client
},
}))
require.NoError(t, err, "first page")
require.Equalf(t, page.Users, allUsers[:limit], "first page, limit=%d", limit)
require.Equalf(t, onlyUsernames(page.Users), onlyUsernames(allUsers[:limit]), "first page, limit=%d", limit)
count += len(page.Users)

for {
Expand Down Expand Up @@ -1846,14 +1842,14 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client
}))
require.NoError(t, err, "next offset page")

var expected []codersdk.User
var expected []database.User
if count+limit > len(allUsers) {
expected = allUsers[count:]
} else {
expected = allUsers[count : count+limit]
}
require.Equalf(t, page.Users, expected, "next users, after=%s, limit=%d", afterCursor, limit)
require.Equalf(t, offsetPage.Users, expected, "offset users, offset=%d, limit=%d", count, limit)
require.Equalf(t, onlyUsernames(page.Users), onlyUsernames(expected), "next users, after=%s, limit=%d", afterCursor, limit)
require.Equalf(t, onlyUsernames(offsetPage.Users), onlyUsernames(expected), "offset users, offset=%d, limit=%d", count, limit)

// Also check the before
prevPage, err := client.Users(ctx, opt(codersdk.UsersRequest{
Expand All @@ -1863,7 +1859,7 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client
},
}))
require.NoError(t, err, "prev page")
require.Equal(t, allUsers[count-limit:count], prevPage.Users, "prev users")
require.Equal(t, onlyUsernames(allUsers[count-limit:count]), onlyUsernames(prevPage.Users), "prev users")
count += len(page.Users)
}
}
Expand All @@ -1875,6 +1871,25 @@ func sortUsers(users []codersdk.User) {
})
}

func sortDatabaseUsers(users []database.User) {
slices.SortFunc(users, func(a, b database.User) int {
return slice.Ascending(strings.ToLower(a.Username), strings.ToLower(b.Username))
})
}

func onlyUsernames[U codersdk.User | database.User](users []U) []string {
var out []string
for _, u := range users {
switch u := (any(u)).(type) {
case codersdk.User:
out = append(out, u.Username)
case database.User:
out = append(out, u.Username)
}
}
return out
}

func BenchmarkUsersMe(b *testing.B) {
client := coderdtest.New(b, nil)
_ = coderdtest.CreateFirstUser(b, client)
Expand Down