Test sharding - take 2 (#2832)

Follow-up work on #2810. This PR aimed to validate the more complex test optimization strategy of #2810 (pre-calculated perfect allocation; implemented by github.com/pulumi/sharder) by demonstrating the effects of a simpler strategy (random allocation; implemented by github.com/pulumi/shard). Initial testing showed that both strategies were bounded by the 15 minute job [`Test and Lint / test (1.22.x, windows-latest, DEFAULT, 0)`](https://github.com/pulumi/pulumi-terraform-bridge/actions/runs/12770018688/job/35593903461?pr=2832#logs), which is not effected by sharding. Since both approaches appear identical, we will merge the simpler strategy. --------- Co-authored-by: Venelin <[email protected]>
pulumi · Jan 15, 2025 · cb11d40 · cb11d40
1 parent 1bec71d
commit cb11d40
Show file tree

Hide file tree

Showing 6 changed files with 179 additions and 996 deletions.
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
@@ -8,6 +8,9 @@ on:
         required: false
 env:
   GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+  # Note this needs to match the shard input to the test matrix below as well as pattern in exclude.
+  # see jobs.test.strategy.matrix.{shard,exclude}
+  TOTAL_SHARDS: 15
 
 jobs:
   test:
@@ -21,6 +24,72 @@ jobs:
         go-version: [1.22.x, 1.23.x]
         platform: [ubuntu-latest, macos-latest, windows-latest]
         feature-flags: ["DEFAULT", "PULUMI_TF_BRIDGE_ACCURATE_BRIDGE_PREVIEW"]
+        # Needs to match TOTAL_SHARDS
+        shard: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+        exclude:
+          # We do not run non-default feature flags on non-ubuntu. 
+          - platform: windows-latest
+            feature-flags: "PULUMI_TF_BRIDGE_ACCURATE_BRIDGE_PREVIEW"
+          - platform: macos-latest
+            feature-flags: "PULUMI_TF_BRIDGE_ACCURATE_BRIDGE_PREVIEW"
+          # Windows and mac test runs do not need to be sharded as they are fast enough.
+          # In order to do that we will skip all except the 0-th shard.
+          - platform: windows-latest
+            shard: 1
+          - platform: windows-latest
+            shard: 2
+          - platform: windows-latest
+            shard: 3
+          - platform: windows-latest
+            shard: 4
+          - platform: windows-latest
+            shard: 5
+          - platform: windows-latest
+            shard: 6
+          - platform: windows-latest
+            shard: 7
+          - platform: windows-latest
+            shard: 8
+          - platform: windows-latest
+            shard: 9
+          - platform: windows-latest
+            shard: 10
+          - platform: windows-latest
+            shard: 11
+          - platform: windows-latest
+            shard: 12
+          - platform: windows-latest
+            shard: 13
+          - platform: windows-latest
+            shard: 14
+          - platform: macos-latest
+            shard: 1
+          - platform: macos-latest
+            shard: 2
+          - platform: macos-latest
+            shard: 3
+          - platform: macos-latest
+            shard: 4
+          - platform: macos-latest
+            shard: 5
+          - platform: macos-latest
+            shard: 6
+          - platform: macos-latest
+            shard: 7
+          - platform: macos-latest
+            shard: 8
+          - platform: macos-latest
+            shard: 9
+          - platform: macos-latest
+            shard: 10
+          - platform: macos-latest
+            shard: 11
+          - platform: macos-latest
+            shard: 12
+          - platform: macos-latest
+            shard: 13
+          - platform: macos-latest
+            shard: 14
     runs-on: ${{ matrix.platform }}
     steps:
       - name: Install pulumi
@@ -39,16 +108,19 @@ jobs:
           go-version: ${{ matrix.go-version }}
           cache-dependency-path: |
             **/go.sum
+          # disable caching on windows because it's very slow
+          # see https://github.com/actions/setup-go/issues/495
+          cache: ${{ matrix.platform != 'windows-latest' }}
       - name: export feature flags
         run: echo ${{ matrix.feature-flags }}=true >> $GITHUB_ENV
         if: ${{ matrix.platform != 'windows-latest' && matrix.feature-flags != 'DEFAULT' }}
-      - name: export feature flags
-        run: echo ${{ matrix.feature-flags }}=true >> $env:GITHUB_ENV
-        if: ${{ matrix.platform == 'windows-latest' && matrix.feature-flags != 'DEFAULT' }}
       - name: Build
         run: make build
       - name: Build PF
         run: cd pkg/pf && make build
+      - name: Shard tests
+        run: echo "RUN_TEST_CMD=$(go run github.com/pulumi/shard@5b6297aaffa0c06291fb8231968d7a9f4e6832e6 --total ${{ env.TOTAL_SHARDS }} --index ${{ matrix.shard }} --seed 314)" >> $GITHUB_ENV
+        if: ${{ matrix.platform == 'ubuntu-latest' }}
       - name: Test
         run: make test
       - name: Upload coverage reports to Codecov
@@ -78,3 +150,25 @@ jobs:
           version: v1.62
       - name: Lint
         run: make lint
+  sentinel:
+    name: sentinel
+    if: github.event_name == 'repository_dispatch' ||
+      github.event.pull_request.head.repo.full_name == github.repository
+    permissions:
+      statuses: write
+    needs:
+    - test
+    - lint
+    runs-on: ubuntu-latest
+    steps:
+    - uses: guibranco/github-status-action-v2@0849440ec82c5fa69b2377725b9b7852a3977e76 # v1.1.13
+      with:
+        authToken: ${{secrets.GITHUB_TOKEN}}
+        # Write an explicit status check called "Sentinel" which will only pass if this code really runs.
+        # This should always be a required check for PRs.
+        context: 'Sentinel'
+        description: 'All required checks passed'
+        state: 'success'
+        # Write to the PR commit SHA if it's available as we don't want the merge commit sha,
+        # otherwise use the current SHA for any other type of build.
+        sha: ${{ github.event.pull_request.head.sha || github.sha }}
diff --git a/Makefile b/Makefile
@@ -27,11 +27,12 @@ lint:
 lint_fix:
 	go run scripts/build.go fix-lint
 
+RUN_TEST_CMD ?= ./...
 test:: install_plugins
 	@mkdir -p bin
 	go build -o bin ./internal/testing/pulumi-terraform-bridge-test-provider
 	PULUMI_TERRAFORM_BRIDGE_TEST_PROVIDER=$(shell pwd)/bin/pulumi-terraform-bridge-test-provider \
-		go test -count=1 -coverprofile="coverage.txt" -coverpkg=./... -timeout 2h -parallel ${TESTPARALLELISM} ./...
+		go test -count=1 -coverprofile="coverage.txt" -coverpkg=./... -timeout 2h -parallel ${TESTPARALLELISM} $(value RUN_TEST_CMD)
 
 # Run tests while accepting current output as expected output "golden"
 # tests. In case where system behavior changes intentionally this can

diff --git a/pkg/internal/tests/pulcheck/pulcheck.go b/pkg/internal/tests/pulcheck/pulcheck.go
@@ -52,8 +52,39 @@ func resourceNeedsUpdate(res *schema.Resource) bool {
 	return false
 }
 
+func copyMap[K comparable, V any](m map[K]V, cp func(V) V) map[K]V {
+	dst := make(map[K]V, len(m))
+	for k, v := range m {
+		dst[k] = cp(v)
+	}
+	return dst
+}
+
+// WithValidProvider returns a copy of tfp, with all required fields filled with default
+// values.
+//
 // This is an experimental API.
-func EnsureProviderValid(t T, tfp *schema.Provider) {
+func WithValidProvider(t T, tfp *schema.Provider) *schema.Provider {
+	if tfp == nil {
+		return nil
+	}
+
+	// Copy tfp as deep as we will mutate.
+	{
+		dst := *tfp // memcopy
+		dst.ResourcesMap = copyMap(tfp.ResourcesMap, func(v *schema.Resource) *schema.Resource {
+			cp := *v // memcopy
+			cp.Schema = copyMap(cp.Schema, func(s *schema.Schema) *schema.Schema {
+				cp := *s
+				return &cp
+			})
+			return &cp
+		})
+		tfp = &dst
+	}
+
+	// Now ensure that tfp is valid
+
 	for _, r := range tfp.ResourcesMap {
 		if r.Schema["id"] == nil {
 			r.Schema["id"] = &schema.Schema{
@@ -108,6 +139,8 @@ func EnsureProviderValid(t T, tfp *schema.Provider) {
 		}
 	}
 	require.NoError(t, tfp.InternalValidate())
+
+	return tfp
 }
 
 func ProviderServerFromInfo(
@@ -206,7 +239,7 @@ func BridgedProvider(t T, providerName string, tfp *schema.Provider, opts ...Bri
 		opt(&options)
 	}
 
-	EnsureProviderValid(t, tfp)
+	tfp = WithValidProvider(t, tfp)
 
 	// If the PULUMI_ACCURATE_BRIDGE_PREVIEWS environment variable is set, use it to enable
 	// accurate bridge previews.
@@ -230,10 +263,8 @@ func BridgedProvider(t T, providerName string, tfp *schema.Provider, opts ...Bri
 		EnableAccurateBridgePreview:    accurateBridgePreviews,
 		Config:                         options.configInfo,
 	}
-	makeToken := func(module, name string) (string, error) {
-		return tokens.MakeStandard(providerName)(module, name)
-	}
-	provider.MustComputeTokens(tokens.SingleModule(providerName, "index", makeToken))
+	provider.MustComputeTokens(tokens.SingleModule(providerName,
+		"index", tokens.MakeStandard(providerName)))
 
 	return provider
 }

diff --git a/pkg/tests/import_test.go b/pkg/tests/import_test.go
@@ -16,54 +16,57 @@ import (
 	"gopkg.in/yaml.v3"
 
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/internal/tests/pulcheck"
+	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
 )
 
 func TestFullyComputedNestedAttribute(t *testing.T) {
 	t.Parallel()
-	resMap := map[string]*schema.Resource{
-		"prov_test": {
-			Schema: map[string]*schema.Schema{
-				"attached_disks": {
-					Type:     schema.TypeList,
-					Optional: true,
-					Elem: &schema.Resource{
-						Schema: map[string]*schema.Schema{
-							"name": {
-								Optional: true,
-								Type:     schema.TypeString,
-							},
-							"key256": {
-								Computed: true,
-								Type:     schema.TypeString,
+
+	bridgedProvider := func(importVal any) info.Provider {
+		return pulcheck.BridgedProvider(t, "prov", &schema.Provider{
+			ResourcesMap: map[string]*schema.Resource{
+				"prov_test": {
+					Schema: map[string]*schema.Schema{
+						"attached_disks": {
+							Type:     schema.TypeList,
+							Optional: true,
+							Elem: &schema.Resource{
+								Schema: map[string]*schema.Schema{
+									"name": {
+										Optional: true,
+										Type:     schema.TypeString,
+									},
+									"key256": {
+										Computed: true,
+										Type:     schema.TypeString,
+									},
+								},
 							},
 						},
+						"top_level_computed": {
+							Type:     schema.TypeString,
+							Computed: true,
+						},
 					},
-				},
-				"top_level_computed": {
-					Type:     schema.TypeString,
-					Computed: true,
-				},
-			},
-		},
-	}
-
-	importer := func(val any) func(context.Context, *schema.ResourceData, interface{}) ([]*schema.ResourceData, error) {
-		return func(ctx context.Context, rd *schema.ResourceData, i interface{}) ([]*schema.ResourceData, error) {
-			elMap := map[string]any{
-				"name":   "disk1",
-				"key256": val,
-			}
-			err := rd.Set("attached_disks", []map[string]any{elMap})
-			require.NoError(t, err)
+					Importer: &schema.ResourceImporter{
+						StateContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) ([]*schema.ResourceData, error) {
+							elMap := map[string]any{
+								"name":   "disk1",
+								"key256": importVal,
+							}
+							err := rd.Set("attached_disks", []map[string]any{elMap})
+							require.NoError(t, err)
 
-			err = rd.Set("top_level_computed", "computed_val")
-			require.NoError(t, err)
+							err = rd.Set("top_level_computed", "computed_val")
+							require.NoError(t, err)
 
-			return []*schema.ResourceData{rd}, nil
-		}
+							return []*schema.ResourceData{rd}, nil
+						},
+					},
+				},
+			},
+		})
 	}
-	tfp := &schema.Provider{ResourcesMap: resMap}
-	bridgedProvider := pulcheck.BridgedProvider(t, "prov", tfp)
 
 	program := `
 name: test
@@ -83,9 +86,7 @@ runtime: yaml
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			resMap["prov_test"].Importer = &schema.ResourceImporter{
-				StateContext: importer(tc.importVal),
-			}
+			bridgedProvider := bridgedProvider(tc.importVal)
 
 			pt := pulcheck.PulCheck(t, bridgedProvider, program)
 

diff --git a/pkg/tests/tfcheck/tfcheck.go b/pkg/tests/tfcheck/tfcheck.go
@@ -76,7 +76,7 @@ func NewTfDriver(t pulcheck.T, dir, providerName string, prov any) *TFDriver {
 }
 
 func newTfDriverSDK(t pulcheck.T, dir, providerName string, prov *schema.Provider) *TFDriver {
-	pulcheck.EnsureProviderValid(t, prov)
+	prov = pulcheck.WithValidProvider(t, prov)
 	v6server, err := tf5to6server.UpgradeServer(context.Background(),
 		func() tfprotov5.ProviderServer { return prov.GRPCProvider() })
 	require.NoError(t, err)