PR feedback for version info command update

1. Increase timeout to 5s
2. Defer closing response body
3. Use back tick for string formatting
4. Refactor unit tests to use test tables
5. Indent version command output with space
6. Favor http.NewRequest/Do over Client.Get for consistency

Signed-off-by: Edward Wilde <ewilde@gmail.com>
This commit is contained in:
Edward Wilde
2018-07-11 22:20:22 +01:00
committed by Alex Ellis
parent a2a5380b40
commit 10e41c1840
3 changed files with 116 additions and 337 deletions

View File

@@ -12,6 +12,8 @@ import (
"os"
"time"
"net/http"
"github.com/morikuni/aec"
"github.com/openfaas/faas-cli/proxy"
"github.com/openfaas/faas-cli/stack"
@@ -49,8 +51,10 @@ func runVersion(cmd *cobra.Command, args []string) {
fmt.Println(version.BuildVersion())
} else {
printFiglet()
fmt.Printf("CLI commit: %s\n", version.GitCommit)
fmt.Printf("CLI version: %s\n", version.BuildVersion())
fmt.Printf(`CLI:
commit: %s
version: %s
`, version.GitCommit, version.BuildVersion())
printServerVersions()
}
@@ -71,14 +75,28 @@ func printServerVersions() {
gatewayAddress = getGatewayURL(gateway, defaultGateway, yamlGateway, os.Getenv(openFaaSURLEnvironment))
timeout := 2 * time.Second
timeout := 5 * time.Second
client := proxy.MakeHTTPClient(&timeout)
response, err := client.Get(gatewayAddress + "/system/info")
infoEndPoint := gatewayAddress + "/system/info"
req, err := http.NewRequest("GET", infoEndPoint, nil)
if err != nil {
fmt.Printf("Warning could not contact gateway for version information on %s %s\n", gatewayAddress+"/system/info", err.Error())
fmt.Printf("Warning could create request for %s %s\n", infoEndPoint, err.Error())
return
}
response, err := client.Do(req)
if err != nil {
fmt.Printf("Warning could not contact gateway for version information on %s %s\n", infoEndPoint, err.Error())
return
}
defer func() {
if response.Body != nil {
response.Body.Close()
}
}()
info := make(map[string]interface{})
upstreamBody, _ := ioutil.ReadAll(response.Body)
err = json.Unmarshal(upstreamBody, &info)
@@ -91,17 +109,26 @@ func printServerVersions() {
printGatewayDetails(gatewayAddress, version, sha, commit)
name, orchestration, sha, version := getProviderDetails(info)
fmt.Printf("- Provider \n\tname: %s \n\torchestration: %s \n\tversion: %s \n\tsha: %s\n",
name, orchestration, version, sha)
fmt.Printf(`
Provider
name: %s
orchestration: %s
version: %s
sha: %s
`, name, orchestration, version, sha)
}
func printGatewayDetails(gatewayAddress, version, sha, commit string) {
fmt.Printf("- Gateway \n\turi: %s", gatewayAddress)
fmt.Printf(`
Gateway
uri: %s`, gatewayAddress)
if version != "" {
fmt.Printf("\n\tversion: %s", version)
fmt.Printf("\n\tsha: %s", sha)
fmt.Printf("\n\tcommit: %s", commit)
fmt.Printf(`
version: %s
sha: %s
commit: %s
`, version, sha, commit)
}
fmt.Println()

View File

@@ -22,12 +22,14 @@ func Test_addVersionDev(t *testing.T) {
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:CLI commit: sha-test)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
expected := "commit: sha-test"
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:%s)`, expected), stdOut); err != nil || !found {
t.Fatalf("Commit is not as expected - want: %s, got:\n%s", expected, stdOut)
}
if found, err := regexp.MatchString(`(?m:CLI version: dev)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
expected = "version: dev"
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:%s)`, expected), stdOut); err != nil || !found {
t.Fatalf("Version is not as expected - want: %s, got: %s", expected, stdOut)
}
}
@@ -40,12 +42,14 @@ func Test_addVersion(t *testing.T) {
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:CLI commit: sha-test)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
expected := "commit: sha-test"
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:%s)`, expected), stdOut); err != nil || !found {
t.Fatalf("Commit is not as expected:\n%s", stdOut)
}
if found, err := regexp.MatchString(`(?m:CLI version: version.tag)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
expected = "version: version.tag"
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:%s)`, expected), stdOut); err != nil || !found {
t.Fatalf("Version is not as expected - want: %s, got: %s", expected, stdOut)
}
}
@@ -58,278 +62,103 @@ func Test_addVersion_short_version(t *testing.T) {
})
if found, err := regexp.MatchString("^version\\.tag", stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
t.Fatalf("Version is not as expected - want: %s, got: %s", version.Version, stdOut)
}
}
func Test_gateway_and_provider_information(t *testing.T) {
var testCases = []struct {
responseBody string
params []struct {
name string
value string
}
}{
{
responseBody: gateway_response_0_8_4_onwards,
params: []struct {
name string
value string
}{
{"gateway version", "version: gateway-0.4.3"},
{"gateway sha", "sha: 999a6669148c30adeb64400609953cf59db2fb64"},
{"gateway commit", "commit: Bump faas-swarm to latest"},
{"provider name", "name: faas-swarm"},
{"provider orchestration", "orchestration: swarm"},
{"provider version", "version: provider-0.3.3"},
{"provider sha", "sha: c890cba302d059de8edbef3f3de7fe15444b1ecf"},
},
},
{
responseBody: gateway_response_prior_to_0_8_4,
params: []struct {
name string
value string
}{
{"provider name", "name: faas-swarm"},
{"provider orchestration", "orchestration: swarm"},
{"provider version", "version: provider-0.3.3"},
{"provider sha", "sha: c890cba302d059de8edbef3f3de7fe15444b1ecf"},
},
},
}
for _, testCase := range testCases {
stdOut, _ := executeVersionCmd(t, testCase.responseBody)
for _, param := range testCase.params {
t.Run(param.name, func(t *testing.T) {
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:%s)`, param.value), stdOut); err != nil || !found {
t.Fatalf("%s is not as expected - want: `%s` got:\n`%s`", param.name, param.value, stdOut)
}
})
}
}
}
func Test_gateway_uri(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut, gatewayUri := executeVersionCmd(t, gateway_response_0_8_4_onwards)
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:uri: %s)`, s.URL), stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_gateway_version(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:version: gateway-0.4.3)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_gateway_sha(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:sha: 999a6669148c30adeb64400609953cf59db2fb64)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_gateway_commit(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:commit: Bump faas-swarm to latest)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_provider_name(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:name: faas-swarm)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_provider_orchestration(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:orchestration: swarm)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_provider_version(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:version: provider-0.3.3)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_provider_sha(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_0_8_4_onwards,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:sha: c890cba302d059de8edbef3f3de7fe15444b1ecf)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:uri: %s)`, gatewayUri), stdOut); err != nil || !found {
t.Fatalf("Gateway uri is not as expected - want: %s, got:\n%s", gatewayUri, stdOut)
}
}
func Test_gateway_uri_prior_to_0_8_4(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_prior_to_0_8_4,
},
})
defer s.Close()
stdOut, gatewayUri := executeVersionCmd(t, gateway_response_prior_to_0_8_4)
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:uri: %s)`, s.URL), stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
if found, err := regexp.MatchString(fmt.Sprintf(`(?m:uri: %s)`, gatewayUri), stdOut); err != nil || !found {
t.Fatalf("Gateway uri is not as expected - want: %s, got:\n%s", gatewayUri, stdOut)
}
}
func Test_gateway_details_prior_to_0_8_4_should_not_be_displayed(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_prior_to_0_8_4,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
stdOut, _ := executeVersionCmd(t, gateway_response_prior_to_0_8_4)
if found, err := regexp.MatchString(`(?m:\tversion: $)`, stdOut); err != nil || found {
t.Fatalf("Output is not as expected for version:\n%s", stdOut)
t.Fatalf("Version is not as expected - want is not to be there, got:\n%s", stdOut)
}
if found, err := regexp.MatchString(`(?m:\tsha: $)`, stdOut); err != nil || found {
t.Fatalf("Output is not as expected for sha:\n%s", stdOut)
t.Fatalf("Sha is not as expected - want it not to be there, got:\n%s", stdOut)
}
if found, err := regexp.MatchString(`(?m:\tcommit: $)`, stdOut); err != nil || found {
t.Fatalf("Output is not as expected for commit:\n%s", stdOut)
t.Fatalf("Commit is not as expected for commit - want it not to be there, got:\n%s", stdOut)
}
}
func Test_provider_name_prior_to_0_8_4(t *testing.T) {
func executeVersionCmd(t *testing.T, responseBody string) (versionInfo string, gatewayUri string) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_prior_to_0_8_4,
ResponseBody: responseBody,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
@@ -340,84 +169,7 @@ func Test_provider_name_prior_to_0_8_4(t *testing.T) {
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:name: faas-swarm)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_provider_sha_prior_to_0_8_4(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_prior_to_0_8_4,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:sha: c890cba302d059de8edbef3f3de7fe15444b1ecf)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_provider_version_prior_to_0_8_4(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_prior_to_0_8_4,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:version: provider-0.3.3)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
}
func Test_provider_orchestration_prior_to_0_8_4(t *testing.T) {
resetForTest()
s := test.MockHttpServer(t, []test.Request{
{
Method: http.MethodGet,
Uri: "/system/info",
ResponseStatusCode: http.StatusOK,
ResponseBody: gateway_response_prior_to_0_8_4,
},
})
defer s.Close()
stdOut := test.CaptureStdout(func() {
faasCmd.SetArgs([]string{
"version",
"--gateway=" + s.URL,
})
faasCmd.Execute()
})
if found, err := regexp.MatchString(`(?m:orchestration: swarm)`, stdOut); err != nil || !found {
t.Fatalf("Output is not as expected:\n%s", stdOut)
}
return stdOut, s.URL
}
const gateway_response_0_8_4_onwards = `{

View File

@@ -4,7 +4,7 @@ FAAS_BINARY=$1
GIT_COMMIT=$(git rev-list -1 HEAD)
echo "TEST: Version command prints GitCommit"
${FAAS_BINARY} version | tee /dev/stderr | grep "CLI commit: ${GIT_COMMIT}"
${FAAS_BINARY} version | tee /dev/stderr | grep "commit: ${GIT_COMMIT}"
if [ $? -eq 0 ]; then
echo OK
else