Merge pull request #2656 from fjl/console-test

cmd/geth: make console tests more robust

3 files changed, 45 insertions, 35 deletions

diff --git a/cmd/geth/consolecmd_test.go b/cmd/geth/consolecmd_test.go
index 9cfb3e4e3..e59fe1415 100644
--- a/cmd/geth/consolecmd_test.go
+++ b/cmd/geth/consolecmd_test.go
@@ -27,7 +27,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/ethereum/go-ethereum/console"
 	"github.com/ethereum/go-ethereum/rpc"
 )
 
@@ -37,9 +36,10 @@ func TestConsoleWelcome(t *testing.T) {
 	coinbase := "0x8605cdbbdb6d264aa742e77020dcbc58fcdce182"
 
 	// Start a geth console, make sure it's cleaned up and terminate the console
-	geth := runGeth(t, "--nat", "none", "--nodiscover", "--etherbase", coinbase, "-shh", "console")
-	defer geth.expectExit()
-	geth.stdin.Close()
+	geth := runGeth(t,
+		"--port", "0", "--maxpeers", "0", "--nodiscover", "--nat", "none",
+		"--etherbase", coinbase, "--shh",
+		"console")
 
 	// Gather all the infos the welcome message needs to contain
 	geth.setTemplateFunc("goos", func() string { return runtime.GOOS })
@@ -51,7 +51,6 @@ func TestConsoleWelcome(t *testing.T) {
 		sort.Strings(apis)
 		return apis
 	})
-	geth.setTemplateFunc("prompt", func() string { return console.DefaultPrompt })
 
 	// Verify the actual welcome message to the required template
 	geth.expect(`
@@ -63,52 +62,63 @@ at block: 0 ({{niltime}})
  datadir: {{.Datadir}}
  modules:{{range apis}} {{.}}:1.0{{end}}
 
-{{prompt}}
+> {{.InputLine "exit"}}
 `)
+	geth.expectExit()
 }
 
 // Tests that a console can be attached to a running node via various means.
 func TestIPCAttachWelcome(t *testing.T) {
 	// Configure the instance for IPC attachement
 	coinbase := "0x8605cdbbdb6d264aa742e77020dcbc58fcdce182"
-
 	var ipc string
 	if runtime.GOOS == "windows" {
 		ipc = `\\.\pipe\geth` + strconv.Itoa(rand.Int())
 	} else {
 		ws := tmpdir(t)
 		defer os.RemoveAll(ws)
-
 		ipc = filepath.Join(ws, "geth.ipc")
 	}
-	// Run the parent geth and attach with a child console
-	geth := runGeth(t, "--nat", "none", "--nodiscover", "--etherbase", coinbase, "-shh", "--ipcpath", ipc)
-	defer geth.interrupt()
+	// Note: we need --shh because testAttachWelcome checks for default
+	// list of ipc modules and shh is included there.
+	geth := runGeth(t,
+		"--port", "0", "--maxpeers", "0", "--nodiscover", "--nat", "none",
+		"--etherbase", coinbase, "--shh", "--ipcpath", ipc)
 
 	time.Sleep(2 * time.Second) // Simple way to wait for the RPC endpoint to open
 	testAttachWelcome(t, geth, "ipc:"+ipc)
+
+	geth.interrupt()
+	geth.expectExit()
 }
 
 func TestHTTPAttachWelcome(t *testing.T) {
 	coinbase := "0x8605cdbbdb6d264aa742e77020dcbc58fcdce182"
 	port := strconv.Itoa(rand.Intn(65535-1024) + 1024) // Yeah, sometimes this will fail, sorry :P
-
-	geth := runGeth(t, "--nat", "none", "--nodiscover", "--etherbase", coinbase, "--rpc", "--rpcport", port)
-	defer geth.interrupt()
+	geth := runGeth(t,
+		"--port", "0", "--maxpeers", "0", "--nodiscover", "--nat", "none",
+		"--etherbase", coinbase, "--rpc", "--rpcport", port)
 
 	time.Sleep(2 * time.Second) // Simple way to wait for the RPC endpoint to open
 	testAttachWelcome(t, geth, "http://localhost:"+port)
+
+	geth.interrupt()
+	geth.expectExit()
 }
 
 func TestWSAttachWelcome(t *testing.T) {
 	coinbase := "0x8605cdbbdb6d264aa742e77020dcbc58fcdce182"
 	port := strconv.Itoa(rand.Intn(65535-1024) + 1024) // Yeah, sometimes this will fail, sorry :P
 
-	geth := runGeth(t, "--nat", "none", "--nodiscover", "--etherbase", coinbase, "--ws", "--wsport", port)
-	defer geth.interrupt()
+	geth := runGeth(t,
+		"--port", "0", "--maxpeers", "0", "--nodiscover", "--nat", "none",
+		"--etherbase", coinbase, "--ws", "--wsport", port)
 
 	time.Sleep(2 * time.Second) // Simple way to wait for the RPC endpoint to open
 	testAttachWelcome(t, geth, "ws://localhost:"+port)
+
+	geth.interrupt()
+	geth.expectExit()
 }
 
 func testAttachWelcome(t *testing.T, geth *testgeth, endpoint string) {
@@ -135,7 +145,6 @@ func testAttachWelcome(t *testing.T, geth *testgeth, endpoint string) {
 		sort.Strings(apis)
 		return apis
 	})
-	attach.setTemplateFunc("prompt", func() string { return console.DefaultPrompt })
 
 	// Verify the actual welcome message to the required template
 	attach.expect(`
@@ -147,6 +156,7 @@ at block: 0 ({{niltime}}){{if ipc}}
  datadir: {{datadir}}{{end}}
  modules:{{range apis}} {{.}}:1.0{{end}}
 
-{{prompt}}
+> {{.InputLine "exit" }}
 `)
+	attach.expectExit()
 }
diff --git a/console/console.go b/console/console.go
index a19b267bc..baa9cf545 100644
--- a/console/console.go
+++ b/console/console.go
@@ -244,15 +244,13 @@ func (c *Console) AutoCompleteInput(line string, pos int) (string, []string, str
 // console's available modules.
 func (c *Console) Welcome() {
 	// Print some generic Geth metadata
+	fmt.Fprintf(c.printer, "Welcome to the Geth JavaScript console!\n\n")
 	c.jsre.Run(`
-    (function () {
-			console.log("Welcome to the Geth JavaScript console!\n");
-      console.log("instance: " + web3.version.node);
-      console.log("coinbase: " + eth.coinbase);
-      console.log("at block: " + eth.blockNumber + " (" + new Date(1000 * eth.getBlock(eth.blockNumber).timestamp) + ")");
-      console.log(" datadir: " + admin.datadir);
-    })();
-  `)
+		console.log("instance: " + web3.version.node);
+		console.log("coinbase: " + eth.coinbase);
+		console.log("at block: " + eth.blockNumber + " (" + new Date(1000 * eth.getBlock(eth.blockNumber).timestamp) + ")");
+		console.log(" datadir: " + admin.datadir);
+	`)
 	// List all the supported modules for the user to call
 	if apis, err := c.client.SupportedModules(); err == nil {
 		modules := make([]string, 0, len(apis))
@@ -260,9 +258,9 @@ func (c *Console) Welcome() {
 			modules = append(modules, fmt.Sprintf("%s:%s", api, version))
 		}
 		sort.Strings(modules)
-		c.jsre.Run("(function () { console.log(' modules: " + strings.Join(modules, " ") + "'); })();")
+		fmt.Fprintln(c.printer, " modules:", strings.Join(modules, " "))
 	}
-	c.jsre.Run("(function () { console.log(); })();")
+	fmt.Fprintln(c.printer)
 }
 
 // Evaluate executes code and pretty prints the result to the specified output
diff --git a/internal/jsre/jsre.go b/internal/jsre/jsre.go
index a95efd379..481389304 100644
--- a/internal/jsre/jsre.go
+++ b/internal/jsre/jsre.go
@@ -24,7 +24,6 @@ import (
 	"io"
 	"io/ioutil"
 	"math/rand"
-	"sync"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -44,7 +43,7 @@ type JSRE struct {
 	output        io.Writer
 	evalQueue     chan *evalReq
 	stopEventLoop chan bool
-	loopWg        sync.WaitGroup
+	closed        chan struct{}
 }
 
 // jsTimer is a single timer instance with a callback function
@@ -66,10 +65,10 @@ func New(assetPath string, output io.Writer) *JSRE {
 	re := &JSRE{
 		assetPath:     assetPath,
 		output:        output,
+		closed:        make(chan struct{}),
 		evalQueue:     make(chan *evalReq),
 		stopEventLoop: make(chan bool),
 	}
-	re.loopWg.Add(1)
 	go re.runEventLoop()
 	re.Set("loadScript", re.loadScript)
 	re.Set("inspect", prettyPrintJS)
@@ -98,6 +97,8 @@ func randomSource() *rand.Rand {
 // functions should be used if and only if running a routine that was already
 // called from JS through an RPC call.
 func (self *JSRE) runEventLoop() {
+	defer close(self.closed)
+
 	vm := otto.New()
 	r := randomSource()
 	vm.SetRandomSource(r.Float64)
@@ -213,8 +214,6 @@ loop:
 		timer.timer.Stop()
 		delete(registry, timer)
 	}
-
-	self.loopWg.Done()
 }
 
 // Do executes the given function on the JS event loop.
@@ -227,8 +226,11 @@ func (self *JSRE) Do(fn func(*otto.Otto)) {
 
 // stops the event loop before exit, optionally waits for all timers to expire
 func (self *JSRE) Stop(waitForCallbacks bool) {
-	self.stopEventLoop <- waitForCallbacks
-	self.loopWg.Wait()
+	select {
+	case <-self.closed:
+	case self.stopEventLoop <- waitForCallbacks:
+		<-self.closed
+	}
 }
 
 // Exec(file) loads and runs the contents of a file