chore(coderd/database/dbpurge): replace usage of time.* with quartz #14480
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mafredri
approved these changes
Aug 29, 2024
| agent := mustCreateAgentWithLogs(ctx, t, db, user, org, tmpl, tv, now.Add(-8*24*time.Hour), t.Name()) | ||
| // After dbpurge completes, the ticker is reset. Trap this call. | ||
| trapReset := clk.Trap().TickerReset() | ||
| defer trapReset.Close() |
There was a problem hiding this comment.
This seems shared between all tests, how about refactoring to a helper like below, or consider only testing it in one test and assuming it applies to the rest?
t.Helper()
// After dbpurge completes, the ticker is reset. Trap this call.
trapReset := clk.Trap().TickerReset()
defer trapReset.Close()
closer := do(clk)
// Wait for the initial nanosecond tick.
clk.Advance(time.Nanosecond).MustWait(ctx)
trapReset.MustWait(ctx).Release() // Wait for ticker.Reset()
return closerPerhaps this example can even be taken further, just here as food for thought.
There was a problem hiding this comment.
I modify this test in the follow-up PR #14460 so the sharing between tests won't be an issue.
But I do like the idea of extracting this to a test helper!
dannykopping
approved these changes
Aug 29, 2024
| } | ||
|
|
||
| //nolint:paralleltest // It uses LockIDDBPurge. | ||
| func TestDeleteOldWorkspaceAgentStats(t *testing.T) { | ||
| db, _ := dbtestutil.NewDB(t) | ||
| logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) | ||
| ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) |
There was a problem hiding this comment.
Nit: testutil.Context is a nice convenience for this:
func Context(t *testing.T, dur time.Duration) context.Context {
ctx, cancel := context.WithTimeout(context.Background(), dur)
t.Cleanup(cancel)
return ctx
}There was a problem hiding this comment.
Will address in follow-up 👍
…teOldWorkspaceAgentLogs
…CreateAgentWithLogs
chore(dbpurge): use quartz.TickerFunc instead
a7e0f15 to
e826ae9
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #10576
This PR introduces quartz to
coderd/database/dbpurgeand updates the following unit tests to make use of Quartz's functionality:TestPurgeTestDeleteOldWorkspaceAgentLogsAdditionally, updates
DeleteOldWorkspaceAgentLogsto replace the hard-coded interval with a parameter passed into the query. This aids in testing and brings us a step towards allowing operators to configure the cutoff interval for workspace agent logs.