From 338402ccb0d2254486a535a62798f36f896b4936 Mon Sep 17 00:00:00 2001 From: George Gelashvili Date: Wed, 7 Aug 2024 18:16:09 +0000 Subject: [PATCH 1/5] Deduplicate tests from previous merge Co-authored-by: Greg Weresch --- .../v7/shared/app_summary_displayer_test.go | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/command/v7/shared/app_summary_displayer_test.go b/command/v7/shared/app_summary_displayer_test.go index 79f58fabaf7..14a3fd47557 100644 --- a/command/v7/shared/app_summary_displayer_test.go +++ b/command/v7/shared/app_summary_displayer_test.go @@ -778,45 +778,7 @@ var _ = Describe("app summary displayer", func() { Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) - }) - }) - When("the deployment strategy is canary", func() { - When("the deployment is paused", func() { - BeforeEach(func() { - summary = v7action.DetailedApplicationSummary{ - ApplicationSummary: v7action.ApplicationSummary{ - Application: resources.Application{ - Name: "some-app", - }, - }, - Deployment: resources.Deployment{ - Strategy: constant.DeploymentStrategyCanary, - StatusValue: constant.DeploymentStatusValueActive, - StatusReason: constant.DeploymentStatusReasonPaused, - }, - } - }) - It("displays the message", func() { - Expect(testUI.Out).To(Say("Canary deployment currently PAUSED.")) - Expect(testUI.Out).To(Say("Please run `cf continue-deployment some-app` to promote the canary deployment, or `cf cancel-deployment some-app` to rollback to the previous version.")) - }) - }) - - When("the deployment is cancelled", func() { - BeforeEach(func() { - summary = v7action.DetailedApplicationSummary{ - Deployment: resources.Deployment{ - Strategy: constant.DeploymentStrategyCanary, - StatusValue: constant.DeploymentStatusValueActive, - StatusReason: constant.DeploymentStatusReasonCanceling, - }, - } - }) - - It("displays the message", func() { - Expect(testUI.Out).To(Say("Canary deployment currently CANCELING.")) - }) }) }) }) From be346b1c5f08cc6c1de9614d286f5baa8644117f Mon Sep 17 00:00:00 2001 From: George Gelashvili Date: Wed, 7 Aug 2024 19:30:37 +0000 Subject: [PATCH 2/5] Print max-in-flight value as part of deployment information in process table when it is set to non-default value (currently 1) Co-authored-by: Greg Weresch --- .../ccv3/constant/deployment.go | 2 + command/v7/shared/app_summary_displayer.go | 20 +- .../v7/shared/app_summary_displayer_test.go | 330 +++++++++++++++--- 3 files changed, 282 insertions(+), 70 deletions(-) diff --git a/api/cloudcontroller/ccv3/constant/deployment.go b/api/cloudcontroller/ccv3/constant/deployment.go index 2ae6159ebb7..b35baa374c8 100644 --- a/api/cloudcontroller/ccv3/constant/deployment.go +++ b/api/cloudcontroller/ccv3/constant/deployment.go @@ -31,3 +31,5 @@ const ( DeploymentStatusValueActive DeploymentStatusValue = "ACTIVE" DeploymentStatusValueFinalized DeploymentStatusValue = "FINALIZED" ) + +const DeploymentMaxInFlightDefaultValue int = 1 diff --git a/command/v7/shared/app_summary_displayer.go b/command/v7/shared/app_summary_displayer.go index 3d05941343f..1eab2046b33 100644 --- a/command/v7/shared/app_summary_displayer.go +++ b/command/v7/shared/app_summary_displayer.go @@ -165,6 +165,12 @@ func (display AppSummaryDisplayer) displayProcessTable(summary v7action.Detailed if summary.Deployment.StatusValue == constant.DeploymentStatusValueActive { display.UI.DisplayNewline() display.UI.DisplayText(display.getDeploymentStatusText(summary)) + + var maxInFlight = summary.Deployment.Options.MaxInFlight + if maxInFlight > 0 && maxInFlight != constant.DeploymentMaxInFlightDefaultValue { + display.UI.DisplayText(fmt.Sprintf("max-in-flight: %d", maxInFlight)) + } + if summary.Deployment.Strategy == constant.DeploymentStrategyCanary && summary.Deployment.StatusReason == constant.DeploymentStatusReasonPaused { display.UI.DisplayNewline() display.UI.DisplayText(fmt.Sprintf("Please run `cf continue-deployment %s` to promote the canary deployment, or `cf cancel-deployment %s` to rollback to the previous version.", summary.Application.Name, summary.Application.Name)) @@ -174,25 +180,15 @@ func (display AppSummaryDisplayer) displayProcessTable(summary v7action.Detailed func (display AppSummaryDisplayer) getDeploymentStatusText(summary v7action.DetailedApplicationSummary) string { var lastStatusChangeTime = display.getLastStatusChangeTime(summary) - if lastStatusChangeTime != "" { return fmt.Sprintf("%s deployment currently %s (since %s)", cases.Title(language.English, cases.NoLower).String(string(summary.Deployment.Strategy)), summary.Deployment.StatusReason, lastStatusChangeTime) } else { - var sb strings.Builder - sb.WriteString(fmt.Sprintf("%s deployment currently %s.", + return fmt.Sprintf("%s deployment currently %s.", cases.Title(language.English, cases.NoLower).String(string(summary.Deployment.Strategy)), - summary.Deployment.StatusReason)) - - if summary.Deployment.Strategy == constant.DeploymentStrategyCanary && summary.Deployment.StatusReason == constant.DeploymentStatusReasonPaused { - sb.WriteString("\n") - sb.WriteString(fmt.Sprintf( - "Please run `cf continue-deployment %s` to promote the canary deployment, or `cf cancel-deployment %s` to rollback to the previous version.", - summary.Application.Name, summary.Application.Name)) - } - return sb.String() + summary.Deployment.StatusReason) } } diff --git a/command/v7/shared/app_summary_displayer_test.go b/command/v7/shared/app_summary_displayer_test.go index 14a3fd47557..756a6f12251 100644 --- a/command/v7/shared/app_summary_displayer_test.go +++ b/command/v7/shared/app_summary_displayer_test.go @@ -659,10 +659,58 @@ var _ = Describe("app summary displayer", func() { When("there is an active deployment", func() { var LastStatusChangeTimeString = "2024-07-29T17:32:29Z" var dateTimeRegexPattern = `[a-zA-Z]{3}\s\d{2}\s[a-zA-Z]{3}\s\d{2}\:\d{2}\:\d{2}\s[A-Z]{3}\s\d{4}` + var maxInFlightDefaultValue = 1 When("the deployment strategy is rolling", func() { When("the deployment is in progress", func() { - When("last status change has a timestamp", func() { + When("last status change has a timestamp and max-in-flight is non-default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyRolling, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonDeploying, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: 2, + }, + }, + } + }) + + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + }) + It("displays max-in-flight value", func() { + Expect(testUI.Out).To(Say(`max-in-flight: 2`)) + }) + }) + When("last status change has a timestamp and max-in-flight is default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyRolling, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonDeploying, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: maxInFlightDefaultValue, + }, + }, + } + }) + + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) + }) + // 'unset' is important for the newer-CLI-than-CAPI scenario + When("last status change has a timestamp and max-in-flight is unset", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ Deployment: resources.Deployment{ @@ -678,9 +726,35 @@ var _ = Describe("app summary displayer", func() { var actualOut = fmt.Sprintf("%s", testUI.Out) Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) }) - When("last status change is an empty string", func() { + When("last status change is an empty string and max-in-flight is non-default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyRolling, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonDeploying, + LastStatusChange: "", + Options: resources.DeploymentOpts{ + MaxInFlight: 2, + }, + }, + } + }) + + It("displays the message", func() { + Expect(testUI.Out).To(Say(`Rolling deployment currently DEPLOYING`)) + Expect(testUI.Out).NotTo(Say(`\(since`)) + }) + It("displays max-in-flight value", func() { + Expect(testUI.Out).To(Say(`max-in-flight: 2`)) + }) + }) + When("last status change is an empty string and max-in-flight is default", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ Deployment: resources.Deployment{ @@ -688,6 +762,9 @@ var _ = Describe("app summary displayer", func() { StatusValue: constant.DeploymentStatusValueActive, StatusReason: constant.DeploymentStatusReasonDeploying, LastStatusChange: "", + Options: resources.DeploymentOpts{ + MaxInFlight: maxInFlightDefaultValue, + }, }, } }) @@ -696,89 +773,223 @@ var _ = Describe("app summary displayer", func() { Expect(testUI.Out).To(Say(`Rolling deployment currently DEPLOYING`)) Expect(testUI.Out).NotTo(Say(`\(since`)) }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) }) }) When("the deployment is cancelled", func() { - BeforeEach(func() { - summary = v7action.DetailedApplicationSummary{ - Deployment: resources.Deployment{ - Strategy: constant.DeploymentStrategyRolling, - StatusValue: constant.DeploymentStatusValueActive, - StatusReason: constant.DeploymentStatusReasonCanceling, - LastStatusChange: LastStatusChangeTimeString, - }, - } + When("max-in-flight value is non-default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyRolling, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonCanceling, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: 2, + }, + }, + } + }) + + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + }) + It("displays max-in-flight value", func() { + Expect(testUI.Out).To(Say(`max-in-flight: 2`)) + }) }) + When("max-in-flight value is default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyRolling, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonCanceling, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: maxInFlightDefaultValue, + }, + }, + } + }) - It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) }) }) }) When("the deployment strategy is canary", func() { When("the deployment is in progress", func() { - BeforeEach(func() { - summary = v7action.DetailedApplicationSummary{ - Deployment: resources.Deployment{ - Strategy: constant.DeploymentStrategyCanary, - StatusValue: constant.DeploymentStatusValueActive, - StatusReason: constant.DeploymentStatusReasonDeploying, - LastStatusChange: LastStatusChangeTimeString, - }, - } + When("max-in-flight value is non-default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyCanary, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonDeploying, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: 2, + }, + }, + } + }) + + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) + }) + It("displays max-in-flight value", func() { + Expect(testUI.Out).To(Say(`max-in-flight: 2`)) + }) }) + When("max-in-flight value is default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyCanary, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonDeploying, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: maxInFlightDefaultValue, + }, + }, + } + }) - It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) - Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) + }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) }) }) When("the deployment is paused", func() { - BeforeEach(func() { - summary = v7action.DetailedApplicationSummary{ - ApplicationSummary: v7action.ApplicationSummary{ - Application: resources.Application{ - Name: "foobar", + When("max-in-flight value is non-default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + ApplicationSummary: v7action.ApplicationSummary{ + Application: resources.Application{ + Name: "foobar", + }, }, - }, - Deployment: resources.Deployment{ - Strategy: constant.DeploymentStrategyCanary, - StatusValue: constant.DeploymentStatusValueActive, - StatusReason: constant.DeploymentStatusReasonPaused, - LastStatusChange: LastStatusChangeTimeString, - }, - } + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyCanary, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonPaused, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: 2, + }, + }, + } + }) + + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say("Please run `cf continue-deployment foobar` to promote the canary deployment, or `cf cancel-deployment foobar` to rollback to the previous version.")) + }) + It("displays max-in-flight value", func() { + Expect(testUI.Out).To(Say(`max-in-flight: 2`)) + }) }) + When("max-in-flight value is default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + ApplicationSummary: v7action.ApplicationSummary{ + Application: resources.Application{ + Name: "foobar", + }, + }, + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyCanary, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonPaused, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: maxInFlightDefaultValue, + }, + }, + } + }) - It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) - Expect(testUI.Out).To(Say("Please run `cf continue-deployment foobar` to promote the canary deployment, or `cf cancel-deployment foobar` to rollback to the previous version.")) + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say("Please run `cf continue-deployment foobar` to promote the canary deployment, or `cf cancel-deployment foobar` to rollback to the previous version.")) + }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) }) }) When("the deployment is canceling", func() { - BeforeEach(func() { - summary = v7action.DetailedApplicationSummary{ - Deployment: resources.Deployment{ - Strategy: constant.DeploymentStrategyCanary, - StatusValue: constant.DeploymentStatusValueActive, - StatusReason: constant.DeploymentStatusReasonCanceling, - LastStatusChange: LastStatusChangeTimeString, - }, - } - }) + When("max-in-flight value is non-default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyCanary, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonCanceling, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: 2, + }, + }, + } + }) - It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) - Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) + }) + It("displays max-in-flight value", func() { + Expect(testUI.Out).To(Say(`max-in-flight: 2`)) + }) }) + When("max-in-flight value is default", func() { + BeforeEach(func() { + summary = v7action.DetailedApplicationSummary{ + Deployment: resources.Deployment{ + Strategy: constant.DeploymentStrategyCanary, + StatusValue: constant.DeploymentStatusValueActive, + StatusReason: constant.DeploymentStatusReasonCanceling, + LastStatusChange: LastStatusChangeTimeString, + Options: resources.DeploymentOpts{ + MaxInFlight: maxInFlightDefaultValue, + }, + }, + } + }) + It("displays the message", func() { + var actualOut = fmt.Sprintf("%s", testUI.Out) + Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) + }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) + }) }) }) }) @@ -799,6 +1010,9 @@ var _ = Describe("app summary displayer", func() { cases.Title(language.English, cases.NoLower).String(string(summary.Deployment.Strategy)), summary.Deployment.StatusReason))) }) + It("does not display max-in-flight", func() { + Expect(testUI.Out).NotTo(Say(`max-in-flight`)) + }) }) }) }) From 9017f663df305f4b9f1c847c5d214c091d08a851 Mon Sep 17 00:00:00 2001 From: Greg Weresch Date: Mon, 12 Aug 2024 14:23:31 -0400 Subject: [PATCH 3/5] Fix flakey tests that look for output from cf app - these tests depend on a short-lived state --- integration/v7/isolated/app_command_test.go | 22 ++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/integration/v7/isolated/app_command_test.go b/integration/v7/isolated/app_command_test.go index 8f62bc84672..7c46f0317ed 100644 --- a/integration/v7/isolated/app_command_test.go +++ b/integration/v7/isolated/app_command_test.go @@ -267,7 +267,7 @@ applications: session := helpers.CF("restart", appName, "--strategy", "rolling") session1 := helpers.CF("app", appName) - Eventually(session1).Should(Say("Rolling deployment currently DEPLOYING.")) + Eventually(session1).Should(Say("Rolling deployment currently DEPLOYING")) Eventually(session).Should(Exit(0)) Eventually(session1).Should(Exit(0)) }) @@ -279,9 +279,11 @@ applications: return helpers.CF("cancel-deployment", appName).Wait() }).Should(Exit(0)) - session2 := helpers.CF("app", appName) - Eventually(session2).Should(Say("Rolling deployment currently CANCELING.")) - Eventually(session2).Should(Exit(0)) + Eventually(func(g Gomega) { + session := helpers.CF("app", appName).Wait() + g.Expect(session).Should(Say("Rolling deployment currently CANCELING")) + g.Expect(session).Should(Exit(0)) + }).Should(Succeed()) }) }) }) @@ -292,19 +294,21 @@ applications: Eventually(helpers.CF("restart", appName, "--strategy", "canary")).Should(Exit(0)) session1 := helpers.CF("app", appName) - Eventually(session1).Should(Say("Canary deployment currently PAUSED.")) + Eventually(session1).Should(Say("Canary deployment currently PAUSED")) Eventually(session1).Should(Exit(0)) }) }) When("the deployment is cancelled after it is paused", func() { - It("no deployment information is displayed", func() { + It("displays the message", func() { Eventually(helpers.CF("restart", appName, "--strategy", "canary")).Should(Exit(0)) Eventually(helpers.CF("cancel-deployment", appName)).Should(Exit(0)) - session2 := helpers.CF("app", appName) - Eventually(session2).ShouldNot(Say("Canary deployment currently CANCELING.")) - Eventually(session2).Should(Exit(0)) + Eventually(func(g Gomega) { + session := helpers.CF("app", appName).Wait() + g.Expect(session).Should(Say("Canary deployment currently CANCELING")) + g.Expect(session).Should(Exit(0)) + }).Should(Succeed()) }) }) }) From 6f463a29f01c81df9a8aa82ee48fe61ffd639607 Mon Sep 17 00:00:00 2001 From: Greg Weresch Date: Mon, 12 Aug 2024 15:54:15 -0400 Subject: [PATCH 4/5] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Pereira --- .../v7/shared/app_summary_displayer_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/command/v7/shared/app_summary_displayer_test.go b/command/v7/shared/app_summary_displayer_test.go index 756a6f12251..87b4de373aa 100644 --- a/command/v7/shared/app_summary_displayer_test.go +++ b/command/v7/shared/app_summary_displayer_test.go @@ -682,10 +682,12 @@ var _ = Describe("app summary displayer", func() { var actualOut = fmt.Sprintf("%s", testUI.Out) Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) }) + It("displays max-in-flight value", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) }) + When("last status change has a timestamp and max-in-flight is default", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ @@ -705,10 +707,12 @@ var _ = Describe("app summary displayer", func() { var actualOut = fmt.Sprintf("%s", testUI.Out) Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) }) + // 'unset' is important for the newer-CLI-than-CAPI scenario When("last status change has a timestamp and max-in-flight is unset", func() { BeforeEach(func() { @@ -726,6 +730,7 @@ var _ = Describe("app summary displayer", func() { var actualOut = fmt.Sprintf("%s", testUI.Out) Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) @@ -750,10 +755,12 @@ var _ = Describe("app summary displayer", func() { Expect(testUI.Out).To(Say(`Rolling deployment currently DEPLOYING`)) Expect(testUI.Out).NotTo(Say(`\(since`)) }) + It("displays max-in-flight value", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) }) + When("last status change is an empty string and max-in-flight is default", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ @@ -773,6 +780,7 @@ var _ = Describe("app summary displayer", func() { Expect(testUI.Out).To(Say(`Rolling deployment currently DEPLOYING`)) Expect(testUI.Out).NotTo(Say(`\(since`)) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) @@ -799,10 +807,12 @@ var _ = Describe("app summary displayer", func() { var actualOut = fmt.Sprintf("%s", testUI.Out) Expect(actualOut).To(MatchRegexp(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) }) + It("displays max-in-flight value", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) }) + When("max-in-flight value is default", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ @@ -822,6 +832,7 @@ var _ = Describe("app summary displayer", func() { var actualOut = fmt.Sprintf("%s", testUI.Out) Expect(actualOut).To(MatchRegexp(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) @@ -850,10 +861,12 @@ var _ = Describe("app summary displayer", func() { Expect(actualOut).To(MatchRegexp(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) + It("displays max-in-flight value", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) }) + When("max-in-flight value is default", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ @@ -874,6 +887,7 @@ var _ = Describe("app summary displayer", func() { Expect(actualOut).To(MatchRegexp(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) @@ -906,6 +920,7 @@ var _ = Describe("app summary displayer", func() { Expect(actualOut).To(MatchRegexp(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).To(Say("Please run `cf continue-deployment foobar` to promote the canary deployment, or `cf cancel-deployment foobar` to rollback to the previous version.")) }) + It("displays max-in-flight value", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) @@ -935,6 +950,7 @@ var _ = Describe("app summary displayer", func() { Expect(actualOut).To(MatchRegexp(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).To(Say("Please run `cf continue-deployment foobar` to promote the canary deployment, or `cf cancel-deployment foobar` to rollback to the previous version.")) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) @@ -962,6 +978,7 @@ var _ = Describe("app summary displayer", func() { Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) + It("displays max-in-flight value", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) @@ -986,6 +1003,7 @@ var _ = Describe("app summary displayer", func() { Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) @@ -1010,6 +1028,7 @@ var _ = Describe("app summary displayer", func() { cases.Title(language.English, cases.NoLower).String(string(summary.Deployment.Strategy)), summary.Deployment.StatusReason))) }) + It("does not display max-in-flight", func() { Expect(testUI.Out).NotTo(Say(`max-in-flight`)) }) From 483b5eea04db74537a82db69a303380143fb477f Mon Sep 17 00:00:00 2001 From: Greg Weresch Date: Mon, 12 Aug 2024 16:15:31 -0400 Subject: [PATCH 5/5] Refactor app summary displayer tests to use Say instead of MatchRegexp --- .../v7/shared/app_summary_displayer_test.go | 39 +++++++------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/command/v7/shared/app_summary_displayer_test.go b/command/v7/shared/app_summary_displayer_test.go index 87b4de373aa..be1d53d2afb 100644 --- a/command/v7/shared/app_summary_displayer_test.go +++ b/command/v7/shared/app_summary_displayer_test.go @@ -679,8 +679,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) }) It("displays max-in-flight value", func() { @@ -704,8 +703,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) }) It("does not display max-in-flight", func() { @@ -713,8 +711,7 @@ var _ = Describe("app summary displayer", func() { }) }) - // 'unset' is important for the newer-CLI-than-CAPI scenario - When("last status change has a timestamp and max-in-flight is unset", func() { + When("an older version of CAPI does not return max-in-flight", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ Deployment: resources.Deployment{ @@ -727,8 +724,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Rolling deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) }) It("does not display max-in-flight", func() { @@ -804,8 +800,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) }) It("displays max-in-flight value", func() { @@ -829,8 +824,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Rolling deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) }) It("does not display max-in-flight", func() { @@ -839,6 +833,7 @@ var _ = Describe("app summary displayer", func() { }) }) }) + When("the deployment strategy is canary", func() { When("the deployment is in progress", func() { When("max-in-flight value is non-default", func() { @@ -857,8 +852,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) @@ -883,8 +877,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Canary deployment currently DEPLOYING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) @@ -916,8 +909,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).To(Say("Please run `cf continue-deployment foobar` to promote the canary deployment, or `cf cancel-deployment foobar` to rollback to the previous version.")) }) @@ -925,6 +917,7 @@ var _ = Describe("app summary displayer", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) }) + When("max-in-flight value is default", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ @@ -946,8 +939,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Canary deployment currently PAUSED \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).To(Say("Please run `cf continue-deployment foobar` to promote the canary deployment, or `cf cancel-deployment foobar` to rollback to the previous version.")) }) @@ -974,8 +966,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) }) @@ -983,6 +974,7 @@ var _ = Describe("app summary displayer", func() { Expect(testUI.Out).To(Say(`max-in-flight: 2`)) }) }) + When("max-in-flight value is default", func() { BeforeEach(func() { summary = v7action.DetailedApplicationSummary{ @@ -999,8 +991,7 @@ var _ = Describe("app summary displayer", func() { }) It("displays the message", func() { - var actualOut = fmt.Sprintf("%s", testUI.Out) - Expect(actualOut).To(MatchRegexp(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) + Expect(testUI.Out).To(Say(`Canary deployment currently CANCELING \(since %s\)`, dateTimeRegexPattern)) Expect(testUI.Out).NotTo(Say(`promote the canary deployment`)) })