From 365cb3f70cbaab7eb2e3d1a63066085748a61daa Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Sat, 2 Mar 2024 19:24:59 +0100 Subject: [PATCH] chore(test): Improving tests (#440) This will allow us to test custom driver extensions a lot more easily. --- client_handler_test.go | 10 ++++---- driver_test.go | 54 ++++++++++++++++++++++++++--------------- handle_auth_test.go | 16 ++++++------ handle_dirs_test.go | 10 ++++---- handle_files_test.go | 8 +++--- handle_misc_test.go | 21 +++++++++------- transfer_active_test.go | 2 +- transfer_test.go | 20 +++++++-------- 8 files changed, 81 insertions(+), 60 deletions(-) diff --git a/client_handler_test.go b/client_handler_test.go index 4fb26681..ae8738c6 100644 --- a/client_handler_test.go +++ b/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, diff --git a/driver_test.go b/driver_test.go index ddcbed30..2365a1b3 100644 --- a/driver_test.go +++ b/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!" } diff --git a/handle_auth_test.go b/handle_auth_test.go index e4c6190d..5186a4ca 100644 --- a/handle_auth_test.go +++ b/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 diff --git a/handle_dirs_test.go b/handle_dirs_test.go index 525f44a0..7043fad7 100644 --- a/handle_dirs_test.go +++ b/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}}), ) }) } diff --git a/handle_files_test.go b/handle_files_test.go index 9ef83444..f6b059d8 100644 --- a/handle_files_test.go +++ b/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, diff --git a/handle_misc_test.go b/handle_misc_test.go index 519ebfb1..ad9dcd92 100644 --- a/handle_misc_test.go +++ b/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{ diff --git a/transfer_active_test.go b/transfer_active_test.go index aef87a43..19bff436 100644 --- a/transfer_active_test.go +++ b/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, diff --git a/transfer_test.go b/transfer_test.go index f556361b..80345c5c 100644 --- a/transfer_test.go +++ b/transfer_test.go @@ -168,7 +168,7 @@ func ftpDelete(t *testing.T, ftp *goftp.Client, filename string) { } func TestTransferIPv6(t *testing.T) { - s := NewTestServerWithDriver( + s := NewTestServerWithTestDriver( t, &TestServerDriver{ Debug: false, @@ -190,7 +190,7 @@ func TestTransferIPv6(t *testing.T) { // TestTransfer validates the upload of file in both active and passive mode func TestTransfer(t *testing.T) { t.Run("without-tls", func(t *testing.T) { - s := NewTestServerWithDriver( + s := NewTestServerWithTestDriver( t, &TestServerDriver{ Debug: false, @@ -204,7 +204,7 @@ func TestTransfer(t *testing.T) { testTransferOnConnection(t, s, true, false, false) }) t.Run("with-tls", func(t *testing.T) { - s := NewTestServerWithDriver( + s := NewTestServerWithTestDriver( t, &TestServerDriver{ Debug: false, @@ -220,7 +220,7 @@ func TestTransfer(t *testing.T) { }) t.Run("with-implicit-tls", func(t *testing.T) { - s := NewTestServerWithDriver(t, &TestServerDriver{ + s := NewTestServerWithTestDriver(t, &TestServerDriver{ Debug: false, TLS: true, Settings: &Settings{ @@ -273,7 +273,7 @@ func testTransferOnConnection(t *testing.T, server *FtpServer, active, enableTLS } func TestActiveModeDisabled(t *testing.T) { - server := NewTestServerWithDriver(t, &TestServerDriver{ + server := NewTestServerWithTestDriver(t, &TestServerDriver{ Debug: false, Settings: &Settings{ ActiveTransferPortNon20: true, @@ -382,7 +382,7 @@ func TestFailingFileTransfer(t *testing.T) { driver := &TestServerDriver{ Debug: false, } - s := NewTestServerWithDriver(t, driver) + s := NewTestServerWithTestDriver(t, driver) conf := goftp.Config{ User: authUser, Password: authPass, @@ -446,7 +446,7 @@ func TestAPPEExistingFile(t *testing.T) { driver := &TestServerDriver{ Debug: false, } - s := NewTestServerWithDriver(t, driver) + s := NewTestServerWithTestDriver(t, driver) conf := goftp.Config{ User: authUser, Password: authPass, @@ -495,7 +495,7 @@ func TestAPPENewFile(t *testing.T) { driver := &TestServerDriver{ Debug: false, } - s := NewTestServerWithDriver(t, driver) + s := NewTestServerWithTestDriver(t, driver) conf := goftp.Config{ User: authUser, Password: authPass, @@ -527,7 +527,7 @@ func TestTransfersFromOffset(t *testing.T) { driver := &TestServerDriver{ Debug: false, } - s := NewTestServerWithDriver(t, driver) + s := NewTestServerWithTestDriver(t, driver) conf := goftp.Config{ User: authUser, Password: authPass, @@ -924,7 +924,7 @@ func TestASCIITransfersInvalidFiles(t *testing.T) { } func TestPASVWrappedListenerError(t *testing.T) { - s := NewTestServerWithDriver(t, &TestServerDriver{ + s := NewTestServerWithTestDriver(t, &TestServerDriver{ Debug: true, errPassiveListener: os.ErrClosed, })