Skip to content

Commit

Permalink
chore: Review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@arthurpitman @jskelin
arthurpitman authored and jskelin committed Jan 3, 2025
1 parent 272444f commit 3c07df7
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
8 changes: 4 additions & 4 deletions pkg/delete/delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ func TestDelete_Segments(t *testing.T) {
}
err := delete.Configs(context.TODO(), client.ClientSet{SegmentClient: &c}, given)
assert.NoError(t, err)
assert.True(t, c.called, "there should be delete call")
assert.True(t, c.called, "delete should have been called")
})

t.Run("simple case with FF turned off", func(t *testing.T) {
Expand All @@ -1209,7 +1209,7 @@ func TestDelete_Segments(t *testing.T) {
}
err := delete.Configs(context.TODO(), client.ClientSet{SegmentClient: &c}, given)
assert.NoError(t, err)
assert.False(t, c.called, "there should NOT be delete call")
assert.False(t, c.called, "delete should not have been called")
})
}

Expand All @@ -1228,7 +1228,7 @@ func TestDeleteAll_Segments(t *testing.T) {

err := delete.All(context.TODO(), client.ClientSet{SegmentClient: &c}, api.APIs{})
assert.NoError(t, err)
assert.True(t, c.called, "there should be delete call")
assert.True(t, c.called, "delete should have been called")
})

t.Run("FF is turned off", func(t *testing.T) {
Expand All @@ -1238,6 +1238,6 @@ func TestDeleteAll_Segments(t *testing.T) {

err := delete.All(context.TODO(), client.ClientSet{SegmentClient: &c}, api.APIs{})
assert.NoError(t, err)
assert.False(t, c.called, "there should NOT be delete call")
assert.False(t, c.called, "delete should not have been called")
})
}
25 changes: 14 additions & 11 deletions pkg/delete/internal/segment/delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ func (s *stubClient) Delete(_ context.Context, id string) (libSegment.Response,
return s.delete(id)
}

func TestDelete(t *testing.T) {
func TestDeleteByCoordinate(t *testing.T) {

t.Run("delete via coordinate", func(t *testing.T) {
t.Run("success if one segment matches generated external ID", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
Identifier: "monaco_identifier",
Expand All @@ -72,7 +72,7 @@ func TestDelete(t *testing.T) {
assert.True(t, c.called, "delete command wasn't invoked")
})

t.Run("config declared via coordinate doesn't exists - no error (wanted state achieved)", func(t *testing.T) {
t.Run("no error if no segment matches generated external ID", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
Identifier: "monaco_identifier",
Expand All @@ -89,7 +89,7 @@ func TestDelete(t *testing.T) {
assert.NoError(t, err)
})

t.Run("config declared via coordinate have multiple match - an error", func(t *testing.T) {
t.Run("error if multiple segments match generated external ID", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
Identifier: "monaco_identifier",
Expand All @@ -108,7 +108,7 @@ func TestDelete(t *testing.T) {
assert.False(t, c.called, "it's not known what needs to be deleted")
})

t.Run("config declared via coordinate failed to get externalId - an error", func(t *testing.T) {
t.Run("error if list fails", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
Identifier: "monaco_identifier",
Expand All @@ -124,8 +124,11 @@ func TestDelete(t *testing.T) {
err := segment.Delete(context.TODO(), &c, []pointer.DeletePointer{given})
assert.Error(t, err)
})
}

func TestDeleteByObjectId(t *testing.T) {

t.Run("delete via originID", func(t *testing.T) {
t.Run("sucess if segment exists", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
OriginObjectId: "originObjectID",
Expand All @@ -143,7 +146,7 @@ func TestDelete(t *testing.T) {
assert.True(t, c.called)
})

t.Run("config declared via originID doesn't exists - no error (wanted state achieved)", func(t *testing.T) {
t.Run("no error if segment doesn't exist", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
OriginObjectId: "originObjectID",
Expand All @@ -160,7 +163,7 @@ func TestDelete(t *testing.T) {
assert.NoError(t, err)
})

t.Run("error during delete - an error", func(t *testing.T) {
t.Run("error if delete fails", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
OriginObjectId: "originObjectID",
Expand All @@ -177,7 +180,7 @@ func TestDelete(t *testing.T) {
assert.Error(t, err)
})

t.Run("server error during delete - an error", func(t *testing.T) {
t.Run("error if server error during delete", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
OriginObjectId: "originObjectID",
Expand All @@ -194,7 +197,7 @@ func TestDelete(t *testing.T) {
assert.Error(t, err)
})

t.Run("error during delete - continue to delete, an error", func(t *testing.T) {
t.Run("deletion continues even if error occurs", func(t *testing.T) {
given := pointer.DeletePointer{
Type: "segment",
OriginObjectId: "originObjectID",
Expand Down Expand Up @@ -231,7 +234,7 @@ func TestDeleteAll(t *testing.T) {
assert.NoError(t, err)
})

t.Run("error during delete - continue to delete, an error", func(t *testing.T) {
t.Run("deletion continues even if error occurs during delete", func(t *testing.T) {
c := stubClient{
list: func() (libSegment.Response, error) {
return libSegment.Response{Data: []byte(`[{"uid": "uid_1"},{"uid": "uid_2"},{"uid": "uid_3"}]`)}, nil
Expand Down

0 comments on commit 3c07df7

Please sign in to comment.