Skip to content

Commit 1adfee6

Browse files
authored
deprecate collections and environment packages (#1837)
* deprecate collections and environment, migrate internal callers Per the Terratest V2 proposal, the collections and environment packages are scheduled for removal in v2 because the standard library now covers them. - Add // Deprecated: notices to every exported symbol in both packages, pointing at the stdlib replacement (slices, strings, os.Getenv). - Migrate the in-repo callers off them so the deprecations do not trip staticcheck SA1019 in CI: aws, azure, gcp, k8s, and test-structure now use the standard library directly via small local helpers. No behavior change. The packages keep working for the rest of v1. * test: cover GetNameFromResourceIDE error path GetNameFromResourceIDE now owns its error construction instead of delegating to the deprecated collections package, so add a direct test for both the success and the no-separator error path. * refactor: share list helpers via internal/collections Replace the per-package listIntersection/listSubtract helpers with a single internal/collections package (generic Intersection/Subtract over slices), imported by aws, azure, gcp, and test-structure. Keeps the logic in one tested place without re-exposing it as public API. * Address review feedback on deprecations and helpers - internal/collections: switch Intersection/Subtract to map lookups; table-driven tests + Example tests - azure: typed ResourceIDNameNotFoundError; resolve name via strings.LastIndex instead of strings.Split; require over assert for error checks - collections/environment: deprecation comments now show the concrete stdlib replacement inline - gcp: use require.NotEmptyf instead of t.Fatalf * azure: treat trailing-slash resource IDs as parse errors A value like "foo/bar/" previously returned an empty name with a nil error; return the not-found error when the final segment is empty too. (CodeRabbit review.) * Address review follow-ups from two-agent review - azure: export ResourceIDNameNotFoundError.ResourceID so callers can inspect it; cover the trailing-slash behavior change with tests and assert the typed error via errors.As - internal/collections: add nil-input and duplicate-in-list2 test cases; comment the delete-based dedup - collections: note the error/bounds semantics the stringslice deprecation snippets don't reproduce - environment: finish the dangling GetFirstNonEmptyEnvVarOrFatal doc sentence - gcp: mark firstNonEmptyEnvVarOrFatal as a test helper * collections: clarify deprecation has no public replacement ListIntersection/ListSubtract are not re-exposed publicly in v2 (the package is dropped); say so explicitly so the inline slices guidance reads as intentional rather than hiding a replacement. * Fix lint failures: wsl whitespace and deprecated environment usage - gcp/provider.go, azure/resourceid_test.go: add the blank lines wsl_v5 requires after t.Helper()/t.Parallel() before the following assignment - slack/validate_test.go: stop calling the now-deprecated environment.RequireEnvVar (SA1019); use require.NotEmptyf with os.Getenv directly Verified with golangci-lint 2.11.3: 0 issues.
1 parent 19735de commit 1adfee6

18 files changed

Lines changed: 316 additions & 35 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Package collections holds small generic slice helpers shared across
2+
// Terratest's own packages. It is internal and not part of the public API; the
3+
// public modules/collections package is deprecated and scheduled for removal in
4+
// v2.
5+
package collections
6+
7+
// Intersection returns the items present in both lists, de-duplicated, in the
8+
// order they appear in list1.
9+
func Intersection[T comparable](list1 []T, list2 []T) []T {
10+
lookups := make(map[T]struct{}, len(list2))
11+
for _, item := range list2 {
12+
lookups[item] = struct{}{}
13+
}
14+
15+
out := make([]T, 0, min(len(list1), len(list2)))
16+
17+
for _, item := range list1 {
18+
if _, found := lookups[item]; found {
19+
out = append(out, item)
20+
delete(lookups, item) // delete so a repeated list1 item isn't emitted twice
21+
}
22+
}
23+
24+
return out
25+
}
26+
27+
// Subtract returns the items in list1 that are not in list2.
28+
func Subtract[T comparable](list1 []T, list2 []T) []T {
29+
lookups := make(map[T]struct{}, len(list2))
30+
for _, item := range list2 {
31+
lookups[item] = struct{}{}
32+
}
33+
34+
out := make([]T, 0, len(list1))
35+
36+
for _, item := range list1 {
37+
if _, found := lookups[item]; !found {
38+
out = append(out, item)
39+
}
40+
}
41+
42+
return out
43+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package collections_test
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/gruntwork-io/terratest/internal/collections"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestIntersection(t *testing.T) {
12+
t.Parallel()
13+
14+
tests := []struct {
15+
name string
16+
list1 []string
17+
list2 []string
18+
want []string
19+
}{
20+
{"common items, ordered by list1", []string{"a", "b", "c"}, []string{"b", "c", "d"}, []string{"b", "c"}},
21+
{"dedups output", []string{"a", "a"}, []string{"a"}, []string{"a"}},
22+
{"dedups duplicates in list2", []string{"a"}, []string{"a", "a"}, []string{"a"}},
23+
{"no overlap returns empty, not nil", []string{"a"}, []string{"b"}, []string{}},
24+
{"nil inputs return empty, not nil", nil, nil, []string{}},
25+
}
26+
27+
for _, tc := range tests {
28+
t.Run(tc.name, func(t *testing.T) {
29+
t.Parallel()
30+
assert.Equal(t, tc.want, collections.Intersection(tc.list1, tc.list2))
31+
})
32+
}
33+
}
34+
35+
func TestSubtract(t *testing.T) {
36+
t.Parallel()
37+
38+
tests := []struct {
39+
name string
40+
list1 []string
41+
list2 []string
42+
want []string
43+
}{
44+
{"removes list2 items", []string{"a", "b", "c"}, []string{"b", "c"}, []string{"a"}},
45+
{"everything removed returns empty, not nil", []string{"a", "b"}, []string{"a", "b"}, []string{}},
46+
{"nil list1 returns empty, not nil", nil, []string{"a"}, []string{}},
47+
{"nil list2 keeps list1", []string{"a", "b"}, nil, []string{"a", "b"}},
48+
}
49+
50+
for _, tc := range tests {
51+
t.Run(tc.name, func(t *testing.T) {
52+
t.Parallel()
53+
assert.Equal(t, tc.want, collections.Subtract(tc.list1, tc.list2))
54+
})
55+
}
56+
}
57+
58+
func TestSubtractDoesNotMutateInput(t *testing.T) {
59+
t.Parallel()
60+
61+
in := []string{"a", "b"}
62+
collections.Subtract(in, []string{"a"})
63+
assert.Equal(t, []string{"a", "b"}, in, "does not mutate the input slice")
64+
}
65+
66+
func ExampleIntersection() {
67+
fmt.Println(collections.Intersection([]int{1, 2, 3}, []int{2, 3, 4}))
68+
// Output: [2 3]
69+
}
70+
71+
func ExampleSubtract() {
72+
fmt.Println(collections.Subtract([]int{1, 2, 3}, []int{2, 3}))
73+
// Output: [1]
74+
}

modules/aws/region.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"github.com/aws/aws-sdk-go-v2/aws"
99
"github.com/aws/aws-sdk-go-v2/service/ec2"
1010
"github.com/aws/aws-sdk-go-v2/service/ssm"
11-
"github.com/gruntwork-io/terratest/modules/collections"
11+
"github.com/gruntwork-io/terratest/internal/collections"
1212
"github.com/gruntwork-io/terratest/modules/logger"
1313
"github.com/gruntwork-io/terratest/modules/random"
1414
"github.com/gruntwork-io/terratest/modules/testing"
@@ -53,11 +53,11 @@ func GetRandomStableRegionContextE(t testing.TestingT, ctx context.Context, appr
5353
regionsToPickFrom := stableRegions
5454

5555
if len(approvedRegions) > 0 {
56-
regionsToPickFrom = collections.ListIntersection(regionsToPickFrom, approvedRegions)
56+
regionsToPickFrom = collections.Intersection(regionsToPickFrom, approvedRegions)
5757
}
5858

5959
if len(forbiddenRegions) > 0 {
60-
regionsToPickFrom = collections.ListSubtract(regionsToPickFrom, forbiddenRegions)
60+
regionsToPickFrom = collections.Subtract(regionsToPickFrom, forbiddenRegions)
6161
}
6262

6363
return GetRandomRegionContextE(t, ctx, regionsToPickFrom, nil)
@@ -122,7 +122,7 @@ func GetRandomRegionContextE(t testing.TestingT, ctx context.Context, approvedRe
122122
regionsToPickFrom = allRegions
123123
}
124124

125-
regionsToPickFrom = collections.ListSubtract(regionsToPickFrom, forbiddenRegions)
125+
regionsToPickFrom = collections.Subtract(regionsToPickFrom, forbiddenRegions)
126126
region := random.RandomString(regionsToPickFrom)
127127

128128
logger.Default.Logf(t, "Using region %s", region)

modules/azure/errors.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,21 @@ func (e *UnknownEnvironmentError) Error() string {
7272
e.EnvironmentName)
7373
}
7474

75+
// ResourceIDNameNotFoundError is returned when a name cannot be resolved from an Azure resource ID.
76+
// The offending ID is exported so callers can inspect it via errors.As.
77+
type ResourceIDNameNotFoundError struct {
78+
ResourceID string
79+
}
80+
81+
func (err ResourceIDNameNotFoundError) Error() string {
82+
return fmt.Sprintf("could not resolve name from resource ID %q", err.ResourceID)
83+
}
84+
85+
// NewResourceIDNameNotFoundError creates a ResourceIDNameNotFoundError for the given resource ID.
86+
func NewResourceIDNameNotFoundError(resourceID string) ResourceIDNameNotFoundError {
87+
return ResourceIDNameNotFoundError{ResourceID: resourceID}
88+
}
89+
7590
// ResourceNotFoundErrorExists checks the Service Error Code for the 'Resource Not Found' error.
7691
func ResourceNotFoundErrorExists(err error) bool {
7792
if err == nil {

modules/azure/region.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package azure
33
import (
44
"context"
55

6-
"github.com/gruntwork-io/terratest/modules/collections"
6+
"github.com/gruntwork-io/terratest/internal/collections"
77
"github.com/gruntwork-io/terratest/modules/random"
88
"github.com/gruntwork-io/terratest/modules/testing"
99
"github.com/stretchr/testify/require"
@@ -72,11 +72,11 @@ func GetRandomStableRegionContext(t testing.TestingT, ctx context.Context, appro
7272
regionsToPickFrom := stableRegions
7373

7474
if len(approvedRegions) > 0 {
75-
regionsToPickFrom = collections.ListIntersection(regionsToPickFrom, approvedRegions)
75+
regionsToPickFrom = collections.Intersection(regionsToPickFrom, approvedRegions)
7676
}
7777

7878
if len(forbiddenRegions) > 0 {
79-
regionsToPickFrom = collections.ListSubtract(regionsToPickFrom, forbiddenRegions)
79+
regionsToPickFrom = collections.Subtract(regionsToPickFrom, forbiddenRegions)
8080
}
8181

8282
return GetRandomRegionContext(t, ctx, regionsToPickFrom, nil, subscriptionID) //nolint:staticcheck
@@ -145,7 +145,7 @@ func GetRandomRegionContextE(t testing.TestingT, ctx context.Context, approvedRe
145145
regionsToPickFrom = allRegions
146146
}
147147

148-
regionsToPickFrom = collections.ListSubtract(regionsToPickFrom, forbiddenRegions)
148+
regionsToPickFrom = collections.Subtract(regionsToPickFrom, forbiddenRegions)
149149
region := random.RandomString(regionsToPickFrom)
150150

151151
return region, nil

modules/azure/resourceid.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package azure
22

3-
import "github.com/gruntwork-io/terratest/modules/collections"
3+
import (
4+
"strings"
5+
)
46

57
// GetNameFromResourceID gets the Name from an Azure Resource ID.
68
func GetNameFromResourceID(resourceID string) string {
@@ -15,10 +17,10 @@ func GetNameFromResourceID(resourceID string) string {
1517
// GetNameFromResourceIDE gets the Name from an Azure Resource ID.
1618
// This function would fail the test if there is an error.
1719
func GetNameFromResourceIDE(resourceID string) (string, error) {
18-
id, err := collections.GetSliceLastValueE(resourceID, "/")
19-
if err != nil {
20-
return "", err
20+
i := strings.LastIndex(resourceID, "/")
21+
if i == -1 || i == len(resourceID)-1 {
22+
return "", NewResourceIDNameNotFoundError(resourceID)
2123
}
2224

23-
return id, nil
25+
return resourceID[i+1:], nil
2426
}

modules/azure/resourceid_test.go

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,57 @@ import (
55

66
"github.com/gruntwork-io/terratest/modules/azure"
77
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
89
)
910

1011
func TestGetNameFromResourceID(t *testing.T) {
1112
t.Parallel()
1213

13-
resultSuccess := azure.GetNameFromResourceID("this/is/a/long/slash/separated/string/ResourceID")
14-
assert.Equal(t, "ResourceID", resultSuccess)
14+
tests := []struct {
15+
name string
16+
resourceID string
17+
want string
18+
}{
19+
{"normal resource ID", "this/is/a/long/slash/separated/string/ResourceID", "ResourceID"},
20+
{"no separator", "noresourcepresent", ""},
21+
{"trailing slash", "this/is/a/ResourceID/", ""},
22+
{"empty", "", ""},
23+
}
1524

16-
resultBadSeparator := azure.GetNameFromResourceID("noresourcepresent")
17-
assert.Empty(t, resultBadSeparator)
25+
for _, tc := range tests {
26+
t.Run(tc.name, func(t *testing.T) {
27+
t.Parallel()
28+
assert.Equal(t, tc.want, azure.GetNameFromResourceID(tc.resourceID))
29+
})
30+
}
31+
}
32+
33+
func TestGetNameFromResourceIDE(t *testing.T) {
34+
t.Parallel()
35+
36+
name, err := azure.GetNameFromResourceIDE("this/is/a/long/slash/separated/string/ResourceID")
37+
require.NoError(t, err)
38+
assert.Equal(t, "ResourceID", name)
39+
40+
tests := []struct {
41+
name string
42+
resourceID string
43+
}{
44+
{"no separator", "noresourcepresent"},
45+
{"trailing slash", "this/is/a/ResourceID/"},
46+
{"empty", ""},
47+
}
48+
49+
for _, tc := range tests {
50+
t.Run(tc.name, func(t *testing.T) {
51+
t.Parallel()
52+
53+
_, err := azure.GetNameFromResourceIDE(tc.resourceID)
54+
require.Error(t, err)
55+
56+
var notFound azure.ResourceIDNameNotFoundError
57+
require.ErrorAs(t, err, &notFound)
58+
assert.Equal(t, tc.resourceID, notFound.ResourceID)
59+
})
60+
}
1861
}

modules/collections/collections.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,10 @@
11
// Package collections allows to interact with lists of things.
2+
//
3+
// Deprecated: The collections package is scheduled for removal in Terratest v2.
4+
// Go's standard library covers these helpers as of Go 1.21+. Replace at the call
5+
// site:
6+
//
7+
// ListContains(haystack, needle) -> slices.Contains(haystack, needle)
8+
// ListIntersection / ListSubtract -> a short slices.Contains loop (see each function)
9+
// GetSliceLastValueE / GetSliceIndexValueE -> strings.Split, then index the result
210
package collections

modules/collections/errors.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package collections
22

33
// SliceValueNotFoundError is returned when a provided values file input is not found on the host path.
4+
//
5+
// Deprecated: scheduled for removal in Terratest v2 along with the collections package.
46
type SliceValueNotFoundError struct {
57
sourceString string
68
}
@@ -10,6 +12,8 @@ func (err SliceValueNotFoundError) Error() string {
1012
}
1113

1214
// NewSliceValueNotFoundError creates a new slice found error
15+
//
16+
// Deprecated: scheduled for removal in Terratest v2 along with the collections package.
1317
func NewSliceValueNotFoundError(sourceString string) SliceValueNotFoundError {
1418
return SliceValueNotFoundError{sourceString}
1519
}

modules/collections/lists.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,17 @@ import "slices"
44

55
// ListIntersection returns all the items in both list1 and list2. Note that this will dedup the items so that the
66
// output is more predictable. Otherwise, the end list depends on which list was used as the base.
7+
//
8+
// Deprecated: scheduled for removal in Terratest v2. The collections package is being
9+
// dropped, so there is no drop-in public replacement; build it inline with the slices
10+
// package (Go 1.21+) at the call site, e.g.:
11+
//
12+
// out := []T{}
13+
// for _, x := range list1 {
14+
// if slices.Contains(list2, x) && !slices.Contains(out, x) {
15+
// out = append(out, x)
16+
// }
17+
// }
718
func ListIntersection[T comparable](list1 []T, list2 []T) []T {
819
out := []T{}
920

@@ -18,6 +29,17 @@ func ListIntersection[T comparable](list1 []T, list2 []T) []T {
1829
}
1930

2031
// ListSubtract removes all the items in list2 from list1.
32+
//
33+
// Deprecated: scheduled for removal in Terratest v2. The collections package is being
34+
// dropped, so there is no drop-in public replacement; build it inline with the slices
35+
// package (Go 1.21+) at the call site, e.g.:
36+
//
37+
// out := []T{}
38+
// for _, x := range list1 {
39+
// if !slices.Contains(list2, x) {
40+
// out = append(out, x)
41+
// }
42+
// }
2143
func ListSubtract[T comparable](list1 []T, list2 []T) []T {
2244
out := []T{}
2345

@@ -32,7 +54,8 @@ func ListSubtract[T comparable](list1 []T, list2 []T) []T {
3254

3355
// ListContains returns true if the given list of strings (haystack) contains the given string (needle).
3456
//
35-
// Deprecated: Use slices.Contains instead.
57+
// Deprecated: scheduled for removal in Terratest v2. Replace at the call site with
58+
// slices.Contains(haystack, needle) (Go 1.21+).
3659
func ListContains(haystack []string, needle string) bool {
3760
return slices.Contains(haystack, needle)
3861
}

0 commit comments

Comments
 (0)