fix: use database for user creation to prevent flake #10992
Conversation
|
Talked with @Emyrk and I'm gonna take a stab at speeding this test up, he had some ideas here. |
|
I looked at the test data in CI and interestingly this test averages very fast, despite the comments in the code saying otherwise. That being said, I still think if we are timing out at 50 seconds something on the CI box is running out of resources, and moving this to the database will still reduce the test startup burden for this test. |
coderd/users_test.go
Outdated
| for i := range allUsers[:limit] { | ||
| require.Equalf(t, page.Users[i].Username, allUsers[i].Username, "first page, limit=%d", limit) | ||
| } |
There was a problem hiding this comment.
I think the original comparisons are better since the new comparisons fail at the first instance, and do not show the lists and the list lengths in the failure output. Switching to assert feels like it might be spammy.
Can we just make a test helper func like:
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
}Then we can bring back the old requires.
require.Equalf(t, onlyUsernames(page.Users), onlyUsernames(allUsers[:limit]), "first page, limit=%d", limit)There was a problem hiding this comment.
Sounds good to me, good idea. I have yet to really use generics, this will be fun.
There was a problem hiding this comment.
It's a bit of a hack since I cast to any in the function 😄
It just gives type safety on the call arguments.
This is definitely subjective, but I think >100ms is on the slower side for our tests. Is it something to be concerned about? Idk, probably not. |
After speaking with @Emyrk I moved the user creation of these tests to use the database directly to speed things up and do less http requests in order to stay under the 50 second timeout.
Closes #10978