From dcab1a6708b30bad59d55221655e29b96eaeec63 Mon Sep 17 00:00:00 2001
From: Seebs <seebs@molecula.com>
Date: Tue, 16 Nov 2021 14:23:35 -0600
Subject: [PATCH] unbreak featurebase holder subcommand

The "just open the holder" subcommand doesn't work the way it used
to, because now that we rely on etcd to open a holder, trying to open
a holder without things set up just coredumps.

Step 1: Fix that.
Step 2: Also add a test that covers it so we don't get bitrotted again.
Step 3: Remove an unrelated stale comment that doesn't deserve its
own commit log, having to do with an option that no longer exists
which is no longer being set right under the comment saying we set it.
---
 server.go             | 18 +++++-------------
 server/server.go      |  3 ++-
 server/server_test.go |  8 ++++++++
 test/pilosa.go        |  2 --
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/server.go b/server.go
index 8058c25f4..bcb86f119 100644
--- a/server.go
+++ b/server.go
@@ -538,25 +538,17 @@ func (s *Server) SetAPI(api *API) {
 // UpAndDown brings the server up minimally and shuts it down
 // again; basically, it exists for testing holder open and close.
 func (s *Server) UpAndDown() error {
-	s.logger.Infof("open server. PID %v", os.Getpid())
-
 	// Log startup
 	err := s.holder.logStartup()
 	if err != nil {
 		log.Println(errors.Wrap(err, "logging startup"))
 	}
-
-	// Open holder.
-	if err := s.holder.Open(); err != nil {
-		return errors.Wrap(err, "opening Holder")
+	s.logger.Infof("open server. PID %v", os.Getpid())
+	if err = s.Open(); err != nil {
+		return errors.Wrap(err, "starting server")
 	}
-
-	errh := s.holder.Close()
-	if errh != nil {
-		return errors.Wrap(errh, "closing holder")
-	}
-
-	return nil
+	err = s.Close()
+	return errors.Wrap(err, "shutting down server")
 }
 
 // Open opens and initializes the server.
diff --git a/server/server.go b/server/server.go
index 3feaf4582..29eb20f1d 100644
--- a/server/server.go
+++ b/server/server.go
@@ -293,6 +293,7 @@ func (m *Command) UpAndDown() (err error) {
 	if err != nil {
 		return errors.Wrap(err, "setting up server")
 	}
+	m.logger.Infof("bringing server up and shutting it down immediately")
 
 	go func() {
 		err := m.Handler.Serve()
@@ -306,7 +307,7 @@ func (m *Command) UpAndDown() (err error) {
 		return errors.Wrap(err, "bringing server up and down")
 	}
 
-	m.logger.Errorf("brought up and shut down again")
+	m.logger.Infof("teardown complete")
 
 	return nil
 }
diff --git a/server/server_test.go b/server/server_test.go
index 5e2a47dfa..a449d5838 100644
--- a/server/server_test.go
+++ b/server/server_test.go
@@ -531,6 +531,14 @@ func TestClusteringNodesReplica1(t *testing.T) {
 	}
 }
 
+func TestUpAndDown(t *testing.T) {
+	c := test.NewCommandNode(t)
+	err := c.UpAndDown()
+	if err != nil {
+		t.Fatalf("server up-and-down: %v", err)
+	}
+}
+
 func TestClusteringNodesReplica2(t *testing.T) {
 	// Because this test shuts down 2 nodes, it needs to start as a 5-node
 	// cluster in order to retain enough available nodes for raft leader
diff --git a/test/pilosa.go b/test/pilosa.go
index 925213cd1..d1deb491e 100644
--- a/test/pilosa.go
+++ b/test/pilosa.go
@@ -60,8 +60,6 @@ func newCommand(tb testing.TB, opts ...server.CommandOption) *Command {
 	// a problem with PDK tests which used pilosa/client as well. We put it at the
 	// beginning of the option slice so that it can be overridden by user-passed
 	// options.
-	// Also set TranslateFile MapSize to a smaller number so memory allocation
-	// does not fail on 32-bit systems.
 	opts = append([]server.CommandOption{
 		server.OptCommandCloseTimeout(time.Millisecond * 2),
 	}, opts...)