Some fixups for helpers

build-support/scripts/check-allowed-imports.sh

@@ -52,6 +52,12 @@ function main {
       esac
    done
 
+   # If we could guarantee this ran with bash 4.2+ then the final argument could
+   # be just ${allowed_packages[@]}. However that with older versions of bash
+   # in combination with set -u causes bash to emit errors about using unbound
+   # variables when no allowed packages have been specified (i.e. the module should
+   # generally be disallowed with no exceptions). This syntax is very strange
+   # but seems to be the prescribed workaround I found.
    check_imports "$module_root" ${allowed_packages[@]+"${allowed_packages[@]}"}
    return $?
 }

internal/resource/resourcetest/builder.go

@@ -139,12 +139,12 @@ func (b *resourceBuilder) Write(t T, client pbresource.ResourceServiceClient) *p
 
 		if strings.Contains(err.Error(), storage.ErrWrongUid.Error()) {
 			r.Fatalf("resource write failed due to uid mismatch - most likely a transient issue when talking to a non-leader")
+		} else {
+			// other errors are unexpected and should cause an immediate failure
+			r.Stop(err)
 		}
-		// other failed precondition errors will be checked outside of the retry
 	})
 
-	require.NoError(t, err)
-
 	if !b.dontCleanup {
 		cleaner, ok := t.(CleanupT)
 		require.True(t, ok, "T does not implement a Cleanup method and cannot be used with automatic resource cleanup")

internal/resource/resourcetest/client.go

@@ -13,7 +13,6 @@ import (
 	"golang.org/x/exp/slices"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-	"google.golang.org/protobuf/proto"
 )
 
 type Client struct {
@@ -84,7 +83,7 @@ func (client *Client) PublishResources(t T, resources []*pbresource.Resource) {
 
 			cleaner, ok := t.(CleanupT)
 			if ok {
-				id := proto.Clone(rsp.Resource.Id).(*pbresource.ID)
+				id := rsp.Resource.Id
 				cleaner.Cleanup(func() {
 					client.MustDelete(t, id)
 				})
@@ -237,22 +236,19 @@ func (client *Client) ResolveResourceID(t T, id *pbresource.ID) *pbresource.ID {
 }
 
 func (client *Client) MustDelete(t T, id *pbresource.ID) {
+
 	client.retry(t, func(r *retry.R) {
 		_, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: id})
 		if status.Code(err) == codes.NotFound {
 			return
 		}
 
+		// codes.Aborted indicates a CAS failure and that the delete request should
+		// be retried. Anything else should be considered an unrecoverable error.
 		if err != nil && status.Code(err) != codes.Aborted {
 			r.Stop(fmt.Errorf("failed to delete the resource: %w", err))
 		}
 
 		require.NoError(r, err)
 	})
-	// _, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: id})
-	// if status.Code(err) == codes.NotFound {
-	// 	return
-	// }
-
-	// require.NoError(t, err)
 }