From 2c98f7fd8fc58458d590c360af6a80754988903e Mon Sep 17 00:00:00 2001
From: Injun Song <ijsong@gmail.com>
Date: Mon, 25 Sep 2023 18:51:35 +0900
Subject: [PATCH] feat(varlogtest): create new instance of admin and log

This PR renames `pkg/varlogtest.(*VarlogTest).Admin` and `pkg/varlogtest.(*VarlogTest).Log` to
`pkg/varlogtest.(*VarlogTest).NewAdminClient` and `pkg/varlogtest.(*VarlogTest).NewLogClient`
respectively. It also makes them return a new instance of the admin client and the log client when
invoked.
Previously, they returned the same instances, so callers shared them. However, it wasn't the right
decision because the callers had to share the clients regardless of their needs.
---
 pkg/varlogtest/admin.go           |  7 ++++---
 pkg/varlogtest/log.go             |  9 +++++----
 pkg/varlogtest/varlogtest.go      | 16 ++++------------
 pkg/varlogtest/varlogtest_test.go | 14 +++++++-------
 4 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/pkg/varlogtest/admin.go b/pkg/varlogtest/admin.go
index 4fc39a0e1..c1453211e 100644
--- a/pkg/varlogtest/admin.go
+++ b/pkg/varlogtest/admin.go
@@ -23,14 +23,15 @@ import (
 )
 
 type testAdmin struct {
-	vt *VarlogTest
+	vt     *VarlogTest
+	closed bool
 }
 
 var _ varlog.Admin = (*testAdmin)(nil)
 
 func (c *testAdmin) lock() error {
 	c.vt.cond.L.Lock()
-	if c.vt.adminClientClosed {
+	if c.closed {
 		c.vt.cond.L.Unlock()
 		return verrors.ErrClosed
 	}
@@ -534,6 +535,6 @@ func (c *testAdmin) RemoveMRPeer(ctx context.Context, raftURL string, opts ...va
 func (c *testAdmin) Close() error {
 	c.vt.cond.L.Lock()
 	defer c.vt.cond.L.Unlock()
-	c.vt.adminClientClosed = true
+	c.closed = true
 	return nil
 }
diff --git a/pkg/varlogtest/log.go b/pkg/varlogtest/log.go
index f4b0abc65..1a78742dc 100644
--- a/pkg/varlogtest/log.go
+++ b/pkg/varlogtest/log.go
@@ -14,14 +14,15 @@ import (
 )
 
 type testLog struct {
-	vt *VarlogTest
+	vt     *VarlogTest
+	closed bool
 }
 
 var _ varlog.Log = (*testLog)(nil)
 
 func (c *testLog) lock() error {
 	c.vt.cond.L.Lock()
-	if c.vt.varlogClientClosed {
+	if c.closed {
 		c.vt.cond.L.Unlock()
 		return verrors.ErrClosed
 	}
@@ -35,7 +36,7 @@ func (c *testLog) unlock() {
 func (c *testLog) Close() error {
 	c.vt.cond.L.Lock()
 	defer c.vt.cond.L.Unlock()
-	c.vt.varlogClientClosed = true
+	c.closed = true
 	c.vt.cond.Broadcast()
 	return nil
 }
@@ -247,7 +248,7 @@ func (c *testLog) SubscribeTo(ctx context.Context, topicID types.TopicID, logStr
 		return logEntries
 	}
 	s.vt.closedClient = func() bool {
-		return c.vt.varlogClientClosed
+		return c.closed
 	}
 
 	s.wg.Add(1)
diff --git a/pkg/varlogtest/varlogtest.go b/pkg/varlogtest/varlogtest.go
index d0688b80a..1fee01a50 100644
--- a/pkg/varlogtest/varlogtest.go
+++ b/pkg/varlogtest/varlogtest.go
@@ -17,9 +17,6 @@ import (
 type VarlogTest struct {
 	config
 
-	admin *testAdmin
-	vlg   *testLog
-
 	rng *rand.Rand
 
 	mu               sync.Mutex
@@ -38,9 +35,6 @@ type VarlogTest struct {
 	nextTopicID       types.TopicID
 	nextStorageNodeID types.StorageNodeID
 	nextLogStreamID   types.LogStreamID
-
-	adminClientClosed  bool
-	varlogClientClosed bool
 }
 
 func New(opts ...Option) (*VarlogTest, error) {
@@ -62,8 +56,6 @@ func New(opts ...Option) (*VarlogTest, error) {
 		leaderMR:         types.InvalidNodeID,
 	}
 	vt.cond = sync.NewCond(&vt.mu)
-	vt.admin = &testAdmin{vt: vt}
-	vt.vlg = &testLog{vt: vt}
 
 	for _, mrn := range vt.initialMRNodes {
 		if vt.leaderMR == types.InvalidNodeID {
@@ -76,12 +68,12 @@ func New(opts ...Option) (*VarlogTest, error) {
 	return vt, nil
 }
 
-func (vt *VarlogTest) Admin() varlog.Admin {
-	return vt.admin
+func (vt *VarlogTest) NewAdminClient() varlog.Admin {
+	return &testAdmin{vt: vt}
 }
 
-func (vt *VarlogTest) Log() varlog.Log {
-	return vt.vlg
+func (vt *VarlogTest) NewLogClient() varlog.Log {
+	return &testLog{vt: vt}
 }
 
 func (vt *VarlogTest) generateTopicID() types.TopicID {
diff --git a/pkg/varlogtest/varlogtest_test.go b/pkg/varlogtest/varlogtest_test.go
index dd951a0ef..257663bf5 100644
--- a/pkg/varlogtest/varlogtest_test.go
+++ b/pkg/varlogtest/varlogtest_test.go
@@ -115,8 +115,8 @@ func TestVarlotTest_LogStreamAppender(t *testing.T) {
 			)
 			require.NoError(t, err)
 
-			adm := vt.Admin()
-			vlg := vt.Log()
+			adm := vt.NewAdminClient()
+			vlg := vt.NewLogClient()
 			defer func() {
 				require.NoError(t, vlg.Close())
 				require.NoError(t, adm.Close())
@@ -176,8 +176,8 @@ func TestVarlogTest(t *testing.T) {
 	)
 	require.NoError(t, err)
 
-	adm := vt.Admin()
-	vlg := vt.Log()
+	adm := vt.NewAdminClient()
+	vlg := vt.NewLogClient()
 	defer func() {
 		require.NoError(t, vlg.Close())
 		require.NoError(t, adm.Close())
@@ -594,8 +594,8 @@ func TestVarlogTest_Trim(t *testing.T) {
 	)
 	require.NoError(t, err)
 
-	adm := vt.Admin()
-	vlg := vt.Log()
+	adm := vt.NewAdminClient()
+	vlg := vt.NewLogClient()
 	defer func() {
 		require.NoError(t, vlg.Close())
 		require.NoError(t, adm.Close())
@@ -783,7 +783,7 @@ func TestVarlogTestAdminMetadataRepository(t *testing.T) {
 			)
 			require.NoError(t, err)
 
-			tc.testf(t, vt.Admin())
+			tc.testf(t, vt.NewAdminClient())
 		})
 	}
 }