Lint upstream as a normal unit test (#4918)

blampe · web-flow · commit df65048bb989 · 2024-12-16T12:22:49.000-08:00
This removes the last job in `extraTests` by making it a standard unit test. Currently the job runs without caching and takes upwards of 30-40 minutes because it re-compiles everything from scratch. This makes it the long poll when sharding is [turned on](https://github.com/pulumi/pulumi-aws/actions/runs/12321334449). By running with the rest of our tests it can benefit from caching. Locally the test takes only ~20s. This is run during unit tests, which still take about ~5 minutes before and after this change. They're actually slightly faster because I parallelized some of them while I was in here.
diff --git a/.ci-mgmt.yaml b/.ci-mgmt.yaml
@@ -63,32 +63,3 @@ actions:
         role-to-assume: ${{ secrets.AWS_CI_ROLE_ARN }}
     - name: Make upstream
       run: make upstream
-
-extraTests:
-
-  upstream_lint:
-    name: Run upstream provider-lint
-    runs-on: ubuntu-latest
-    timeout-minutes: 60
-    steps:
-    - name: Free Disk Space (Ubuntu)
-      uses: jlumbroso/free-disk-space@main
-      with:
-        swap-storage: false
-        tool-cache: false
-    - name: Checkout Repo
-      uses: actions/checkout@v4
-      with:
-        ref: ${{ env.PR_COMMIT_SHA }}
-        submodules: true
-    - name: Install Go
-      uses: actions/setup-go@v5
-      with:
-        go-version: "1.23.x"
-        cache: false
-    - name: Prepare local workspace
-      run: make prepare_local_workspace
-    - name: upstream lint
-      run: |
-        cd upstream
-        make provider-lint
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -113,30 +113,3 @@ jobs:
         - go
         - java
         testTarget: [local]
-  upstream_lint:
-      name: Run upstream provider-lint
-      runs-on: ubuntu-latest
-      steps:
-          - name: Free Disk Space (Ubuntu)
-            uses: jlumbroso/free-disk-space@main
-            with:
-              swap-storage: false
-              tool-cache: false
-          - name: Checkout Repo
-            uses: actions/checkout@v4
-            with:
-              ref: ${{ env.PR_COMMIT_SHA }}
-              submodules: true
-          - name: Install Go
-            uses: actions/setup-go@v5
-            with:
-              cache: false
-              go-version: 1.23.x
-          - name: Prepare local workspace
-            run: make prepare_local_workspace
-          - name: upstream lint
-            run: |
-              cd upstream
-              make provider-lint
-      timeout-minutes: 60
-
diff --git a/patches/0079-Speed-up-providerlint-by-re-using-build-cache-and-ig.patch b/patches/0079-Speed-up-providerlint-by-re-using-build-cache-and-ig.patch
@@ -0,0 +1,24 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Bryce Lampe <bryce@pulumi.com>
+Date: Fri, 13 Dec 2024 13:44:02 -0800
+Subject: [PATCH] Speed up providerlint by re-using build cache and ignoring
+ tests
+
+
+diff --git a/GNUmakefile b/GNUmakefile
+index 9b4adecff8..32137c8919 100644
+--- a/GNUmakefile
++++ b/GNUmakefile
+@@ -326,9 +326,10 @@ prereq-go: ## If $(GO_VER) is not installed, install it
+ 
+ provider-lint: ## [CI] ProviderLint Checks / providerlint
+ 	@echo "make: ProviderLint Checks / providerlint..."
+-	@cd .ci/providerlint && go install -buildvcs=false .
+-	@providerlint \
++	@cd .ci/providerlint && go build -buildvcs=false .
++	@.ci/providerlint/providerlint \
+ 		-c 1 \
++		-test=false \
+ 		-AT001.ignored-filename-suffixes=_data_source_test.go \
+ 		-AWSAT006=false \
+ 		-AWSR002=false \
diff --git a/provider/cmd/pulumi-tfgen-aws/wafv2_test.go b/provider/cmd/pulumi-tfgen-aws/wafv2_test.go
@@ -10,6 +10,8 @@ import (
 )
 
 func TestReplaceWafV2TypesWithRecursive(t *testing.T) {
+	t.Parallel()
+
 	spec := schema.PackageSpec{
 		Types: map[string]schema.ComplexTypeSpec{
 			// Root statement - keep it.
diff --git a/provider/configure_test.go b/provider/configure_test.go
@@ -29,6 +29,8 @@ import (
 )
 
 func TestCheckConfigWithUnknownKeys(t *testing.T) {
+	t.Parallel()
+
 	// Double checking that this is a failure, but no longer over-fitting the test on the exact
 	// error message. See pulumi/pulumi-terraform-bridge codebase instead for controlling the
 	// generated error message.
@@ -57,6 +59,8 @@ func TestCheckConfigWithUnknownKeys(t *testing.T) {
 }
 
 func TestCheckConfigRunsUpstreamValidators(t *testing.T) {
+	t.Parallel()
+
 	replaySequence(t, strings.ReplaceAll(`
 	[{
 	  "method": "/pulumirpc.ResourceProvider/CheckConfig",
diff --git a/provider/diag_test.go b/provider/diag_test.go
@@ -10,6 +10,8 @@ import (
 )
 
 func TestFormatDiags(t *testing.T) {
+	t.Parallel()
+
 	type testCase struct {
 		name   string
 		config diag.Diagnostics
diff --git a/provider/diff_test.go b/provider/diff_test.go
@@ -27,6 +27,8 @@ import (
 //
 // See also pulumi/pulumi-aws#3650.
 func TestRegressLandingZoneDiff(t *testing.T) {
+	t.Parallel()
+
 	event := `
 	[{
 	  "method": "/pulumirpc.ResourceProvider/Diff",
@@ -66,6 +68,8 @@ func TestRegressLandingZoneDiff(t *testing.T) {
 }
 
 func TestRegress1738(t *testing.T) {
+	t.Parallel()
+
 	containerDefinitionsOld := `
 	[
 	  {
diff --git a/provider/enum_test.go b/provider/enum_test.go
@@ -9,6 +9,8 @@ import (
 )
 
 func TestInstanceTypeName(t *testing.T) {
+	t.Parallel()
+
 	type testCase struct {
 		Value string
 		Name  string
@@ -34,6 +36,8 @@ func TestInstanceTypeName(t *testing.T) {
 }
 
 func TestInstanceTypeNameErr(t *testing.T) {
+	t.Parallel()
+
 	testCases := []string{
 		"a1.metal.pc",
 	}
diff --git a/provider/fast_test.go b/provider/fast_test.go
@@ -10,7 +10,9 @@ func init() {
 	version.Version = "1.2.4"
 }
 
-func TestProvider(_ *testing.T) {
+func TestProvider(t *testing.T) {
+	t.Parallel()
+
 	Provider()
 }
 
diff --git a/provider/pkg/batch/compute_environment_test.go b/provider/pkg/batch/compute_environment_test.go
@@ -11,6 +11,8 @@ import (
 )
 
 func TestComputeEnvironmentTransformFromState(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 	pm := resource.PropertyMap{
 		"computeResources": resource.NewObjectProperty(resource.PropertyMap{
diff --git a/provider/pkg/minimalschema/minimalschema_test.go b/provider/pkg/minimalschema/minimalschema_test.go
@@ -17,6 +17,8 @@ const (
 )
 
 func TestNoDanglingReferencesInLightSchema(t *testing.T) {
+	t.Parallel()
+
 	// Check that minimal schema has no dangling references.
 	bytes, err := os.ReadFile(minimalSchemaFile)
 	require.NoError(t, err)
diff --git a/provider/resources.go b/provider/resources.go
@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"time"
 	"unicode"
@@ -493,9 +494,13 @@ var namespaceMap = map[string]string{
 	"aws": "Aws",
 }
 
+var _nslock = sync.Mutex{}
+
 // awsMember manufactures a type token for the AWS package and the given module, file name, and type.
 func awsMember(moduleTitle string, fn string, mem string) tokens.ModuleMember {
 	moduleName := strings.ToLower(moduleTitle)
+	_nslock.Lock()
+	defer _nslock.Unlock()
 	namespaceMap[moduleName] = moduleTitle
 	if fn != "" {
 		moduleName += "/" + fn
diff --git a/provider/resources_test.go b/provider/resources_test.go
@@ -4,12 +4,14 @@ import (
 	"context"
 	"testing"
 
-	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
+	shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestParseDuration(t *testing.T) {
+	t.Parallel()
+
 	for _, v := range []string{
 		"1",
 		"60",
@@ -29,6 +31,8 @@ func TestParseDuration(t *testing.T) {
 }
 
 func TestHasNonComputedTagsAndTagsAllOptimized(t *testing.T) {
+	t.Parallel()
+
 	p := Provider()
 	p.P.ResourcesMap().Range(func(key string, value shim.Resource) bool {
 		actual := hasNonComputedTagsAndTagsAllOptimized(key, value)
@@ -50,6 +54,8 @@ func TestHasNonComputedTagsAndTagsAllOptimized(t *testing.T) {
 }
 
 func TestHasOptionalOrRequiredNamePropertyOptimized(t *testing.T) {
+	t.Parallel()
+
 	p := Provider()
 	p.P.ResourcesMap().Range(func(key string, value shim.Resource) bool {
 		actual := hasOptionalOrRequiredNameProperty(p.P, key)
@@ -67,6 +73,8 @@ func TestHasOptionalOrRequiredNamePropertyOptimized(t *testing.T) {
 	}
 }
 func TestExtractTags(t *testing.T) {
+	t.Parallel()
+
 	tests := []struct {
 		name     string
 		vars     resource.PropertyMap
diff --git a/provider/shim_test.go b/provider/shim_test.go
@@ -23,6 +23,8 @@ import (
 
 // This checks that any runtime checks in the underlying provider (with patches) are passed.
 func TestProviderShim(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 	_, err := shim.NewUpstreamProvider(ctx)
 	if err != nil {
diff --git a/provider/tags_test.go b/provider/tags_test.go
@@ -27,6 +27,8 @@ import (
 )
 
 func TestApplyTags(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	type gen = *rapid.Generator[resource.PropertyValue]
@@ -221,6 +223,8 @@ func TestApplyTags(t *testing.T) {
 }
 
 func TestApplyTagsOutputs(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	type gen = *rapid.Generator[resource.PropertyValue]
@@ -402,6 +406,8 @@ func TestApplyTagsOutputs(t *testing.T) {
 }
 
 func TestAddingEmptyTagProducesChangeDiff(t *testing.T) {
+	t.Parallel()
+
 	replayEvent := `
 	[
 	  {
@@ -498,6 +504,8 @@ func TestAddingEmptyTagProducesChangeDiff(t *testing.T) {
 }
 
 func TestTagsAllNoLongerComputed(t *testing.T) {
+	t.Parallel()
+
 	p := Provider().P
 	p.ResourcesMap().Range(func(key string, res tfshim.Resource) bool {
 		tagsAll, ok := res.Schema().GetOk("tags_all")
diff --git a/provider/upstream_test.go b/provider/upstream_test.go
@@ -0,0 +1,39 @@
+// Copyright 2024, Pulumi Corporation.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package provider
+
+import (
+	"os"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUpstreamLint(t *testing.T) {
+	t.Parallel()
+
+	cmd := exec.Command("make", "provider-lint")
+	cmd.Dir = "../upstream"
+
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+
+	err := cmd.Start()
+	require.NoError(t, err)
+
+	err = cmd.Wait()
+	assert.NoError(t, err)
+}

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,8 @@ import (`
`10`	`10`	`)`
`11`	`11`
`12`	`12`	`func TestReplaceWafV2TypesWithRecursive(t *testing.T) {`
	`13`	`+ t.Parallel()`
	`14`	`+`
`13`	`15`	`spec := schema.PackageSpec{`
`14`	`16`	`Types: map[string]schema.ComplexTypeSpec{`
`15`	`17`	`// Root statement - keep it.`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,8 @@ import (`
`27`	`27`	`//`
`28`	`28`	`// See also pulumi/pulumi-aws#3650.`
`29`	`29`	`func TestRegressLandingZoneDiff(t *testing.T) {`
	`30`	`+ t.Parallel()`
	`31`	`+`
`30`	`32`	event := `
`31`	`33`	`[{`
`32`	`34`	`"method": "/pulumirpc.ResourceProvider/Diff",`
`@@ -66,6 +68,8 @@ func TestRegressLandingZoneDiff(t *testing.T) {`
`66`	`68`	`}`
`67`	`69`
`68`	`70`	`func TestRegress1738(t *testing.T) {`
	`71`	`+ t.Parallel()`
	`72`	`+`
`69`	`73`	containerDefinitionsOld := `
`70`	`74`	`[`
`71`	`75`	`{`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,8 @@ import (`
`9`	`9`	`)`
`10`	`10`
`11`	`11`	`func TestInstanceTypeName(t *testing.T) {`
	`12`	`+ t.Parallel()`
	`13`	`+`
`12`	`14`	`type testCase struct {`
`13`	`15`	`Value string`
`14`	`16`	`Name string`
`@@ -34,6 +36,8 @@ func TestInstanceTypeName(t *testing.T) {`
`34`	`36`	`}`
`35`	`37`
`36`	`38`	`func TestInstanceTypeNameErr(t *testing.T) {`
	`39`	`+ t.Parallel()`
	`40`	`+`
`37`	`41`	`testCases := []string{`
`38`	`42`	`"a1.metal.pc",`
`39`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,9 @@ func init() {`
`10`	`10`	`version.Version = "1.2.4"`
`11`	`11`	`}`
`12`	`12`
`13`		`-func TestProvider(_ *testing.T) {`
	`13`	`+func TestProvider(t *testing.T) {`
	`14`	`+ t.Parallel()`
	`15`	`+`
`14`	`16`	`Provider()`
`15`	`17`	`}`
`16`	`18`
Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,8 @@ import (`
`11`	`11`	`)`
`12`	`12`
`13`	`13`	`func TestComputeEnvironmentTransformFromState(t *testing.T) {`
	`14`	`+ t.Parallel()`
	`15`	`+`
`14`	`16`	`ctx := context.Background()`
`15`	`17`	`pm := resource.PropertyMap{`
`16`	`18`	`"computeResources": resource.NewObjectProperty(resource.PropertyMap{`