fix: randomize test data to avoid conflict, fix naming and styling is…

…sues

modules/pagerduty-service-integration-email/README.md

@@ -1,6 +1,6 @@
-## PagerDuty service integration - Email
+## PagerDuty Email Service Integration
 
-A `pagerduty-service-integration-email` is a [`pagerduty_service_integration`](https://registry.terraform.io/providers/PagerDuty/pagerduty/latest/docs/resources/service_integration) of the type `generic_email_inbound_integration` that belongs to a service.
+A `pagerduty-service-integration-email` is a [`pagerduty_service_integration`](https://registry.terraform.io/providers/PagerDuty/pagerduty/latest/docs/resources/service_integration) of the type `generic_email_inbound_integration`. Use this module to trigger incidents via email messages.
 
 <!-- BEGIN_TF_DOCS -->
 ## Requirements

modules/pagerduty-service-integration-email/generic-email-inbound-integration.tf

@@ -2,14 +2,19 @@ resource "pagerduty_service_integration" "generic_email_inbound_integration" {
   name              = var.name
   service           = var.service_id
   integration_email = var.integration_email
-  # hardcoded now since we don't support multiple email filters.
+  # In the initial implementation of this module, we hardcode this value because
+  # we expose configuration for only 1 email-filter object,
+  # Further Enhancement: Expose this as a variable when we start exposing email filters
+  # as a collection of "email_filter" objects.
   email_filter_mode = "and-rules-email"
-  # hardcoded because this is a generic_email_inbound_integration module.
+  # This module will only encapsulate integrations of the type `generic_email_inbound_integration`
   type                    = "generic_email_inbound_integration"
   email_incident_creation = var.email_incident_creation
 
   email_filter {
-    # v1 we don't expose this yet.
+    # We chose not to expose the body filter functionality in the initial implementation of this module, and instead hardcode
+    # the best fit default here.
+    # Further enhancement: Expose this attribute as a variable.
     body_mode = "always"
 
     from_email_mode  = var.email_filter.from_email_regex == null ? "always" : "match"

test/pagerduty_business_service_test.go

@@ -52,7 +52,7 @@ func createPagerdutyBusinessService(t *testing.T, workingDir string, businessSer
 
 func destroyPagerdutyBusinessService(t *testing.T, workingDir string) {
 	_, err := terraform.DestroyE(t, test_structure.LoadTerraformOptions(t, workingDir))
-	// have to re-do destroy sometimes coz of race conditions (i.e. try to delete team while it still has associations)
+	// have to re-do destroy sometimes because of race conditions (i.e. try to delete team while it still has associations)
 	// In the retry the team will get deleted properly because the associations have been deleted in previous run
 	if err != nil {
 		terraform.Destroy(t, test_structure.LoadTerraformOptions(t, workingDir))
@@ -65,6 +65,6 @@ func verifyPagerdutyBusinessService(t *testing.T, serviceID string, expectedBuss
 	if serviceErr != nil {
 		log.Println("error getting service: ", serviceErr)
 	}
-	fmt.Printf("checking strings %s with %s", expectedBussinessServiceName, businessService.Name)
+	fmt.Printf("comparing strings %s with %s", expectedBussinessServiceName, businessService.Name)
 	assert.True(t, strings.HasPrefix(businessService.Name, expectedBussinessServiceName))
 }

test/pagerduty_escalation_policy_test.go

@@ -54,7 +54,7 @@ func createEscalationPolicy(t *testing.T, workingDir string, runID string) strin
 
 func destroyEscalationPolicy(t *testing.T, workingDir string) {
 	_, err := terraform.DestroyE(t, test_structure.LoadTerraformOptions(t, workingDir))
-	// have to re-do destroy sometimes coz of race conditions (i.e. try to delete team while it still has associations)
+	// have to re-do destroy sometimes because of race conditions (i.e. try to delete team while it still has associations)
 	// In the retry the team will get deleted properly because the associations have been deleted in previous run
 	if err != nil {
 		terraform.Destroy(t, test_structure.LoadTerraformOptions(t, workingDir))

test/pagerduty_service_email_integration_test.go

@@ -86,7 +86,7 @@ type EmailFilter struct {
 
 func destroyPagerdutyServiceEmailIntegration(t *testing.T, workingDir string) {
 	_, err := terraform.DestroyE(t, test_structure.LoadTerraformOptions(t, workingDir))
-	// have to re-do destroy sometimes coz of race conditions (i.e. try to delete team while it still has associations)
+	// have to re-do destroy sometimes because of race conditions (i.e. try to delete team while it still has associations)
 	// In the retry the team will get deleted properly because the associations have been deleted in previous run
 	if err != nil {
 		terraform.Destroy(t, test_structure.LoadTerraformOptions(t, workingDir))

test/pagerduty_service_test.go

@@ -46,7 +46,7 @@ func createPagerdutyService(t *testing.T, workingDir string, runID string) (stri
 
 func destroyPagerdutyService(t *testing.T, workingDir string) {
 	_, err := terraform.DestroyE(t, test_structure.LoadTerraformOptions(t, workingDir))
-	// have to re-do destroy sometimes coz of race conditions (i.e. try to delete team while it still has associations)
+	// have to re-do destroy sometimes because of race conditions (i.e. try to delete team while it still has associations)
 	// In the retry the team will get deleted properly because the associations have been deleted in previous run
 	if err != nil {
 		terraform.Destroy(t, test_structure.LoadTerraformOptions(t, workingDir))

test/pagerduty_user_test.go

@@ -19,23 +19,24 @@ func TestPagerdutyUser(t *testing.T) {
 	// Working dirs
 	userWorkingDir := ""
 	generatedUserNameSuffix := ""
+	userEmail := ""
 
 	// For assignment later
 	createdUserId := ""
 	defer test_structure.RunTestStage(t, "destroy_user", func() {
-		destroyUser(t, pagerdutyApiToken, userWorkingDir)
+		destroyUser(t, userWorkingDir)
 	})
 
 	test_structure.RunTestStage(t, "create_user", func() {
 
 		// NOTE: We use `read_only_user` (Stakeholder) because those licenses are available.
 		// We don't always have standard `user` licenses available so the test will fail
 		// when trying to create a user with no licenses available.
-		userWorkingDir, createdUserId, generatedUserNameSuffix = createUser(t, "read_only_user", pagerdutyApiToken)
+		userWorkingDir, createdUserId, generatedUserNameSuffix, userEmail = createUser(t, "read_only_user", pagerdutyApiToken)
 	})
 
 	test_structure.RunTestStage(t, "verify_user", func() {
-		verifyUser(t, createdUserId, "read_only_user", pagerdutyApiBaseUrl, pagerdutyApiToken, fmt.Sprintf("example-%s", generatedUserNameSuffix))
+		verifyUser(t, createdUserId, "read_only_user", pagerdutyApiBaseUrl, pagerdutyApiToken, fmt.Sprintf("example-%s", generatedUserNameSuffix), userEmail)
 	})
 }
 
@@ -65,8 +66,10 @@ func TestPagerdutyUser(t *testing.T) {
 //	})
 //}
 
-func createUser(t *testing.T, role string, pagerdutyApiToken string) (string, string, string) {
+func createUser(t *testing.T, role string, pagerdutyApiToken string) (string, string, string, string) {
 	//workingDir := test_structure.CopyTerraformFolderToTemp(t, "..", "modules/pagerduty-user")
+	runID := generateRunId()
+	userEmail := fmt.Sprintf("pagerduty-user-example%s@honestbank.com", runID)
 	workingDir := "../examples/pagerduty-user"
 	log.Println("createUser - workingDir is: ", workingDir)
 
@@ -78,6 +81,7 @@ func createUser(t *testing.T, role string, pagerdutyApiToken string) (string, st
 		TerraformDir: "../examples/pagerduty-user",
 
 		Vars: map[string]interface{}{
+			"email_address":   userEmail,
 			"pagerduty_token": pagerdutyApiToken,
 			"role":            role,
 		},
@@ -89,15 +93,15 @@ func createUser(t *testing.T, role string, pagerdutyApiToken string) (string, st
 	log.Println("🔢 returning PagerDuty User ID: ", pagerdutyUserId)
 	generatedUserNameSuffix := terraform.Output(t, createUserTerraformOptions, "generated_user_name_suffix")
 
-	return workingDir, pagerdutyUserId, generatedUserNameSuffix
+	return workingDir, pagerdutyUserId, generatedUserNameSuffix, userEmail
 }
 
-func destroyUser(t *testing.T, pagerdutyApiToken string, workingDir string) {
+func destroyUser(t *testing.T, workingDir string) {
 	terraformOptions := test_structure.LoadTerraformOptions(t, workingDir)
 	terraform.Destroy(t, terraformOptions)
 }
 
-func verifyUser(t *testing.T, pagerdutyUserId string, role string, pagerdutyApiBaseUrl string, pagerdutyApiToken string, expectedUserName string) {
+func verifyUser(t *testing.T, pagerdutyUserId string, role string, pagerdutyApiBaseUrl string, pagerdutyApiToken string, expectedUserName string, expectedUserEmail string) {
 	pagerdutyApiOptions := http_helper.HttpDoOptions{
 		Method: "GET",
 		Url:    pagerdutyApiBaseUrl + "/users/" + pagerdutyUserId,
@@ -115,7 +119,7 @@ func verifyUser(t *testing.T, pagerdutyUserId string, role string, pagerdutyApiB
 	assert.Equalf(t, 200, status, "incorrect response code, expected 200")
 	fmt.Printf("json = %s", response)
 	assert.Containsf(t, response, fmt.Sprintf("\"name\":\"%s\"", expectedUserName), "correct name not found")
-	assert.Containsf(t, response, "\"email\":\"pagerduty-user-example@honestbank.com\"", "correct email not found")
+	assert.Containsf(t, response, fmt.Sprintf("\"email\":\"%s\"", expectedUserEmail), "correct email not found")
 
 	client := pagerduty.NewClient(loadPagerdutyToken(t))
 	getUserOptions := pagerduty.GetUserOptions{}