Skip to content

Commit 95249c0

Browse files
committed
direct/testserver: improve app test realism
Make testserver app DELETE asynchronous (sets DELETING state) to match real API behaviour. AppsGet auto-removes the app after returning it in DELETING state, so the next request sees it as gone. app_test.go: use real SDK calls (Create + DeleteByName) to put the app in DELETING state before testing DoCreate retry logic, instead of injecting state directly. RetriesWhenGetReturnsNotFound uses a one-shot POST override so GET returns 404 naturally without a custom GET handler. all_test.go: retry DoRead once after DoDelete to let async deletions (DELETING → gone) clear before asserting the resource is absent. Add Server.GetWorkspace to testserver for pre-seeding state in tests. Co-authored-by: Denis Bilenko
1 parent 21c9c47 commit 95249c0

5 files changed

Lines changed: 68 additions & 82 deletions

File tree

bundle/direct/dresources/all_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,6 +904,11 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
904904
deleteIsNoop := strings.HasSuffix(group, "permissions") || strings.HasSuffix(group, "grants")
905905

906906
remoteAfterDelete, err := adapter.DoRead(ctx, createdID)
907+
// Some resources delete asynchronously (e.g. apps transition through a
908+
// DELETING state). Read once more to let that pending state clear.
909+
if err == nil && remoteAfterDelete != nil && !deleteIsNoop {
910+
remoteAfterDelete, err = adapter.DoRead(ctx, createdID)
911+
}
907912
if deleteIsNoop {
908913
require.NoError(t, err)
909914
} else {

bundle/direct/dresources/app_test.go

Lines changed: 19 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -17,47 +17,6 @@ import (
1717
// an app already exists but is in DELETING state.
1818
func TestAppDoCreate_RetriesWhenAppIsDeleting(t *testing.T) {
1919
server := testserver.New(t)
20-
21-
createCallCount := 0
22-
getCallCount := 0
23-
24-
server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
25-
createCallCount++
26-
if createCallCount == 1 {
27-
return testserver.Response{
28-
StatusCode: 409,
29-
Body: map[string]string{
30-
"error_code": "RESOURCE_ALREADY_EXISTS",
31-
"message": "An app with the same name already exists.",
32-
},
33-
}
34-
}
35-
return apps.App{
36-
Name: "test-app",
37-
ComputeStatus: &apps.ComputeStatus{
38-
State: apps.ComputeStateActive,
39-
},
40-
}
41-
})
42-
43-
server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
44-
getCallCount++
45-
if getCallCount == 1 {
46-
return apps.App{
47-
Name: req.Vars["name"],
48-
ComputeStatus: &apps.ComputeStatus{
49-
State: apps.ComputeStateDeleting,
50-
},
51-
}
52-
}
53-
return apps.App{
54-
Name: req.Vars["name"],
55-
ComputeStatus: &apps.ComputeStatus{
56-
State: apps.ComputeStateActive,
57-
},
58-
}
59-
})
60-
6120
testserver.AddDefaultHandlers(server)
6221

6322
client, err := databricks.NewWorkspaceClient(&databricks.Config{
@@ -66,60 +25,42 @@ func TestAppDoCreate_RetriesWhenAppIsDeleting(t *testing.T) {
6625
})
6726
require.NoError(t, err)
6827

69-
r := (&ResourceApp{}).New(client)
7028
ctx := t.Context()
29+
30+
// Create then delete an app to put it in DELETING state.
31+
// The testserver's DELETE is asynchronous: it sets DELETING rather than
32+
// removing immediately, so the retry create will find the app in that state.
33+
_, err = client.Apps.Create(ctx, apps.CreateAppRequest{App: apps.App{Name: "test-app"}})
34+
require.NoError(t, err)
35+
_, err = client.Apps.DeleteByName(ctx, "test-app")
36+
require.NoError(t, err)
37+
38+
r := (&ResourceApp{}).New(client)
7139
name, _, err := r.DoCreate(ctx, NewNopEngine(reflect.TypeFor[*AppState]()), &AppState{App: apps.App{Name: "test-app"}})
7240

7341
require.NoError(t, err)
7442
assert.Equal(t, "test-app", name)
75-
assert.Equal(t, 2, createCallCount, "expected Create to be called twice (1 retry)")
76-
assert.Equal(t, 2, getCallCount, "expected Get to be called twice: once to check app state, once by waitForApp")
7743
}
7844

7945
// TestAppDoCreate_RetriesWhenGetReturnsNotFound verifies that DoCreate retries
8046
// when the app was just deleted between the create call and the get call.
8147
func TestAppDoCreate_RetriesWhenGetReturnsNotFound(t *testing.T) {
8248
server := testserver.New(t)
8349

84-
createCallCount := 0
85-
getCallCount := 0
86-
50+
// Simulate a race: the app existed when Create was called (returns 409) but
51+
// was deleted before the existence check (GET returns 404). The first POST
52+
// returns 409 without storing anything so the standard GET handler returns
53+
// 404 naturally, and the retry POST creates the app normally.
54+
rejectedOnce := false
8755
server.Handle("POST", "/api/2.0/apps", func(req testserver.Request) any {
88-
createCallCount++
89-
if createCallCount == 1 {
56+
if !rejectedOnce {
57+
rejectedOnce = true
9058
return testserver.Response{
9159
StatusCode: 409,
92-
Body: map[string]string{
93-
"error_code": "RESOURCE_ALREADY_EXISTS",
94-
"message": "An app with the same name already exists.",
95-
},
60+
Body: map[string]string{"error_code": "RESOURCE_ALREADY_EXISTS", "message": "An app with the same name already exists."},
9661
}
9762
}
98-
return apps.App{
99-
Name: "test-app",
100-
ComputeStatus: &apps.ComputeStatus{
101-
State: apps.ComputeStateActive,
102-
},
103-
}
104-
})
105-
106-
server.Handle("GET", "/api/2.0/apps/{name}", func(req testserver.Request) any {
107-
getCallCount++
108-
if getCallCount == 1 {
109-
return testserver.Response{
110-
StatusCode: 404,
111-
Body: map[string]string{
112-
"error_code": "RESOURCE_DOES_NOT_EXIST",
113-
"message": "App not found.",
114-
},
115-
}
116-
}
117-
return apps.App{
118-
Name: req.Vars["name"],
119-
ComputeStatus: &apps.ComputeStatus{
120-
State: apps.ComputeStateActive,
121-
},
122-
}
63+
return req.Workspace.AppsUpsert(req, "")
12364
})
12465

12566
testserver.AddDefaultHandlers(server)
@@ -136,8 +77,6 @@ func TestAppDoCreate_RetriesWhenGetReturnsNotFound(t *testing.T) {
13677

13778
require.NoError(t, err)
13879
assert.Equal(t, "test-app", name)
139-
assert.Equal(t, 2, createCallCount, "expected Create to be called twice")
140-
assert.Equal(t, 2, getCallCount, "expected Get to be called twice: once to check app state, once by waitForApp")
14180
}
14281

14382
func TestAppDoUpdate_UpdateMaskHasAllFields(t *testing.T) {

libs/testserver/apps.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,42 @@ func (s *FakeWorkspace) AppsCreateUpdate(req Request, name string) Response {
7272
}
7373
}
7474

75+
func (s *FakeWorkspace) AppsGet(_ Request, name string) Response {
76+
defer s.LockUnlock()()
77+
78+
app, ok := s.Apps[name]
79+
if !ok {
80+
return Response{StatusCode: 404, Body: map[string]string{"message": "App not found: " + name}}
81+
}
82+
83+
// When an app is in DELETING state, remove it from the store after returning.
84+
// This simulates deletion completing so the next create attempt succeeds.
85+
if app.ComputeStatus != nil && app.ComputeStatus.State == apps.ComputeStateDeleting {
86+
delete(s.Apps, name)
87+
}
88+
89+
return Response{Body: app}
90+
}
91+
92+
// AppsDelete marks an app as DELETING, simulating asynchronous deletion.
93+
// The app is removed from the store when it is next retrieved via AppsGet.
94+
func (s *FakeWorkspace) AppsDelete(_ Request, name string) Response {
95+
defer s.LockUnlock()()
96+
97+
app, ok := s.Apps[name]
98+
if !ok {
99+
return Response{StatusCode: 404}
100+
}
101+
102+
app.ComputeStatus = &apps.ComputeStatus{
103+
State: apps.ComputeStateDeleting,
104+
Message: "App compute is being deleted.",
105+
}
106+
s.Apps[name] = app
107+
108+
return Response{}
109+
}
110+
75111
func (s *FakeWorkspace) AppsGetUpdate(_ Request, name string) Response {
76112
defer s.LockUnlock()()
77113

libs/testserver/handlers.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ func AddDefaultHandlers(server *Server) {
404404
})
405405

406406
server.Handle("GET", "/api/2.0/apps/{name}", func(req Request) any {
407-
return MapGet(req.Workspace, req.Workspace.Apps, req.Vars["name"])
407+
return req.Workspace.AppsGet(req, req.Vars["name"])
408408
})
409409

410410
server.Handle("POST", "/api/2.0/apps", func(req Request) any {
@@ -416,7 +416,7 @@ func AddDefaultHandlers(server *Server) {
416416
})
417417

418418
server.Handle("DELETE", "/api/2.0/apps/{name}", func(req Request) any {
419-
return MapDelete(req.Workspace, req.Workspace.Apps, req.Vars["name"])
419+
return req.Workspace.AppsDelete(req, req.Vars["name"])
420420
})
421421

422422
// Schemas:

libs/testserver/server.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,12 @@ Response.Body = '<response body here>'
296296
return s
297297
}
298298

299+
// GetWorkspace returns (creating if necessary) the FakeWorkspace for the given token.
300+
// Use this in tests to pre-seed state before making requests.
301+
func (s *Server) GetWorkspace(token string) *FakeWorkspace {
302+
return s.getWorkspaceForToken(token)
303+
}
304+
299305
func (s *Server) getWorkspaceForToken(token string) *FakeWorkspace {
300306
if token == "" {
301307
return nil

0 commit comments

Comments
 (0)