Skip to content

Commit 51226c5

Browse files
mafredrimafredri
authored
test(coderd): close metricscache and avoid background context (coder#7996)
1 parent 2e7e99b commit 51226c5

File tree

6 files changed

+33
-7
lines changed

6 files changed

+33
-7
lines changed

‎coderd/coderdtest/coderdtest.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ func newWithCloser(t testing.TB, options *Options) (*codersdk.Client, io.Closer)
169169
}
170170

171171
func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.CancelFunc, *url.URL, *coderd.Options) {
172+
t.Helper()
173+
172174
if options == nil {
173175
options = &Options{}
174176
}
@@ -402,6 +404,8 @@ func NewWithAPI(t testing.TB, options *Options) (*codersdk.Client, io.Closer, *c
402404
// well with coderd testing. It registers the "echo" provisioner for
403405
// quick testing.
404406
func NewProvisionerDaemon(t testing.TB, coderAPI *coderd.API) io.Closer {
407+
t.Helper()
408+
405409
echoClient, echoServer := provisionersdk.MemTransportPipe()
406410
ctx, cancelFunc := context.WithCancel(context.Background())
407411
t.Cleanup(func() {

‎coderd/metricscache/metricscache_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ func TestCache_DeploymentStats(t *testing.T) {
435435
cache := metricscache.New(db, slogtest.Make(t, nil), metricscache.Intervals{
436436
DeploymentStats: testutil.IntervalFast,
437437
})
438+
defer cache.Close()
438439

439440
_, err := db.InsertWorkspaceAgentStat(context.Background(), database.InsertWorkspaceAgentStatParams{
440441
ID: uuid.New(),

‎coderd/workspaceapps/apptest/setup.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U
347347
primaryAppHost, err := client.AppHost(appHostCtx)
348348
require.NoError(t, err)
349349
if primaryAppHost.Host != "" {
350-
manifest, err := agentClient.Manifest(context.Background())
350+
manifest, err := agentClient.Manifest(appHostCtx)
351351
require.NoError(t, err)
352352
proxyURL := fmt.Sprintf(
353353
"http://{{port}}--%s--%s--%s%s",

‎enterprise/coderd/coderd.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import (
3535
// New constructs an Enterprise coderd API instance.
3636
// This handler is designed to wrap the AGPL Coder code and
3737
// layer Enterprise functionality on top as much as possible.
38-
func New(ctx context.Context, options *Options) (*API, error) {
38+
func New(ctx context.Context, options *Options) (_ *API, err error) {
3939
if options.EntitlementsUpdateInterval == 0 {
4040
options.EntitlementsUpdateInterval = 10 * time.Minute
4141
}
@@ -59,6 +59,11 @@ func New(ctx context.Context, options *Options) (*API, error) {
5959
AGPL: coderd.New(options.Options),
6060
Options: options,
6161
}
62+
defer func() {
63+
if err != nil {
64+
_ = api.Close()
65+
}
66+
}()
6267

6368
api.AGPL.Options.SetUserGroups = api.setUserGroups
6469

@@ -312,8 +317,12 @@ type API struct {
312317

313318
func (api *API) Close() error {
314319
api.cancel()
315-
_ = api.replicaManager.Close()
316-
_ = api.derpMesh.Close()
320+
if api.replicaManager != nil {
321+
_ = api.replicaManager.Close()
322+
}
323+
if api.derpMesh != nil {
324+
_ = api.derpMesh.Close()
325+
}
317326
return api.AGPL.Close()
318327
}
319328

@@ -410,6 +419,7 @@ func (api *API) updateEntitlements(ctx context.Context) error {
410419
// is actually changing.
411420
changed = false
412421
} else {
422+
_ = coordinator.Close()
413423
coordinator = haCoordinator
414424
}
415425

‎enterprise/coderd/coderdenttest/coderdenttest.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212

1313
"github.com/golang-jwt/jwt/v4"
1414
"github.com/google/uuid"
15-
"github.com/stretchr/testify/assert"
1615
"github.com/stretchr/testify/require"
1716

1817
"github.com/coder/coder/coderd/coderdtest"
@@ -58,6 +57,8 @@ func New(t *testing.T, options *Options) *codersdk.Client {
5857
}
5958

6059
func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *coderd.API) {
60+
t.Helper()
61+
6162
if options == nil {
6263
options = &Options{}
6364
}
@@ -77,7 +78,7 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c
7778
Keys: Keys,
7879
ProxyHealthInterval: options.ProxyHealthInterval,
7980
})
80-
assert.NoError(t, err)
81+
require.NoError(t, err)
8182
setHandler(coderAPI.AGPL.RootHandler)
8283
var provisionerCloser io.Closer = nopcloser{}
8384
if options.IncludeProvisionerDaemon {

‎enterprise/replicasync/replicasync_test.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@ func TestReplica(t *testing.T) {
3434
db, pubsub := dbtestutil.NewDB(t)
3535
closeChan := make(chan struct{}, 1)
3636
cancel, err := pubsub.Subscribe(replicasync.PubsubEvent, func(ctx context.Context, message []byte) {
37-
closeChan <- struct{}{}
37+
select {
38+
case closeChan <- struct{}{}:
39+
default:
40+
}
3841
})
3942
require.NoError(t, err)
4043
defer cancel()
@@ -70,6 +73,8 @@ func TestReplica(t *testing.T) {
7073
RelayAddress: "http://169.254.169.254",
7174
})
7275
require.NoError(t, err)
76+
defer server.Close()
77+
7378
require.Len(t, server.Regional(), 1)
7479
require.Equal(t, peer.ID, server.Regional()[0].ID)
7580
require.Empty(t, server.Self().Error)
@@ -113,6 +118,8 @@ func TestReplica(t *testing.T) {
113118
TLSConfig: tlsConfig,
114119
})
115120
require.NoError(t, err)
121+
defer server.Close()
122+
116123
require.Len(t, server.Regional(), 1)
117124
require.Equal(t, peer.ID, server.Regional()[0].ID)
118125
require.Empty(t, server.Self().Error)
@@ -138,6 +145,8 @@ func TestReplica(t *testing.T) {
138145
RelayAddress: "http://127.0.0.1:1",
139146
})
140147
require.NoError(t, err)
148+
defer server.Close()
149+
141150
require.Len(t, server.Regional(), 1)
142151
require.Equal(t, peer.ID, server.Regional()[0].ID)
143152
require.NotEmpty(t, server.Self().Error)
@@ -152,6 +161,7 @@ func TestReplica(t *testing.T) {
152161
defer cancelCtx()
153162
server, err := replicasync.New(ctx, slogtest.Make(t, nil), db, pubsub, nil)
154163
require.NoError(t, err)
164+
defer server.Close()
155165
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
156166
w.WriteHeader(http.StatusOK)
157167
}))

0 commit comments

Comments
 (0)