chore(test): Improving tests (#440)

This will allow us to test custom driver extensions a lot more easily.

client_handler_test.go

@@ -125,7 +125,7 @@ func TestTLSMethods(t *testing.T) {
 	})
 
 	t.Run("with-implicit-tls", func(t *testing.T) {
-		s := NewTestServerWithDriver(t, &TestServerDriver{
+		s := NewTestServerWithTestDriver(t, &TestServerDriver{
 			Settings: &Settings{
 				TLSRequired: ImplicitEncryption,
 			},
@@ -145,7 +145,7 @@ func TestConnectionNotAllowed(t *testing.T) {
 		Debug:          true,
 		CloseOnConnect: true,
 	}
-	s := NewTestServerWithDriver(t, driver)
+	s := NewTestServerWithTestDriver(t, driver)
 
 	conn, err := net.DialTimeout("tcp", s.Addr(), 5*time.Second)
 	require.NoError(t, err)
@@ -173,7 +173,7 @@ func TestCloseConnection(t *testing.T) {
 	driver := &TestServerDriver{
 		Debug: false,
 	}
-	s := NewTestServerWithDriver(t, driver)
+	s := NewTestServerWithTestDriver(t, driver)
 
 	conf := goftp.Config{
 		User:     authUser,
@@ -214,7 +214,7 @@ func TestCloseConnection(t *testing.T) {
 
 func TestClientContextConcurrency(t *testing.T) {
 	driver := &TestServerDriver{}
-	s := NewTestServerWithDriver(t, driver)
+	s := NewTestServerWithTestDriver(t, driver)
 
 	conf := goftp.Config{
 		User:     authUser,
@@ -462,7 +462,7 @@ func TestExtraData(t *testing.T) {
 	driver := &TestServerDriver{
 		Debug: false,
 	}
-	s := NewTestServerWithDriver(t, driver)
+	s := NewTestServerWithTestDriver(t, driver)
 
 	conf := goftp.Config{
 		User:     authUser,

driver_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	log "github.com/fclairamb/go-log"
 	"github.com/fclairamb/go-log/gokit"
 	gklog "github.com/go-kit/log"
 	"github.com/spf13/afero"
@@ -38,13 +39,10 @@ var errInvalidTLSCertificate = errors.New("invalid TLS certificate")
 
 // NewTestServer provides a test server with or without debugging
 func NewTestServer(t *testing.T, debug bool) *FtpServer {
-	return NewTestServerWithDriver(t, &TestServerDriver{Debug: debug})
+	return NewTestServerWithTestDriver(t, &TestServerDriver{Debug: debug})
 }
 
-// NewTestServerWithDriver provides a server instantiated with some settings
-func NewTestServerWithDriver(t *testing.T, driver *TestServerDriver) *FtpServer {
-	t.Parallel()
-
+func (driver *TestServerDriver) Init() {
 	if driver.Settings == nil {
 		driver.Settings = &Settings{
 			DefaultTransferType: TransferTypeBinary,
@@ -63,15 +61,35 @@ func NewTestServerWithDriver(t *testing.T, driver *TestServerDriver) *FtpServer
 
 		driver.fs = afero.NewBasePathFs(afero.NewOsFs(), dir)
 	}
+}
 
-	s := NewFtpServer(driver)
+func NewTestServerWithTestDriver(t *testing.T, driver *TestServerDriver) *FtpServer {
+	t.Parallel()
+
+	driver.Init()
 
 	// If we are in debug mode, we should log things
+	var logger log.Logger
 	if driver.Debug {
-		s.Logger = gokit.NewWrap(gklog.NewLogfmtLogger(gklog.NewSyncWriter(os.Stdout))).With(
+		logger = gokit.NewWrap(gklog.NewLogfmtLogger(gklog.NewSyncWriter(os.Stdout))).With(
 			"ts", gokit.GKDefaultTimestampUTC,
 			"caller", gokit.GKDefaultCaller,
 		)
+	} else {
+		logger = nil
+	}
+
+	s := NewTestServerWithDriverAndLogger(t, driver, logger)
+
+	return s
+}
+
+// NewTestServerWithTestDriver provides a server instantiated with some settings
+func NewTestServerWithDriverAndLogger(t *testing.T, driver MainDriver, logger log.Logger) *FtpServer {
+	s := NewFtpServer(driver)
+
+	if logger != nil {
+		s.Logger = logger
 	}
 
 	if err := s.Listen(); err != nil {
@@ -91,6 +109,10 @@ func NewTestServerWithDriver(t *testing.T, driver *TestServerDriver) *FtpServer
 	return s
 }
 
+func NewTestServerWithDriver(t *testing.T, driver MainDriver) *FtpServer {
+	return NewTestServerWithDriverAndLogger(t, driver, nil)
+}
+
 // TestServerDriver defines a minimal serverftp server driver
 type TestServerDriver struct {
 	Debug          bool // To display connection logs information
@@ -104,8 +126,6 @@ type TestServerDriver struct {
 	TLSVerificationReply tlsVerificationReply
 	errPassiveListener   error
 	TLSRequirement       TLSRequirement
-	customAuthMessage    bool
-	customQuitMessage    bool
 }
 
 // TestClientDriver defines a minimal serverftp client driver
@@ -240,12 +260,12 @@ func (driver *TestServerDriver) AuthUser(_ ClientContext, user, pass string) (Cl
 	return nil, errBadUserNameOrPassword
 }
 
-// PostAuthMessage returns a message displayed after authentication
-func (driver *TestServerDriver) PostAuthMessage(_ ClientContext, _ string, authErr error) string {
-	if !driver.customAuthMessage {
-		return ""
-	}
+type MesssageDriver struct {
+	TestServerDriver
+}
 
+// PostAuthMessage returns a message displayed after authentication
+func (driver *MesssageDriver) PostAuthMessage(_ ClientContext, _ string, authErr error) string {
 	if authErr != nil {
 		return "You are not welcome here"
 	}
@@ -254,11 +274,7 @@ func (driver *TestServerDriver) PostAuthMessage(_ ClientContext, _ string, authE
 }
 
 // QuitMessage returns a goodbye message
-func (driver *TestServerDriver) QuitMessage() string {
-	if !driver.customQuitMessage {
-		return ""
-	}
-
+func (driver *MesssageDriver) QuitMessage() string {
 	return "Sayonara, bye bye!"
 }
 

handle_auth_test.go

@@ -91,7 +91,9 @@ func TestLoginFailure(t *testing.T) {
 }
 
 func TestLoginCustom(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{Debug: true, customAuthMessage: true})
+	driver := &MesssageDriver{}
+	driver.Init()
+	s := NewTestServerWithDriver(t, driver)
 	r := require.New(t)
 
 	conf := goftp.Config{
@@ -127,7 +129,7 @@ func TestLoginNil(t *testing.T) {
 }
 
 func TestAuthTLS(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		TLS:   true,
 	})
@@ -177,7 +179,7 @@ func TestAuthExplicitTLSFailure(t *testing.T) {
 }
 
 func TestAuthTLSRequired(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		TLS:   true,
 	})
@@ -217,7 +219,7 @@ func TestAuthTLSRequired(t *testing.T) {
 }
 
 func TestAuthTLSVerificationFailed(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug:                true,
 		TLS:                  true,
 		TLSVerificationReply: tlsVerificationFailed,
@@ -243,7 +245,7 @@ func TestAuthTLSVerificationFailed(t *testing.T) {
 }
 
 func TestAuthTLSCertificate(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug:                true,
 		TLS:                  true,
 		TLSVerificationReply: tlsVerificationAuthenticated,
@@ -274,7 +276,7 @@ func TestAuthTLSCertificate(t *testing.T) {
 }
 
 func TestAuthPerClientTLSRequired(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug:          true,
 		TLS:            true,
 		TLSRequirement: MandatoryEncryption,
@@ -313,7 +315,7 @@ func TestAuthPerClientTLSRequired(t *testing.T) {
 }
 
 func TestUserVerifierError(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		TLS:   true,
 		// setting an invalid TLS requirement will cause the test driver

handle_dirs_test.go

@@ -40,7 +40,7 @@ func TestGetRelativePaths(t *testing.T) {
 
 func TestDirListing(t *testing.T) {
 	// MLSD is disabled we relies on LIST of files listing
-	s := NewTestServerWithDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{DisableMLSD: true}})
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{DisableMLSD: true}})
 	conf := goftp.Config{
 		User:     authUser,
 		Password: authPass,
@@ -82,7 +82,7 @@ func TestDirListing(t *testing.T) {
 
 func TestDirListingPathArg(t *testing.T) {
 	// MLSD is disabled we relies on LIST of files listing
-	s := NewTestServerWithDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{DisableMLSD: true}})
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{DisableMLSD: true}})
 	conf := goftp.Config{
 		User:     authUser,
 		Password: authPass,
@@ -366,7 +366,7 @@ func TestCleanPath(t *testing.T) {
 }
 
 func TestTLSTransfer(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		TLS:   true,
 	})
@@ -412,7 +412,7 @@ func TestTLSTransfer(t *testing.T) {
 }
 
 func TestPerClientTLSTransfer(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug:          true,
 		TLS:            true,
 		TLSRequirement: MandatoryEncryption,
@@ -494,7 +494,7 @@ func TestListArgs(t *testing.T) {
 	t.Run("without-mlsd", func(t *testing.T) {
 		testListDirArgs(
 			t,
-			NewTestServerWithDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{DisableMLSD: true}}),
+			NewTestServerWithTestDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{DisableMLSD: true}}),
 		)
 	})
 }

handle_files_test.go

@@ -116,7 +116,7 @@ func TestALLO(t *testing.T) {
 }
 
 func TestCHMOD(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		TLS:   true,
 	})
@@ -200,7 +200,7 @@ func TestCHOWN(t *testing.T) {
 }
 
 func TestMFMT(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		TLS:   true,
 	})
@@ -513,7 +513,7 @@ func TestHASHDisabled(t *testing.T) {
 }
 
 func TestHASHCommand(t *testing.T) {
-	s := NewTestServerWithDriver(
+	s := NewTestServerWithTestDriver(
 		t,
 		&TestServerDriver{
 			Debug: false,
@@ -688,7 +688,7 @@ func TestCOMB(t *testing.T) {
 }
 
 func TestCOMBAppend(t *testing.T) {
-	s := NewTestServerWithDriver(
+	s := NewTestServerWithTestDriver(
 		t,
 		&TestServerDriver{
 			Debug: false,

handle_misc_test.go

@@ -44,7 +44,7 @@ func TestSiteCommand(t *testing.T) {
 // will timeout. I handle idle timeout myself in SFTPGo but you could be
 // interested to fix this bug
 func TestIdleTimeout(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{IdleTimeout: 2}})
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{IdleTimeout: 2}})
 	conf := goftp.Config{
 		User:     authUser,
 		Password: authPass,
@@ -151,7 +151,7 @@ func TestOPTSUTF8(t *testing.T) {
 }
 
 func TestOPTSHASH(t *testing.T) {
-	s := NewTestServerWithDriver(
+	s := NewTestServerWithTestDriver(
 		t,
 		&TestServerDriver{
 			Debug: false,
@@ -247,7 +247,7 @@ func TestAVBL(t *testing.T) {
 }
 
 func TestQuit(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		TLS:   true,
 	})
@@ -276,11 +276,14 @@ func TestQuit(t *testing.T) {
 }
 
 func TestQuitWithCustomMessage(_t *testing.T) {
-	s := NewTestServerWithDriver(_t, &TestServerDriver{
-		Debug:             true,
-		TLS:               true,
-		customQuitMessage: true,
-	})
+	d := &MesssageDriver{
+		TestServerDriver{
+			Debug: true,
+			TLS:   true,
+		},
+	}
+	d.Init()
+	s := NewTestServerWithDriver(_t, d)
 	t := require.New(_t)
 	conf := goftp.Config{
 		User:     authUser,
@@ -306,7 +309,7 @@ func TestQuitWithCustomMessage(_t *testing.T) {
 }
 
 func TestQuitWithTransferInProgress(t *testing.T) {
-	s := NewTestServerWithDriver(t, &TestServerDriver{
+	s := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 	})
 	conf := goftp.Config{

transfer_active_test.go

@@ -32,7 +32,7 @@ func TestActiveTransferFromPort20(t *testing.T) {
 	err = listener.Close()
 	require.NoError(t, err)
 
-	server := NewTestServerWithDriver(t, &TestServerDriver{
+	server := NewTestServerWithTestDriver(t, &TestServerDriver{
 		Debug: false,
 		Settings: &Settings{
 			ActiveTransferPortNon20: false,