From a0e02f43e1d02a6bac244cdae2839054be40d446 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Wed, 31 May 2017 09:22:00 -0700 Subject: [PATCH] Fix and cleanup update message and improve related tests (#4361) Fixes #4232 --- cmd/server-main.go | 4 +- cmd/update-main.go | 13 +++--- cmd/update-notifier.go | 55 +++++++++++++++---------- cmd/update-notifier_test.go | 82 ++++++++++++++++++++++++++++--------- 4 files changed, 106 insertions(+), 48 deletions(-) diff --git a/cmd/server-main.go b/cmd/server-main.go index ac3efa6a7..0d44cedfa 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -81,8 +81,8 @@ EXAMPLES: func checkUpdate(mode string) { // Its OK to ignore any errors during getUpdateInfo() here. if older, downloadURL, err := getUpdateInfo(1*time.Second, mode); err == nil { - if older > time.Duration(0) { - log.Println(colorizeUpdateMessage(downloadURL, older)) + if updateMsg := computeUpdateMessage(downloadURL, older); updateMsg != "" { + log.Println(updateMsg) } } } diff --git a/cmd/update-main.go b/cmd/update-main.go index b67db05e0..a8d902091 100644 --- a/cmd/update-main.go +++ b/cmd/update-main.go @@ -238,13 +238,16 @@ func getDownloadURL() (downloadURL string) { return minioReleaseURL + "minio" } -func getUpdateInfo(timeout time.Duration, mode string) (older time.Duration, downloadURL string, err error) { - currentReleaseTime, err := GetCurrentReleaseTime() +func getUpdateInfo(timeout time.Duration, mode string) (older time.Duration, + downloadURL string, err error) { + + var currentReleaseTime, latestReleaseTime time.Time + currentReleaseTime, err = GetCurrentReleaseTime() if err != nil { return older, downloadURL, err } - latestReleaseTime, err := getLatestReleaseTime(timeout, mode) + latestReleaseTime, err = getLatestReleaseTime(timeout, mode) if err != nil { return older, downloadURL, err } @@ -274,8 +277,8 @@ func mainUpdate(ctx *cli.Context) { os.Exit(-1) } - if older != time.Duration(0) { - log.Println(colorizeUpdateMessage(downloadURL, older)) + if updateMsg := computeUpdateMessage(downloadURL, older); updateMsg != "" { + log.Println(updateMsg) os.Exit(1) } diff --git a/cmd/update-notifier.go b/cmd/update-notifier.go index 3584f9ed9..16399492d 100644 --- a/cmd/update-notifier.go +++ b/cmd/update-notifier.go @@ -28,25 +28,39 @@ import ( "github.com/fatih/color" ) +// computeUpdateMessage - calculates the update message, only if a +// newer version is available. +func computeUpdateMessage(downloadURL string, older time.Duration) string { + if downloadURL == "" || older <= 0 { + return "" + } + + // Compute friendly duration string to indicate time + // difference between newer and current release. + t := time.Time{} + newerThan := humanize.RelTime(t, t.Add(older), "ago", "") + + // Return the nicely colored and formatted update message. + return colorizeUpdateMessage(downloadURL, newerThan) +} + // colorizeUpdateMessage - inspired from Yeoman project npm package https://github.com/yeoman/update-notifier -func colorizeUpdateMessage(updateString string, newerThan time.Duration) string { +func colorizeUpdateMessage(updateString string, newerThan string) string { // Initialize coloring. cyan := color.New(color.FgCyan, color.Bold).SprintFunc() yellow := color.New(color.FgYellow, color.Bold).SprintfFunc() - // Calculate length without color coding, due to ANSI color - // characters padded to actual string the final length is wrong - // than the original string length. - newerThanStr := humanize.Time(UTCNow().Add(newerThan)) + msgLine1Fmt := " You are running an older version of Minio released %s " + msgLine2Fmt := " Update: %s " - line1Str := fmt.Sprintf(" You are running an older version of Minio released %s ", newerThanStr) - line2Str := fmt.Sprintf(" Update: %s ", updateString) - line1Length := len(line1Str) - line2Length := len(line2Str) + // Calculate length *without* color coding: with ANSI terminal + // color characters, the result is incorrect. + line1Length := len(fmt.Sprintf(msgLine1Fmt, newerThan)) + line2Length := len(fmt.Sprintf(msgLine2Fmt, updateString)) // Populate lines with color coding. - line1InColor := fmt.Sprintf(" You are running an older version of Minio released %s ", yellow(newerThanStr)) - line2InColor := fmt.Sprintf(" Update: %s ", cyan(updateString)) + line1InColor := fmt.Sprintf(msgLine1Fmt, yellow(newerThan)) + line2InColor := fmt.Sprintf(msgLine2Fmt, cyan(updateString)) // calculate the rectangular box size. maxContentWidth := int(math.Max(float64(line1Length), float64(line2Length))) @@ -60,7 +74,7 @@ func colorizeUpdateMessage(updateString string, newerThan time.Duration) string // Box cannot be printed if terminal width is small than maxContentWidth if maxContentWidth > termWidth { - return "\n" + line1InColor + "\n" + line2InColor + "\n" + "\n" + return "\n" + line1InColor + "\n" + line2InColor + "\n\n" } topLeftChar := "┏" @@ -79,14 +93,11 @@ func colorizeUpdateMessage(updateString string, newerThan time.Duration) string vertBarChar = "|" } - message := "\n" - // Add top line - message += yellow(topLeftChar+strings.Repeat(horizBarChar, maxContentWidth)+topRightChar) + "\n" - // Add message lines - message += vertBarChar + line1InColor + strings.Repeat(" ", maxContentWidth-line1Length) + vertBarChar + "\n" - message += vertBarChar + line2InColor + strings.Repeat(" ", maxContentWidth-line2Length) + vertBarChar + "\n" - // Add bottom line - message += yellow(bottomLeftChar+strings.Repeat(horizBarChar, maxContentWidth)+bottomRightChar) + "\n" - - return message + lines := []string{ + yellow(topLeftChar + strings.Repeat(horizBarChar, maxContentWidth) + topRightChar), + vertBarChar + line1InColor + strings.Repeat(" ", maxContentWidth-line1Length) + vertBarChar, + vertBarChar + line2InColor + strings.Repeat(" ", maxContentWidth-line2Length) + vertBarChar, + yellow(bottomLeftChar + strings.Repeat(horizBarChar, maxContentWidth) + bottomRightChar), + } + return "\n" + strings.Join(lines, "\n") + "\n" } diff --git a/cmd/update-notifier_test.go b/cmd/update-notifier_test.go index 6ea79288f..f6506b8df 100644 --- a/cmd/update-notifier_test.go +++ b/cmd/update-notifier_test.go @@ -17,7 +17,7 @@ package cmd import ( - "runtime" + "fmt" "strings" "testing" "time" @@ -26,25 +26,69 @@ import ( ) // Tests update notifier string builder. -func TestUpdateNotifier(t *testing.T) { - plainMsg := "You are running an older version of Minio released " - colorMsg := plainMsg +func TestComputeUpdateMessage(t *testing.T) { + testCases := []struct { + older time.Duration + dlURL string + + expectedSubStr string + }{ + // Testcase index 0 + {72 * time.Hour, "my_download_url", "3 days ago"}, + {3 * time.Hour, "https://my_download_url_is_huge/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "3 hours ago"}, + {-72 * time.Hour, "another_update_url", ""}, + {0, "another_update_url", ""}, + {time.Hour, "", ""}, + {1 * time.Second, "my_download_url", "now"}, + {2 * time.Second, "my_download_url", "1 second ago"}, + {37 * time.Second, "my_download_url", "37 seconds ago"}, + {60 * time.Second, "my_download_url", "60 seconds ago"}, + {61 * time.Second, "my_download_url", "1 minute ago"}, + + // Testcase index 10 + {37 * time.Minute, "my_download_url", "37 minutes ago"}, + {1 * time.Hour, "my_download_url", "60 minutes ago"}, + {61 * time.Minute, "my_download_url", "1 hour ago"}, + {122 * time.Minute, "my_download_url", "2 hours ago"}, + {24 * time.Hour, "my_download_url", "24 hours ago"}, + {25 * time.Hour, "my_download_url", "1 day ago"}, + {49 * time.Hour, "my_download_url", "2 days ago"}, + {7 * 24 * time.Hour, "my_download_url", "7 days ago"}, + {8 * 24 * time.Hour, "my_download_url", "1 week ago"}, + {15 * 24 * time.Hour, "my_download_url", "2 weeks ago"}, + + // Testcase index 20 + {30 * 24 * time.Hour, "my_download_url", "4 weeks ago"}, + {31 * 24 * time.Hour, "my_download_url", "1 month ago"}, + {61 * 24 * time.Hour, "my_download_url", "2 months ago"}, + {360 * 24 * time.Hour, "my_download_url", "12 months ago"}, + {361 * 24 * time.Hour, "my_download_url", "1 year ago"}, + {2 * 365 * 24 * time.Hour, "my_download_url", "2 years ago"}, + } + + plainMsg := "You are running an older version of Minio released" yellow := color.New(color.FgYellow, color.Bold).SprintfFunc() - if runtime.GOOS == "windows" { - plainMsg += "3 days from now" - colorMsg += yellow("3 days from now") - } else { - plainMsg += "2 days from now" - colorMsg += yellow("2 days from now") - } + cyan := color.New(color.FgCyan, color.Bold).SprintFunc() - updateMsg := colorizeUpdateMessage(minioReleaseURL, time.Duration(72*time.Hour)) - - if !(strings.Contains(updateMsg, plainMsg) || strings.Contains(updateMsg, colorMsg)) { - t.Fatal("Duration string not found in colorized update message", updateMsg) - } - - if !strings.Contains(updateMsg, minioReleaseURL) { - t.Fatal("Update message not found in colorized update message", minioReleaseURL) + for i, testCase := range testCases { + output := computeUpdateMessage(testCase.dlURL, testCase.older) + line1 := fmt.Sprintf("%s %s", plainMsg, yellow(testCase.expectedSubStr)) + line2 := fmt.Sprintf("Update: %s", cyan(testCase.dlURL)) + // Uncomment below to see message appearance: + // fmt.Println(output) + switch { + case testCase.dlURL == "" && output != "": + t.Errorf("Testcase %d: no newer release available but got an update message: %s", i, output) + case output == "" && testCase.dlURL != "" && testCase.older > 0: + t.Errorf("Testcase %d: newer release is available but got empty update message!", i) + case output == "" && (testCase.dlURL == "" || testCase.older <= 0): + // Valid no update message case. No further + // validation needed. + continue + case !strings.Contains(output, line1): + t.Errorf("Testcase %d: output '%s' did not contain line 1: '%s'", i, output, line1) + case !strings.Contains(output, line2): + t.Errorf("Testcase %d: output '%s' did not contain line 2: '%s'", i, output, line2) + } } }