makefile refactor for help. Cleanup. Seeing a panic

Author: arrestle

.gitignore

@@ -6,6 +6,7 @@ terraform-provider-aap
 *.tfstate
 *.tfstate.*
 *.tfrc
+*.txt
 
 # Crash log files
 crash.log
@@ -38,3 +39,5 @@ config.tfrc
 
 .env
 .DS_Store
+.tools
+.vscode

Makefile

@@ -1,32 +1,29 @@
+include makefiles/golangci.mk
+
 .PHONY: build test lint
 
+
 default: build
 
-build:
+build: ## Compile the Go package. This is the default.
 	@echo "==> Building package..."
 	go build
 
-lint:
-	@echo "==> Checking source code against linters..."
-	golangci-lint run -v ./...
-
-test:
+test: ## Execute all unit tests with verbose output."
 	@echo "==> Running unit tests..."
 	go test -v ./...
 
-testacc:
-	@echo "==> Running acceptance tests..."
-	TF_ACC=1 AAP_HOST="https://localhost:8043" AAP_INSECURE_SKIP_VERIFY=true go test -count=1 -v ./...
-
-testacc-aapdev:
+testacc: ## Run acceptance tests against local aap-dev instance (https://localhost:9080)."
 	@echo "==> Running acceptance tests..."
 	TF_ACC=1 AAP_HOST="http://localhost:9080" go test -count=1 -v ./...
 
-gofmt:
-	@echo "==> Format code using gofmt..."
-	gofmt -s -w internal/provider
-
-generatedocs:
+generatedocs: ## Format example Terraform configurations and generate plugin documentation."
 	@echo "==> Formatting examples and generating docs..."
 	terraform fmt -recursive ./examples/
 	go run github.com/hashicorp/terraform-plugin-docs/cmd/tfplugindocs generate
+
+.PHONY: help
+help: ## Show this help message
+	@grep -hE '^[a-zA-Z0-9._-]+:.*?##' $(MAKEFILE_LIST) | \
+	awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-24s\033[0m %s\n", $$1, $$2}' | \
+	sort

README.md

@@ -42,7 +42,7 @@ Run `make test`
 
 Acceptance tests apply test terraform configurations to a running AAP instance and make changes to resources in that instance, use with caution!
 
-To run acceptance tests locally, start a local AAP instance following the [docker-compose instructions for local AWX development](https://github.com/ansible/awx/blob/devel/tools/docker-compose/README.md). Create an admin user for the AAP instance and save the credentials to these environment variables:
+To run acceptance tests locally, start a local AAP instance following the [The AAP development environment](https://github.com/ansible/aap-dev/blob/main/README.md). Create an admin user for the AAP instance and save the credentials to these environment variables:
 
 ```bash
 export AAP_USERNAME=<your admin username>

internal/provider/client.go

@@ -150,6 +150,9 @@ func (c *AAPClient) Create(path string, data io.Reader) ([]byte, diag.Diagnostic
 
 // Get sends a GET request to the provided path, checks for errors, and returns the response body with any errors as diagnostics.
 func (c *AAPClient) GetWithStatus(path string) ([]byte, diag.Diagnostics, int) {
+	if c == nil {
+		return nil, diag.Diagnostics{diag.NewErrorDiagnostic("AAPClient is nil", "Client was not initialized properly")}, 0
+	}
 	getResponse, body, err := c.doRequest("GET", path, nil)
 	diags := ValidateResponse(getResponse, body, err, []int{http.StatusOK})
 	return body, diags, getResponse.StatusCode

internal/provider/group_resource_test.go

@@ -19,6 +19,11 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 )
 
+var (
+	reGroupURLPattern = regexp.MustCompile(`^/api(/controller)?/v2/groups/\d+/$`)
+	reInvalidVars     = regexp.MustCompile("Input type `str` is not a dictionary")
+)
+
 func TestGroupResourceSchema(t *testing.T) {
 	t.Parallel()
 
@@ -35,14 +40,13 @@ func TestGroupResourceSchema(t *testing.T) {
 
 	// Validate the schema
 	diagnostics := schemaResponse.Schema.ValidateImplementation(ctx)
-
 	if diagnostics.HasError() {
 		t.Fatalf("Schema validation diagnostics: %+v", diagnostics)
 	}
 }
 
 func TestGroupResourceCreateRequestBody(t *testing.T) {
-	var testTable = []struct {
+	testCases := []struct {
 		name     string
 		input    GroupResourceModel
 		expected []byte
@@ -97,14 +101,14 @@ func TestGroupResourceCreateRequestBody(t *testing.T) {
 		},
 	}
 
-	for _, test := range testTable {
-		t.Run(test.name, func(t *testing.T) {
-			actual, diags := test.input.CreateRequestBody()
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			actual, diags := testCase.input.CreateRequestBody()
 			if diags.HasError() {
 				t.Fatal(diags.Errors())
 			}
-			if !bytes.Equal(test.expected, actual) {
-				t.Errorf("Expected (%s) not equal to actual (%s)", test.expected, actual)
+			if !bytes.Equal(testCase.expected, actual) {
+				t.Errorf("Expected (%s) not equal to actual (%s)", testCase.expected, actual)
 			}
 		})
 	}
@@ -114,7 +118,15 @@ func TestGroupResourceParseHttpResponse(t *testing.T) {
 	jsonError := diag.Diagnostics{}
 	jsonError.AddError("Error parsing JSON response from AAP", "invalid character 'N' looking for beginning of value")
 
-	var testTable = []struct {
+	const groupJSON = `{
+		"inventory": 1,
+		"description": "A basic test group",
+		"name": "group1",
+		"url": "/api/v2/groups/1/",
+		"variables": "{\"foo\":\"bar\",\"nested\":{\"foobar\":\"baz\"}}"
+	}`
+
+	testCases := []struct {
 		name     string
 		input    []byte
 		expected GroupResourceModel
@@ -139,9 +151,8 @@ func TestGroupResourceParseHttpResponse(t *testing.T) {
 			errors: diag.Diagnostics{},
 		},
 		{
-			name: "test with all values",
-			input: []byte(`{"inventory":1,"description":"A basic test group","name":"group1","url":"/api/v2/groups/1/",` +
-				`"variables":"{\"foo\":\"bar\",\"nested\":{\"foobar\":\"baz\"}}"}`),
+			name:  "test with all values",
+			input: []byte(groupJSON),
 			expected: GroupResourceModel{
 				InventoryId: types.Int64Value(1),
 				Id:          types.Int64Value(0),
@@ -154,15 +165,15 @@ func TestGroupResourceParseHttpResponse(t *testing.T) {
 		},
 	}
 
-	for _, test := range testTable {
-		t.Run(test.name, func(t *testing.T) {
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
 			resource := GroupResourceModel{}
-			diags := resource.ParseHttpResponse(test.input)
-			if !test.errors.Equal(diags) {
-				t.Errorf("Expected error diagnostics (%s), actual was (%s)", test.errors, diags)
+			diags := resource.ParseHttpResponse(testCase.input)
+			if !testCase.errors.Equal(diags) {
+				t.Errorf("Expected error diagnostics (%s), actual was (%s)", testCase.errors, diags)
 			}
-			if !reflect.DeepEqual(test.expected, resource) {
-				t.Errorf("Expected (%s) not equal to actual (%s)", test.expected, resource)
+			if !reflect.DeepEqual(testCase.expected, resource) {
+				t.Errorf("Expected (%s) not equal to actual (%s)", testCase.expected, resource)
 			}
 		})
 	}
@@ -172,30 +183,29 @@ func TestGroupResourceParseHttpResponse(t *testing.T) {
 
 func TestAccGroupResource(t *testing.T) {
 	var groupApiModel GroupAPIModel
-	var description = "A test group"
-	var variables = "{\"foo\": \"bar\"}"
 	inventoryName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	groupName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	updatedName := "updated" + groupName
+	description := "A test group"
+	variables := "{\"foo\": \"bar\"}"
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:                 func() { testAccPreCheck(t) },
 		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
-			// Invalid variables testing
+			// Invalid variables testing.
 			{
 				Config:      testAccGroupResourceBadVariables(inventoryName, updatedName),
-				ExpectError: regexp.MustCompile("Input type `str` is not a dictionary"),
+				ExpectError: reInvalidVars,
 			},
-			// Create and Read testing
 			{
 				Config: testAccGroupResourceMinimal(inventoryName, groupName),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccCheckGroupResourceExists("aap_group.test", &groupApiModel),
 					testAccCheckGroupResourceValues(&groupApiModel, groupName, "", ""),
 					resource.TestCheckResourceAttr("aap_group.test", "name", groupName),
 					resource.TestCheckResourceAttrPair("aap_group.test", "inventory_id", "aap_inventory.test", "id"),
-					resource.TestMatchResourceAttr("aap_group.test", "url", regexp.MustCompile("^/api(/controller)?/v2/groups/[0-9]*/$")),
+					resource.TestMatchResourceAttr("aap_group.test", "url", reGroupURLPattern),
 				),
 			},
 			{
@@ -207,7 +217,7 @@ func TestAccGroupResource(t *testing.T) {
 					resource.TestCheckResourceAttrPair("aap_group.test", "inventory_id", "aap_inventory.test", "id"),
 					resource.TestCheckResourceAttr("aap_group.test", "description", description),
 					resource.TestCheckResourceAttr("aap_group.test", "variables", variables),
-					resource.TestMatchResourceAttr("aap_group.test", "url", regexp.MustCompile("^/api(/controller)?/v2/groups/[0-9]*/$")),
+					resource.TestMatchResourceAttr("aap_group.test", "url", reGroupURLPattern),
 				),
 			},
 		},

makefiles/golangci.mk

@@ -0,0 +1,28 @@
+
+ifndef GOLANGCI_LINT_MK_INCLUDED
+
+TOOLS_DIR := $(CURDIR)/.tools
+GOLANGCI_LINT := $(TOOLS_DIR)/golangci-lint
+GOLANGCI_LINT_VERSION ?= v1.60.1
+
+export GOLANGCI_LINT
+
+$(GOLANGCI_LINT):
+	@echo "==> Installing golangci-lint into $(TOOLS_DIR)..."
+	@mkdir -p $(TOOLS_DIR)
+	@curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(TOOLS_DIR) $(GOLANGCI_LINT_VERSION)
+
+.PHONY: lint-tools
+lint-tools: $(GOLANGCI_LINT)
+
+.PHONY: lint
+lint: lint-tools ## Run static analysis via golangci-lint
+	@echo "==> Checking source code against linters..."
+	$(GOLANGCI_LINT) run -v ./...
+
+gofmt: ## Format Go source code in 'internal/provider' using gofmt."
+	@echo "==> Format code using gofmt..."
+	gofmt -s -w internal/provider
+
+GOLANGCI_LINT_MK_INCLUDED := 1
+endif