Skip to content

Commit ccd5e1a

Browse files
authored
fix: use database for user creation to prevent flake (#10992)
1 parent 2f54f76 commit ccd5e1a

File tree

1 file changed

+44
-29
lines changed

1 file changed

+44
-29
lines changed

coderd/users_test.go

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/coder/coder/v2/coderd/coderdtest"
2424
"github.com/coder/coder/v2/coderd/database"
2525
"github.com/coder/coder/v2/coderd/database/dbauthz"
26+
"github.com/coder/coder/v2/coderd/database/dbgen"
2627
"github.com/coder/coder/v2/coderd/database/dbtime"
2728
"github.com/coder/coder/v2/coderd/rbac"
2829
"github.com/coder/coder/v2/coderd/util/slice"
@@ -1699,7 +1700,7 @@ func TestSuspendedPagination(t *testing.T) {
16991700
// them using different page sizes.
17001701
func TestPaginatedUsers(t *testing.T) {
17011702
t.Parallel()
1702-
client := coderdtest.New(t, nil)
1703+
client, db := coderdtest.NewWithDatabase(t, nil)
17031704
coderdtest.CreateFirstUser(t, client)
17041705

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

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

17131713
// When 50 users exist
17141714
total := 50
1715-
allUsers := make([]codersdk.User, total+1) // +1 forme
1716-
allUsers[0] = me
1717-
specialUsers := make([]codersdk.User, total/2)
1715+
allUsers := make([]database.User, total+1)
1716+
allUsers[0] = database.User{
1717+
Email: me.Email,
1718+
Username: me.Username,
1719+
}
1720+
specialUsers := make([]database.User, total/2)
17181721

1719-
eg, egCtx := errgroup.WithContext(ctx)
1722+
eg, _ := errgroup.WithContext(ctx)
17201723
// Create users
17211724
for i := 0; i < total; i++ {
17221725
i := i
@@ -1730,21 +1733,14 @@ func TestPaginatedUsers(t *testing.T) {
17301733
if i%3 == 0 {
17311734
username = strings.ToUpper(username)
17321735
}
1733-
// One side effect of having to use the api vs the db calls directly, is you cannot
1734-
// mock time. Ideally I could pass in mocked times and space these users out.
1735-
//
1736-
// But this also serves as a good test. Postgres has microsecond precision on its timestamps.
1737-
// If 2 users share the same created_at, that could cause an issue if you are strictly paginating via
1738-
// timestamps. The pagination goes by timestamps and uuids.
1739-
newUser, err := client.CreateUser(egCtx, codersdk.CreateUserRequest{
1740-
Email: email,
1741-
Username: username,
1742-
Password: "MySecurePassword!",
1743-
OrganizationID: orgID,
1736+
1737+
// We used to use the API to ceate users, but that is slow.
1738+
// Instead, we create them directly in the database now
1739+
// to prevent timeout flakes.
1740+
newUser := dbgen.User(t, db, database.User{
1741+
Email: email,
1742+
Username: username,
17441743
})
1745-
if err != nil {
1746-
return err
1747-
}
17481744
allUsers[i+1] = newUser
17491745
if i%2 == 0 {
17501746
specialUsers[i/2] = newUser
@@ -1757,8 +1753,8 @@ func TestPaginatedUsers(t *testing.T) {
17571753
require.NoError(t, err, "create users failed")
17581754

17591755
// Sorting the users will sort by username.
1760-
sortUsers(allUsers)
1761-
sortUsers(specialUsers)
1756+
sortDatabaseUsers(allUsers)
1757+
sortDatabaseUsers(specialUsers)
17621758

17631759
gmailSearch := func(request codersdk.UsersRequest) codersdk.UsersRequest {
17641760
request.Search = "gmail"
@@ -1772,7 +1768,7 @@ func TestPaginatedUsers(t *testing.T) {
17721768
tests := []struct {
17731769
name string
17741770
limit int
1775-
allUsers []codersdk.User
1771+
allUsers []database.User
17761772
opt func(request codersdk.UsersRequest) codersdk.UsersRequest
17771773
}{
17781774
{name: "all users", limit: 10, allUsers: allUsers},
@@ -1800,7 +1796,7 @@ func TestPaginatedUsers(t *testing.T) {
18001796
// Assert pagination will page through the list of all users using the given
18011797
// limit for each page. The 'allUsers' is the expected full list to compare
18021798
// against.
1803-
func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client, limit int, allUsers []codersdk.User,
1799+
func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client, limit int, allUsers []database.User,
18041800
opt func(request codersdk.UsersRequest) codersdk.UsersRequest,
18051801
) {
18061802
var count int
@@ -1817,7 +1813,7 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client
18171813
},
18181814
}))
18191815
require.NoError(t, err, "first page")
1820-
require.Equalf(t, page.Users, allUsers[:limit], "first page, limit=%d", limit)
1816+
require.Equalf(t, onlyUsernames(page.Users), onlyUsernames(allUsers[:limit]), "first page, limit=%d", limit)
18211817
count += len(page.Users)
18221818

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

1849-
var expected []codersdk.User
1845+
var expected []database.User
18501846
if count+limit > len(allUsers) {
18511847
expected = allUsers[count:]
18521848
} else {
18531849
expected = allUsers[count : count+limit]
18541850
}
1855-
require.Equalf(t, page.Users, expected, "next users, after=%s, limit=%d", afterCursor, limit)
1856-
require.Equalf(t, offsetPage.Users, expected, "offset users, offset=%d, limit=%d", count, limit)
1851+
require.Equalf(t, onlyUsernames(page.Users), onlyUsernames(expected), "next users, after=%s, limit=%d", afterCursor, limit)
1852+
require.Equalf(t, onlyUsernames(offsetPage.Users), onlyUsernames(expected), "offset users, offset=%d, limit=%d", count, limit)
18571853

18581854
// Also check the before
18591855
prevPage, err := client.Users(ctx, opt(codersdk.UsersRequest{
@@ -1863,7 +1859,7 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client
18631859
},
18641860
}))
18651861
require.NoError(t, err, "prev page")
1866-
require.Equal(t, allUsers[count-limit:count], prevPage.Users, "prev users")
1862+
require.Equal(t, onlyUsernames(allUsers[count-limit:count]), onlyUsernames(prevPage.Users), "prev users")
18671863
count += len(page.Users)
18681864
}
18691865
}
@@ -1875,6 +1871,25 @@ func sortUsers(users []codersdk.User) {
18751871
})
18761872
}
18771873

1874+
func sortDatabaseUsers(users []database.User) {
1875+
slices.SortFunc(users, func(a, b database.User) int {
1876+
return slice.Ascending(strings.ToLower(a.Username), strings.ToLower(b.Username))
1877+
})
1878+
}
1879+
1880+
func onlyUsernames[U codersdk.User | database.User](users []U) []string {
1881+
var out []string
1882+
for _, u := range users {
1883+
switch u := (any(u)).(type) {
1884+
case codersdk.User:
1885+
out = append(out, u.Username)
1886+
case database.User:
1887+
out = append(out, u.Username)
1888+
}
1889+
}
1890+
return out
1891+
}
1892+
18781893
func BenchmarkUsersMe(b *testing.B) {
18791894
client := coderdtest.New(b, nil)
18801895
_ = coderdtest.CreateFirstUser(b, client)

0 commit comments

Comments
 (0)