From 065c1325c3c3b53758b4b4a49bfc88574c2d213b Mon Sep 17 00:00:00 2001 From: Marcela Campo Date: Thu, 30 Mar 2023 12:08:30 +0100 Subject: [PATCH] tests(redis): improvements from code review [#184471699](https://www.pivotaltracker.com/story/show/184471699) --- terraform-tests/redis_test.go | 141 ++++++++++++++++------------------ 1 file changed, 66 insertions(+), 75 deletions(-) diff --git a/terraform-tests/redis_test.go b/terraform-tests/redis_test.go index f1d30d973..f911edd41 100644 --- a/terraform-tests/redis_test.go +++ b/terraform-tests/redis_test.go @@ -273,21 +273,40 @@ var _ = Describe("Redis", Label("redis-terraform"), Ordered, func() { }) Context("slow_log is enabled", func() { - BeforeAll(func() { + var kmsKeyId string + var retentionInDays int + + BeforeEach(func() { + kmsKeyId = "" + retentionInDays = 0 + }) + + JustBeforeEach(func() { plan = ShowPlan(terraformProvisionDir, buildVars(defaultVars, map[string]any{ - "logs_slow_log_loggroup_kms_key_id": "", - "logs_slow_log_loggroup_retention_in_days": 0, + "logs_slow_log_loggroup_kms_key_id": kmsKeyId, + "logs_slow_log_loggroup_retention_in_days": retentionInDays, "logs_slow_log_enabled": true, })) }) - It("should create a log group for slow log", func() { - expectedResources := []string{"aws_cloudwatch_log_group"} + It("should configure a log group for slow log with default values", func() { + slowLogResource := "aws_cloudwatch_log_group" + + By("creating a new loggroup resouce") + expectedResources := []string{slowLogResource} expectedResources = append(expectedResources, getExpectedResources()...) Expect(ResourceChangesTypes(plan)).To(ConsistOf(expectedResources)) - }) - It("should assign that log group to the replication group", func() { + By("configuring the log group with the default values") + Expect(AfterValuesForType(plan, slowLogResource)).To(MatchKeys(IgnoreExtras, Keys{ + "name": Equal("/aws/elasticache/cluster/csb-redis-test/slow-log"), + "retention_in_days": BeNumerically("==", 0), + "kms_key_id": BeNil(), + "skip_destroy": BeFalse(), + "tags": HaveKeyWithValue("key1", "some-redis-value"), + })) + + By("assignining that log group to the replication group") Expect(AfterValuesForType(plan, resource)).To(MatchKeys(IgnoreExtras, Keys{ "log_delivery_configuration": ConsistOf(MatchAllKeys(Keys{ "destination": Equal("/aws/elasticache/cluster/csb-redis-test/slow-log"), @@ -296,67 +315,63 @@ var _ = Describe("Redis", Label("redis-terraform"), Ordered, func() { "log_type": Equal("slow-log"), })), })) + }) - Context("slow_log loggroup configuration", func() { - kmsKeyId := "" - retentionInDays := 0 - JustBeforeEach(func() { - plan = ShowPlan(terraformProvisionDir, buildVars(defaultVars, map[string]any{ - "logs_slow_log_loggroup_kms_key_id": kmsKeyId, - "logs_slow_log_loggroup_retention_in_days": retentionInDays, - "logs_slow_log_enabled": true, - })) + Context("slow_log loggroup custom configuration", func() { + BeforeEach(func() { + kmsKeyId = "test-kms-key" + retentionInDays = 180 }) - It("should configure the log group with default values", func() { + It("should configure the passed values", func() { slowLogResource := "aws_cloudwatch_log_group" Expect(AfterValuesForType(plan, slowLogResource)).To(MatchKeys(IgnoreExtras, Keys{ "name": Equal("/aws/elasticache/cluster/csb-redis-test/slow-log"), "retention_in_days": BeNumerically("==", retentionInDays), - "kms_key_id": BeNil(), + "kms_key_id": Equal(kmsKeyId), "skip_destroy": BeFalse(), "tags": HaveKeyWithValue("key1", "some-redis-value"), })) }) - - Context("providing explicit values", func() { - BeforeEach(func() { - kmsKeyId = "test-kms-key" - retentionInDays = 180 - }) - - It("should configure the passed values", func() { - slowLogResource := "aws_cloudwatch_log_group" - Expect(AfterValuesForType(plan, slowLogResource)).To(MatchKeys(IgnoreExtras, Keys{ - "name": Equal("/aws/elasticache/cluster/csb-redis-test/slow-log"), - "retention_in_days": BeNumerically("==", retentionInDays), - "kms_key_id": Equal(kmsKeyId), - "skip_destroy": BeFalse(), - "tags": HaveKeyWithValue("key1", "some-redis-value"), - })) - }) - }) }) - }) Context("engine_log is enabled", func() { - BeforeAll(func() { + var kmsKeyId string + var retentionInDays int + + BeforeEach(func() { + kmsKeyId = "" + retentionInDays = 0 + }) + + JustBeforeEach(func() { plan = ShowPlan(terraformProvisionDir, buildVars(defaultVars, map[string]any{ - "logs_engine_log_loggroup_kms_key_id": "", - "logs_engine_log_loggroup_retention_in_days": 0, + "logs_engine_log_loggroup_kms_key_id": kmsKeyId, + "logs_engine_log_loggroup_retention_in_days": retentionInDays, "logs_engine_log_enabled": true, })) }) - It("should create a log group for engine log", func() { - expectedResources := []string{"aws_cloudwatch_log_group"} + It("should configure a log group for engine log with default values", func() { + engineLogResource := "aws_cloudwatch_log_group" + + By("creating a new loggroup resouce") + expectedResources := []string{engineLogResource} expectedResources = append(expectedResources, getExpectedResources()...) Expect(ResourceChangesTypes(plan)).To(ConsistOf(expectedResources)) - }) - It("should assign that log group to the replication group", func() { + By("configuring the log group with the default values") + Expect(AfterValuesForType(plan, engineLogResource)).To(MatchKeys(IgnoreExtras, Keys{ + "name": Equal("/aws/elasticache/cluster/csb-redis-test/engine-log"), + "retention_in_days": BeNumerically("==", 0), + "kms_key_id": BeNil(), + "skip_destroy": BeFalse(), + "tags": HaveKeyWithValue("key1", "some-redis-value"), + })) + + By("assignining that log group to the replication group") Expect(AfterValuesForType(plan, resource)).To(MatchKeys(IgnoreExtras, Keys{ "log_delivery_configuration": ConsistOf(MatchAllKeys(Keys{ "destination": Equal("/aws/elasticache/cluster/csb-redis-test/engine-log"), @@ -367,47 +382,23 @@ var _ = Describe("Redis", Label("redis-terraform"), Ordered, func() { })) }) - Context("engine_log loggroup configuration", func() { - kmsKeyId := "" - retentionInDays := 0 - JustBeforeEach(func() { - plan = ShowPlan(terraformProvisionDir, buildVars(defaultVars, map[string]any{ - "logs_engine_log_loggroup_kms_key_id": kmsKeyId, - "logs_engine_log_loggroup_retention_in_days": retentionInDays, - "logs_engine_log_enabled": true, - })) + Context("engine loggroup custom coonfiguration", func() { + BeforeEach(func() { + kmsKeyId = "test-kms-key-2" + retentionInDays = 5 }) - It("should configure the log group with default values", func() { + It("should configure the specified values", func() { engineLogResource := "aws_cloudwatch_log_group" Expect(AfterValuesForType(plan, engineLogResource)).To(MatchKeys(IgnoreExtras, Keys{ "name": Equal("/aws/elasticache/cluster/csb-redis-test/engine-log"), - "retention_in_days": BeNumerically("==", retentionInDays), - "kms_key_id": BeNil(), + "retention_in_days": BeNumerically("==", 5), + "kms_key_id": Equal("test-kms-key-2"), "skip_destroy": BeFalse(), "tags": HaveKeyWithValue("key1", "some-redis-value"), })) }) - - Context("providing explicit values", func() { - BeforeEach(func() { - kmsKeyId = "test-kms-key" - retentionInDays = 180 - }) - - It("should configure the passed values", func() { - engineLogResource := "aws_cloudwatch_log_group" - Expect(AfterValuesForType(plan, engineLogResource)).To(MatchKeys(IgnoreExtras, Keys{ - "name": Equal("/aws/elasticache/cluster/csb-redis-test/engine-log"), - "retention_in_days": BeNumerically("==", retentionInDays), - "kms_key_id": Equal(kmsKeyId), - "skip_destroy": BeFalse(), - "tags": HaveKeyWithValue("key1", "some-redis-value"), - })) - }) - }) }) - }) })