From 9e3d8879eca55b2cfae7b731210df0708b8917ef Mon Sep 17 00:00:00 2001 From: Ken Kaizu Date: Thu, 19 Jan 2023 11:14:58 +0900 Subject: [PATCH] fix: PR status summary should remove Note: Objects have changed outside of Terraform (#3010) * extract PlanSuccess.DiffSummary * commit status updater use DiffSummary instead Summary to show only diff result --- server/events/commit_status_updater.go | 2 +- server/events/commit_status_updater_test.go | 2 +- server/events/models/models.go | 11 +++- server/events/models/models_test.go | 64 +++++++++++++++++++++ 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/server/events/commit_status_updater.go b/server/events/commit_status_updater.go index 794fe1d36f..e84023a9e8 100644 --- a/server/events/commit_status_updater.go +++ b/server/events/commit_status_updater.go @@ -95,7 +95,7 @@ func (d *DefaultCommitStatusUpdater) UpdateProject(ctx command.ProjectContext, c descripWords = genProjectStatusDescription(cmdName.String(), "failed.") case models.SuccessCommitStatus: if result != nil && result.PlanSuccess != nil { - descripWords = result.PlanSuccess.Summary() + descripWords = result.PlanSuccess.DiffSummary() } else { descripWords = genProjectStatusDescription(cmdName.String(), "succeeded.") } diff --git a/server/events/commit_status_updater_test.go b/server/events/commit_status_updater_test.go index 9ccc34fb58..1f1e2e0bf8 100644 --- a/server/events/commit_status_updater_test.go +++ b/server/events/commit_status_updater_test.go @@ -206,7 +206,7 @@ func TestDefaultCommitStatusUpdater_UpdateProject(t *testing.T) { cmd: command.Plan, result: &command.ProjectResult{ PlanSuccess: &models.PlanSuccess{ - TerraformOutput: "aaa\nPlan: 1 to add, 2 to change, 3 to destroy.\nbbb", + TerraformOutput: "aaa\nNote: Objects have changed outside of Terraform\nbbb\nPlan: 1 to add, 2 to change, 3 to destroy.\nbbb", }, }, expDescrip: "Plan: 1 to add, 2 to change, 3 to destroy.", diff --git a/server/events/models/models.go b/server/events/models/models.go index 5f92e53d4d..6881ccdf54 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -373,16 +373,21 @@ var ( reNoChanges = regexp.MustCompile(`No changes. (Infrastructure is up-to-date|Your infrastructure matches the configuration).`) ) -// Summary extracts one line summary of plan changes from TerraformOutput. +// Summary extracts summaries of plan changes from TerraformOutput. func (p *PlanSuccess) Summary() string { note := "" if match := reChangesOutside.FindString(p.TerraformOutput); match != "" { note = "\n**" + match + "**\n" } + return note + p.DiffSummary() +} + +// DiffSummary extracts one line summary of plan changes from TerraformOutput. +func (p *PlanSuccess) DiffSummary() string { if match := rePlanChanges.FindString(p.TerraformOutput); match != "" { - return note + match + return match } - return note + reNoChanges.FindString(p.TerraformOutput) + return reNoChanges.FindString(p.TerraformOutput) } // Diff Markdown regexes diff --git a/server/events/models/models_test.go b/server/events/models/models_test.go index d49437ed8c..eb08145fbf 100644 --- a/server/events/models/models_test.go +++ b/server/events/models/models_test.go @@ -355,6 +355,70 @@ func TestAzureDevopsSplitRepoFullName(t *testing.T) { } } +func TestPlanSuccess_Summary(t *testing.T) { + cases := []struct { + input string + exp string + }{ + { + "Note: Objects have changed outside of Terraform\ndummy\nPlan: 0 to add, 1 to change, 2 to destroy.", + "\n**Note: Objects have changed outside of Terraform**\nPlan: 0 to add, 1 to change, 2 to destroy.", + }, + { + "dummy\nPlan: 100 to add, 111 to change, 222 to destroy.", + "Plan: 100 to add, 111 to change, 222 to destroy.", + }, + { + "Note: Objects have changed outside of Terraform\ndummy\nNo changes. Infrastructure is up-to-date.", + "\n**Note: Objects have changed outside of Terraform**\nNo changes. Infrastructure is up-to-date.", + }, + { + "dummy\nNo changes. Your infrastructure matches the configuration.", + "No changes. Your infrastructure matches the configuration.", + }, + } + for i, c := range cases { + t.Run(fmt.Sprintf("summary %d", i), func(t *testing.T) { + pcs := models.PlanSuccess{ + TerraformOutput: c.input, + } + Equals(t, c.exp, pcs.Summary()) + }) + } +} + +func TestPlanSuccess_DiffSummary(t *testing.T) { + cases := []struct { + input string + exp string + }{ + { + "Note: Objects have changed outside of Terraform\ndummy\nPlan: 0 to add, 1 to change, 2 to destroy.", + "Plan: 0 to add, 1 to change, 2 to destroy.", + }, + { + "dummy\nPlan: 100 to add, 111 to change, 222 to destroy.", + "Plan: 100 to add, 111 to change, 222 to destroy.", + }, + { + "Note: Objects have changed outside of Terraform\ndummy\nNo changes. Infrastructure is up-to-date.", + "No changes. Infrastructure is up-to-date.", + }, + { + "dummy\nNo changes. Your infrastructure matches the configuration.", + "No changes. Your infrastructure matches the configuration.", + }, + } + for i, c := range cases { + t.Run(fmt.Sprintf("summary %d", i), func(t *testing.T) { + pcs := models.PlanSuccess{ + TerraformOutput: c.input, + } + Equals(t, c.exp, pcs.DiffSummary()) + }) + } +} + func TestPolicyCheckSuccess_Summary(t *testing.T) { cases := []string{ "20 tests, 19 passed, 2 warnings, 0 failures, 0 exceptions",