[FAB-11001] Direct errors to stderr, not stdout

When incorrect commandline arguments are passed, the output is directed
to stdout instead of stderr.

This change set fixes this, and also makes sub-commands
that return error to terminate with error code of 1
instead of 0.

Change-Id: I3e9c2172d91b25829e5b9c81aa3e812a205c9cf5
Signed-off-by: yacovm <yacovm@il.ibm.com>

Author: yacovm

cmd/common/cli.go

@@ -25,8 +25,8 @@ const (
 var (
 	// Function used to terminate the CLI
 	terminate = os.Exit
-	// Function used to obtain the stdout
-	outWriter io.Writer = os.Stdout
+	// Function used to redirect output to
+	outWriter io.Writer = os.Stderr
 
 	// CLI arguments
 	mspID                                     *string
@@ -91,6 +91,8 @@ func (cli *CLI) Run(args []string) {
 	err := f(conf)
 	if err != nil {
 		out(err)
+		terminate(1)
+		return
 	}
 }
 
@@ -153,5 +155,5 @@ func evaluateFileFlag(f **os.File) string {
 	return path
 }
 func out(a ...interface{}) {
-	fmt.Fprintln(outWriter, a)
+	fmt.Fprintln(outWriter, a...)
 }

cmd/common/cli_test.go

@@ -16,6 +16,7 @@ import (
 	"os"
 
 	"github.com/hyperledger/fabric/cmd/common/signer"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -30,6 +31,7 @@ func TestCLI(t *testing.T) {
 	testBuff := &bytes.Buffer{}
 	outWriter = testBuff
 
+	var returnValue error
 	cli := NewCLI("cli", "cli help")
 	testCommand := func(conf Config) error {
 		// If we exited, the command wasn't executed
@@ -46,7 +48,7 @@ func TestCLI(t *testing.T) {
 			},
 		}, conf)
 		testCmdInvoked = true
-		return nil
+		return returnValue
 	}
 	cli.Command("test", "test help", testCommand)
 
@@ -60,7 +62,7 @@ func TestCLI(t *testing.T) {
 		assert.True(t, exited)
 	})
 
-	t.Run("Loading a valid config", func(t *testing.T) {
+	t.Run("Loading a valid config and the command succeeds", func(t *testing.T) {
 		defer testBuff.Reset()
 		testCmdInvoked = false
 		exited = false
@@ -69,6 +71,24 @@ func TestCLI(t *testing.T) {
 		// Ensure that a valid config results in running our command
 		cli.Run([]string{"test", "--configFile", filepath.Join(dir, "config.yaml")})
 		assert.True(t, testCmdInvoked)
+		assert.False(t, exited)
+	})
+
+	t.Run("Loading a valid config but the command fails", func(t *testing.T) {
+		returnValue = errors.New("something went wrong")
+		defer func() {
+			returnValue = nil
+		}()
+		defer testBuff.Reset()
+		testCmdInvoked = false
+		exited = false
+		// Overwrite user home directory with testdata
+		dir := filepath.Join("testdata", "valid_config")
+		// Ensure that a valid config results in running our command
+		cli.Run([]string{"test", "--configFile", filepath.Join(dir, "config.yaml")})
+		assert.True(t, testCmdInvoked)
+		assert.True(t, exited)
+		assert.Contains(t, testBuff.String(), "something went wrong")
 	})
 
 	t.Run("Saving a config", func(t *testing.T) {

cmd/discover/main_test.go

@@ -0,0 +1,30 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package main
+
+import (
+	"os/exec"
+	"testing"
+
+	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gbytes"
+	. "github.com/onsi/gomega/gexec"
+)
+
+func TestMissingArguments(t *testing.T) {
+	gt := NewGomegaWithT(t)
+	discover, err := Build("github.com/hyperledger/fabric/cmd/discover")
+	gt.Expect(err).NotTo(HaveOccurred())
+	defer CleanupBuildArtifacts()
+
+	// key and cert flags are missing
+	cmd := exec.Command(discover, "--configFile", "conf.yaml", "--MSP", "SampleOrg", "saveConfig")
+	process, err := Start(cmd, nil, nil)
+	gt.Expect(err).NotTo(HaveOccurred())
+	gt.Eventually(process).Should(Exit(1))
+	gt.Expect(process.Err).To(gbytes.Say("empty string that is mandatory"))
+}

integration/discovery_service/e2e_test.go

@@ -190,8 +190,8 @@ var _ = Describe("DiscoveryService EndToEnd", func() {
 		By("discover endorsers, shouldn't get a valid result since cc was not installed yet")
 		sdRunner := sd.DiscoverEndorsers(d.Channel, "127.0.0.1:7051", "mycc", "")
 		err := helpers.Execute(sdRunner)
-		Expect(err).NotTo(HaveOccurred())
-		Expect(sdRunner.Buffer()).Should(gbytes.Say(`failed constructing descriptor for chaincodes:<name:"mycc"`))
+		Expect(err).To(HaveOccurred())
+		Expect(sdRunner.Err()).Should(gbytes.Say(`failed constructing descriptor for chaincodes:<name:"mycc"`))
 
 		By("install and instantiate chaincode on p0.org1")
 		adminPeer := getPeer(0, 1, testDir)
@@ -209,8 +209,8 @@ var _ = Describe("DiscoveryService EndToEnd", func() {
 		By("discover endorsers, shouldn't get a valid result since cc was not installed on sufficient number of orgs yet")
 		sdRunner = sd.DiscoverEndorsers(d.Channel, "127.0.0.1:7051", "mycc", "")
 		err = helpers.Execute(sdRunner)
-		Expect(err).NotTo(HaveOccurred())
-		Expect(sdRunner.Buffer()).Should(gbytes.Say(`failed constructing descriptor for chaincodes:<name:"mycc"`))
+		Expect(err).To(HaveOccurred())
+		Expect(sdRunner.Err()).Should(gbytes.Say(`failed constructing descriptor for chaincodes:<name:"mycc"`))
 
 		By("install chaincode on p0.org2")
 		adminPeer = getPeer(0, 2, testDir)
@@ -254,14 +254,13 @@ var _ = Describe("DiscoveryService EndToEnd", func() {
 		EventuallyWithOffset(1, func() bool {
 			return runner.VerifyEndorsersDiscovered(sd, expectedChaincodeEndrorsers3, d.Channel, "127.0.0.1:7051", "mycc", "collectionMarbles")
 		}, time.Minute).Should(BeTrue())
-		Expect(err).NotTo(HaveOccurred())
 		By("remove org3 members (except admin) from channel writers")
 		sendChannelConfigUpdate(0, 3, d.Channel, d.Orderer, testDir)
 		By("try to discover peers using org3, should get access denied")
 		sdRunner = sd.DiscoverPeers(d.Channel, "127.0.0.1:9051")
 		err = helpers.Execute(sdRunner)
-		Expect(err).NotTo(HaveOccurred())
-		Expect(sdRunner.Buffer()).Should(gbytes.Say("access denied"))
+		Expect(err).To(HaveOccurred())
+		Expect(sdRunner.Err()).Should(gbytes.Say("access denied"))
 	})
 })
 

integration/runner/discovery_service.go

@@ -13,7 +13,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strings"
 
 	"github.com/hyperledger/fabric/integration/helpers"
 	"github.com/hyperledger/fabric/protos/discovery"
@@ -142,13 +141,13 @@ func VerifyConfigDiscovered(sd *DiscoveryService, expectedConfig discovery.Confi
 
 func VerifyEndorsersDiscovered(sd *DiscoveryService, expectedEndorsementDescriptor helpers.EndorsementDescriptor, channel string, server string, chaincodeName string, collectionName string) bool {
 	sdRunner := sd.DiscoverEndorsers(channel, server, chaincodeName, collectionName)
-	err := helpers.Execute(sdRunner)
-	Expect(err).NotTo(HaveOccurred())
-	if strings.Contains(string(sdRunner.Buffer().Contents()[:]), fmt.Sprintf(`failed constructing descriptor for chaincodes:<name:"%s"`, chaincodeName)) {
+	if err := helpers.Execute(sdRunner); err != nil {
 		return false
 	}
 	var endorsers []helpers.EndorsementDescriptor
-	json.Unmarshal(sdRunner.Buffer().Contents(), &endorsers)
+	if err := json.Unmarshal(sdRunner.Buffer().Contents(), &endorsers); err != nil {
+		return false
+	}
 
 	return helpers.CheckEndorsementContainsExpectedEndorsement(expectedEndorsementDescriptor, endorsers[0]) &&
 		helpers.CheckEndorsementContainsExpectedEndorsement(endorsers[0], expectedEndorsementDescriptor)