diff --git a/cmd/run.go b/cmd/run.go index 6a9aeaa..3019ad6 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -56,7 +56,7 @@ var runCmd = &cobra.Command{ excludeRE = regexp.MustCompile(exclude) } - currentRun := runner.Run(tests, runner.Config{ + currentRun, err := runner.Run(tests, runner.Config{ Include: includeRE, Exclude: excludeRE, ShowTime: showTime, @@ -64,6 +64,9 @@ var runCmd = &cobra.Command{ ConnectTimeout: connectTimeout, ReadTimeout: readTimeout, }) + if err != nil { + log.Fatal().Err(err) + } os.Exit(currentRun.Stats.TotalFailed()) }, diff --git a/ftwhttp/client.go b/ftwhttp/client.go index 729b78b..4d32e2b 100644 --- a/ftwhttp/client.go +++ b/ftwhttp/client.go @@ -21,17 +21,17 @@ func NewClientConfig() ClientConfig { } // NewClient initializes the http client, creating the cookiejar -func NewClient(config ClientConfig) *Client { +func NewClient(config ClientConfig) (*Client, error) { // All users of cookiejar should import "golang.org/x/net/publicsuffix" jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List}) if err != nil { - log.Fatal().Err(err) + return nil, err } c := &Client{ Jar: jar, config: config, } - return c + return c, nil } // NewConnection creates a new Connection based on a Destination diff --git a/ftwhttp/client_test.go b/ftwhttp/client_test.go index 59cf6bb..d58102e 100644 --- a/ftwhttp/client_test.go +++ b/ftwhttp/client_test.go @@ -7,7 +7,8 @@ import ( ) func TestNewClient(t *testing.T) { - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) assert.NotNil(t, c.Jar, "Error creating Client") } @@ -19,9 +20,10 @@ func TestConnectDestinationHTTPS(t *testing.T) { Protocol: "https", } - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) - err := c.NewConnection(*d) + err = c.NewConnection(*d) assert.NoError(t, err, "This should not error") assert.Equal(t, "https", c.Transport.protocol, "Error connecting to example.com using https") } @@ -33,11 +35,12 @@ func TestDoRequest(t *testing.T) { Protocol: "https", } - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) req := generateBaseRequestForTesting() - err := c.NewConnection(*d) + err = c.NewConnection(*d) assert.NoError(t, err, "This should not error") _, err = c.Do(*req) @@ -52,7 +55,8 @@ func TestGetTrackedTime(t *testing.T) { Protocol: "https", } - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) rl := &RequestLine{ Method: "POST", @@ -65,7 +69,7 @@ func TestGetTrackedTime(t *testing.T) { data := []byte(`test=me&one=two&one=twice`) req := NewRequest(rl, h, data, true) - err := c.NewConnection(*d) + err = c.NewConnection(*d) assert.NoError(t, err, "This should not error") c.StartTrackingTime() @@ -90,7 +94,8 @@ func TestClientMultipartFormDataRequest(t *testing.T) { Protocol: "https", } - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) rl := &RequestLine{ Method: "POST", @@ -112,7 +117,7 @@ Some-file-test-here req := NewRequest(rl, h, data, true) - err := c.NewConnection(*d) + err = c.NewConnection(*d) assert.NoError(t, err, "This should not error") c.StartTrackingTime() @@ -127,7 +132,8 @@ Some-file-test-here } func TestNewConnectionCreatesTransport(t *testing.T) { - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) assert.Nil(t, c.Transport, "Transport not expected to initialized yet") server := testServer() @@ -141,7 +147,8 @@ func TestNewConnectionCreatesTransport(t *testing.T) { } func TestNewOrReusedConnectionCreatesTransport(t *testing.T) { - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) assert.Nil(t, c.Transport, "Transport not expected to initialized yet") server := testServer() @@ -155,7 +162,8 @@ func TestNewOrReusedConnectionCreatesTransport(t *testing.T) { } func TestNewOrReusedConnectionReusesTransport(t *testing.T) { - c := NewClient(NewClientConfig()) + c, err := NewClient(NewClientConfig()) + assert.NoError(t, err) assert.Nil(t, c.Transport, "Transport not expected to initialized yet") server := testServer() diff --git a/ftwhttp/connection.go b/ftwhttp/connection.go index f02f333..d273780 100644 --- a/ftwhttp/connection.go +++ b/ftwhttp/connection.go @@ -5,6 +5,7 @@ import ( "bufio" "bytes" "errors" + "fmt" "io" "net" "net/http" @@ -82,7 +83,7 @@ func (c *Connection) Request(request *Request) error { // Build request first, then connect and send, so timers are accurate data, err := buildRequest(request) if err != nil { - log.Fatal().Msgf("ftw/http: fatal error building request: %s", err.Error()) + return fmt.Errorf("ftw/http: fatal error building request: %w", err) } log.Debug().Msgf("ftw/http: sending data:\n%s\n", data) diff --git a/ftwhttp/response_test.go b/ftwhttp/response_test.go index 1503a46..868feb6 100644 --- a/ftwhttp/response_test.go +++ b/ftwhttp/response_test.go @@ -88,7 +88,8 @@ func TestResponse(t *testing.T) { req := generateRequestForTesting(true) - client := NewClient(NewClientConfig()) + client, err := NewClient(NewClientConfig()) + assert.NoError(t, err) err = client.NewConnection(*d) assert.NoError(t, err) @@ -107,7 +108,8 @@ func TestResponseWithCookies(t *testing.T) { assert.NoError(t, err) req := generateRequestForTesting(true) - client := NewClient(NewClientConfig()) + client, err := NewClient(NewClientConfig()) + assert.NoError(t, err) err = client.NewConnection(*d) assert.NoError(t, err) diff --git a/runner/run.go b/runner/run.go index 1671018..4c454ed 100644 --- a/runner/run.go +++ b/runner/run.go @@ -1,6 +1,7 @@ package runner import ( + "errors" "fmt" "regexp" "time" @@ -17,11 +18,16 @@ import ( "github.com/coreruleset/go-ftw/waflog" ) +var errBadTestRequest = errors.New("ftw/run: bad test: choose between data, encoded_request, or raw_request") + // Run runs your tests with the specified Config. Returns error if some test failed -func Run(tests []test.FTWTest, c Config) TestRunContext { +func Run(tests []test.FTWTest, c Config) (TestRunContext, error) { printUnlessQuietMode(c.Quiet, ":rocket:Running go-ftw!\n") - logLines := waflog.NewFTWLogLines(waflog.WithLogFile(config.FTWConfig.LogFile)) + logLines, err := waflog.NewFTWLogLines(waflog.WithLogFile(config.FTWConfig.LogFile)) + if err != nil { + return TestRunContext{}, err + } conf := ftwhttp.NewClientConfig() if c.ConnectTimeout != 0 { @@ -30,7 +36,10 @@ func Run(tests []test.FTWTest, c Config) TestRunContext { if c.ReadTimeout != 0 { conf.ReadTimeout = c.ReadTimeout } - client := ftwhttp.NewClient(conf) + client, err := ftwhttp.NewClient(conf) + if err != nil { + return TestRunContext{}, err + } runContext := TestRunContext{ Include: c.Include, Exclude: c.Exclude, @@ -41,21 +50,23 @@ func Run(tests []test.FTWTest, c Config) TestRunContext { RunMode: config.FTWConfig.RunMode, } - for _, test := range tests { - RunTest(&runContext, test) + for _, tc := range tests { + if err := RunTest(&runContext, tc); err != nil { + return TestRunContext{}, err + } } printSummary(c.Quiet, runContext.Stats) defer cleanLogs(logLines) - return runContext + return runContext, nil } // RunTest runs an individual test. // runContext contains information for the current test run // ftwTest is the test you want to run -func RunTest(runContext *TestRunContext, ftwTest test.FTWTest) { +func RunTest(runContext *TestRunContext, ftwTest test.FTWTest) error { changed := true for _, testCase := range ftwTest.Tests { @@ -78,9 +89,13 @@ func RunTest(runContext *TestRunContext, ftwTest test.FTWTest) { // Iterate over stages for _, stage := range testCase.Stages { ftwCheck := check.NewCheck(config.FTWConfig) - RunStage(runContext, ftwCheck, testCase, stage.Stage) + if err := RunStage(runContext, ftwCheck, testCase, stage.Stage); err != nil { + return err + } } } + + return nil } // RunStage runs an individual test stage. @@ -88,7 +103,7 @@ func RunTest(runContext *TestRunContext, ftwTest test.FTWTest) { // ftwCheck is the current check utility // testCase is the test case the stage belongs to // stage is the stage you want to run -func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase test.Test, stage test.Stage) { +func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase test.Test, stage test.Stage) error { stageStartTime := time.Now() stageID := uuid.NewString() // Apply global overrides initially @@ -101,14 +116,14 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes // Check sanity first if checkTestSanity(testRequest) { - log.Fatal().Msgf("ftw/run: bad test: choose between data, encoded_request, or raw_request") + return errBadTestRequest } // Do not even run test if result is overridden. Just use the override and display the overridden result. if overridden := overriddenTestResult(ftwCheck, testCase.TestTitle); overridden != Failed { addResultToStats(overridden, testCase.TestTitle, &runContext.Stats) displayResult(runContext.Output, overridden, time.Duration(0), time.Duration(0)) - return + return nil } var req *ftwhttp.Request @@ -123,7 +138,7 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes if notRunningInCloudMode(ftwCheck) { startMarker, err := markAndFlush(runContext, dest, stageID) if err != nil && !expectedOutput.ExpectError { - log.Fatal().Caller().Err(err).Msg("Failed to find start marker") + return fmt.Errorf("failed to find start marker: %w", err) } ftwCheck.SetStartMarker(startMarker) } @@ -133,7 +148,7 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes err = runContext.Client.NewConnection(*dest) if err != nil && !expectedOutput.ExpectError { - log.Fatal().Caller().Err(err).Msgf("can't connect to destination %+v", dest) + return fmt.Errorf("can't connect to destination %+v: %w", dest, err) } runContext.Client.StartTrackingTime() @@ -141,13 +156,13 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes runContext.Client.StopTrackingTime() if responseErr != nil && !expectedOutput.ExpectError { - log.Fatal().Caller().Err(responseErr).Msgf("failed sending request to destination %+v", dest) + return fmt.Errorf("failed sending request to destination %+v: %w", dest, responseErr) } if notRunningInCloudMode(ftwCheck) { endMarker, err := markAndFlush(runContext, dest, stageID) if err != nil && !expectedOutput.ExpectError { - log.Fatal().Caller().Err(err).Msg("Failed to find end marker") + return fmt.Errorf("failed to find end marker: %w", err) } ftwCheck.SetEndMarker(endMarker) @@ -171,6 +186,8 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase tes runContext.Stats.Run++ runContext.Stats.RunTime += stageTime + + return nil } func markAndFlush(runContext *TestRunContext, dest *ftwhttp.Destination, stageID string) ([]byte, error) { @@ -386,6 +403,6 @@ func notRunningInCloudMode(c *check.FTWCheck) bool { func cleanLogs(logLines *waflog.FTWLogLines) { if err := logLines.Cleanup(); err != nil { - log.Fatal().Err(err).Msg("Failed to cleanup log file") + log.Error().Err(err).Msg("Failed to cleanup log file") } } diff --git a/runner/run_test.go b/runner/run_test.go index cf92d3d..f3ed92f 100644 --- a/runner/run_test.go +++ b/runner/run_test.go @@ -417,49 +417,55 @@ func TestRun(t *testing.T) { replaceDestinationInTest(&ftwTest, *dest) t.Run("show time and execute all", func(t *testing.T) { - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ ShowTime: true, Quiet: true, }) + assert.NoError(t, err) assert.Equalf(t, res.Stats.TotalFailed(), 0, "Oops, %d tests failed to run!", res.Stats.TotalFailed()) }) t.Run("be verbose and execute all", func(t *testing.T) { - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Include: regexp.MustCompile("0*"), ShowTime: true, }) + assert.NoError(t, err) assert.Equal(t, res.Stats.TotalFailed(), 0, "verbose and execute all failed") }) t.Run("don't show time and execute all", func(t *testing.T) { - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Include: regexp.MustCompile("0*"), }) + assert.NoError(t, err) assert.Equal(t, res.Stats.TotalFailed(), 0, "do not show time and execute all failed") }) t.Run("execute only test 008 but exclude all", func(t *testing.T) { - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Include: regexp.MustCompile("008"), Exclude: regexp.MustCompile("0*"), }) + assert.NoError(t, err) assert.Equal(t, res.Stats.TotalFailed(), 0, "do not show time and execute all failed") }) t.Run("exclude test 010", func(t *testing.T) { - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Exclude: regexp.MustCompile("010"), }) + assert.NoError(t, err) assert.Equal(t, res.Stats.TotalFailed(), 0, "failed to exclude test") }) t.Run("test exceptions 1", func(t *testing.T) { - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Include: regexp.MustCompile("1*"), Exclude: regexp.MustCompile("0*"), Quiet: true, }) + assert.NoError(t, err) assert.Equal(t, res.Stats.TotalFailed(), 0, "failed to test exceptions") }) } @@ -487,9 +493,10 @@ func TestOverrideRun(t *testing.T) { replaceDestinationInTest(&ftwTest, *fakeDestination) - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Quiet: true, }) + assert.NoError(t, err) assert.LessOrEqual(t, 0, res.Stats.TotalFailed(), "Oops, test run failed!") } @@ -516,9 +523,10 @@ func TestBrokenOverrideRun(t *testing.T) { replaceDestinationInTest(&ftwTest, *fakeDestination) // the test should succeed, despite the unknown override property - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Quiet: true, }) + assert.NoError(t, err) assert.LessOrEqual(t, 0, res.Stats.TotalFailed(), "Oops, test run failed!") } @@ -547,16 +555,17 @@ func TestBrokenPortOverrideRun(t *testing.T) { replaceDestinationInTest(&ftwTest, *fakeDestination) // the test should succeed, despite the unknown override property - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Quiet: true, }) + assert.NoError(t, err) assert.LessOrEqual(t, 0, res.Stats.TotalFailed(), "Oops, test run failed!") } func TestDisabledRun(t *testing.T) { t.Cleanup(config.Reset) - err := config.NewConfigFromString(yamlConfig) + err := config.NewConfigFromString(yamlCloudConfig) assert.NoError(t, err) fakeDestination, err := ftwhttp.DestinationFromString("http://example.com:1234") @@ -568,9 +577,10 @@ func TestDisabledRun(t *testing.T) { assert.NoError(t, err) replaceDestinationInTest(&ftwTest, *fakeDestination) - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Quiet: true, }) + assert.NoError(t, err) assert.LessOrEqual(t, 0, res.Stats.TotalFailed(), "Oops, test run failed!") } @@ -589,9 +599,10 @@ func TestLogsRun(t *testing.T) { assert.NoError(t, err) replaceDestinationInTest(&ftwTest, *dest) - res := Run([]test.FTWTest{ftwTest}, Config{ + res, err := Run([]test.FTWTest{ftwTest}, Config{ Quiet: true, }) + assert.NoError(t, err) assert.LessOrEqual(t, 0, res.Stats.TotalFailed(), "Oops, test run failed!") } @@ -630,12 +641,14 @@ func TestCloudRun(t *testing.T) { replaceDestinationInTest(&ftwTest, *dest) assert.NoError(t, err) + client, err := ftwhttp.NewClient(ftwhttp.NewClientConfig()) + assert.NoError(t, err) runContext := TestRunContext{ Include: nil, Exclude: nil, ShowTime: false, Output: true, - Client: ftwhttp.NewClient(ftwhttp.NewClientConfig()), + Client: client, LogLines: nil, } @@ -662,7 +675,8 @@ func TestFailedTestsRun(t *testing.T) { assert.NoError(t, err) replaceDestinationInTest(&ftwTest, *dest) - res := Run([]test.FTWTest{ftwTest}, Config{}) + res, err := Run([]test.FTWTest{ftwTest}, Config{}) + assert.NoError(t, err) assert.Equal(t, 1, res.Stats.TotalFailed()) } @@ -709,6 +723,7 @@ func TestIgnoredTestsRun(t *testing.T) { replaceDestinationInTest(&ftwTest, *dest) - res := Run([]test.FTWTest{ftwTest}, Config{}) + res, err := Run([]test.FTWTest{ftwTest}, Config{}) + assert.NoError(t, err) assert.Equal(t, res.Stats.TotalFailed(), 1, "Oops, test run failed!") } diff --git a/waflog/read.go b/waflog/read.go index 91186cc..cb5f0eb 100644 --- a/waflog/read.go +++ b/waflog/read.go @@ -83,9 +83,6 @@ func (ll *FTWLogLines) getMarkedLines() [][]byte { // logFile is the file to search // stageID is the ID of the current stage, which is part of the marker line func (ll *FTWLogLines) CheckLogForMarker(stageID string) []byte { - if config.FTWConfig.RunMode == config.DefaultRunMode && ll.logFile == nil { - log.Fatal().Caller().Msg("No log file supplied") - } offset, err := ll.logFile.Seek(0, os.SEEK_END) if err != nil { log.Error().Caller().Err(err).Msgf("failed to seek end of log file") diff --git a/waflog/read_test.go b/waflog/read_test.go index 3a6657c..d5d33c4 100644 --- a/waflog/read_test.go +++ b/waflog/read_test.go @@ -31,7 +31,8 @@ func TestReadCheckLogForMarkerNoMarkerAtEnd(t *testing.T) { config.FTWConfig.LogFile = filename t.Cleanup(func() { os.Remove(filename) }) - ll := NewFTWLogLines(WithStartMarker(bytes.ToLower([]byte(markerLine)))) + ll, err := NewFTWLogLines(WithStartMarker(bytes.ToLower([]byte(markerLine)))) + assert.NoError(t, err) marker := ll.CheckLogForMarker(stageID) assert.Nil(t, marker, "unexpectedly found marker") @@ -54,7 +55,8 @@ func TestReadCheckLogForMarkerWithMarkerAtEnd(t *testing.T) { config.FTWConfig.LogFile = filename t.Cleanup(func() { os.Remove(filename) }) - ll := NewFTWLogLines(WithStartMarker(bytes.ToLower([]byte(markerLine)))) + ll, err := NewFTWLogLines(WithStartMarker(bytes.ToLower([]byte(markerLine)))) + assert.NoError(t, err) marker := ll.CheckLogForMarker(stageID) assert.NotNil(t, marker, "no marker found") @@ -80,9 +82,10 @@ func TestReadGetMarkedLines(t *testing.T) { config.FTWConfig.LogFile = filename t.Cleanup(func() { os.Remove(filename) }) - ll := NewFTWLogLines( + ll, err := NewFTWLogLines( WithStartMarker(bytes.ToLower([]byte(startMarkerLine))), WithEndMarker(bytes.ToLower([]byte(endMarkerLine)))) + assert.NoError(t, err) foundLines := ll.getMarkedLines() // logs are scanned backwards @@ -116,9 +119,10 @@ func TestReadGetMarkedLinesWithTrailingEmptyLines(t *testing.T) { config.FTWConfig.LogFile = filename t.Cleanup(func() { os.Remove(filename) }) - ll := NewFTWLogLines( + ll, err := NewFTWLogLines( WithStartMarker(bytes.ToLower([]byte(startMarkerLine))), WithEndMarker(bytes.ToLower([]byte(endMarkerLine)))) + assert.NoError(t, err) foundLines := ll.getMarkedLines() // logs are scanned backwards @@ -155,9 +159,10 @@ func TestReadGetMarkedLinesWithPrecedingLines(t *testing.T) { config.FTWConfig.LogFile = filename t.Cleanup(func() { os.Remove(filename) }) - ll := NewFTWLogLines( + ll, err := NewFTWLogLines( WithStartMarker(bytes.ToLower([]byte(startMarkerLine))), WithEndMarker(bytes.ToLower([]byte(endMarkerLine)))) + assert.NoError(t, err) foundLines := ll.getMarkedLines() // logs are scanned backwards diff --git a/waflog/waflog.go b/waflog/waflog.go index 7be09e1..da9b5f5 100644 --- a/waflog/waflog.go +++ b/waflog/waflog.go @@ -1,15 +1,17 @@ package waflog import ( + "errors" + "fmt" "os" - "github.com/rs/zerolog/log" - "github.com/coreruleset/go-ftw/config" ) +var errNoLogFile = errors.New("no log file supplied") + // NewFTWLogLines is the base struct for reading the log file -func NewFTWLogLines(opts ...FTWLogOption) *FTWLogLines { +func NewFTWLogLines(opts ...FTWLogOption) (*FTWLogLines, error) { ll := &FTWLogLines{ logFile: nil, FileName: config.FTWConfig.LogFile, @@ -25,10 +27,14 @@ func NewFTWLogLines(opts ...FTWLogOption) *FTWLogLines { } if err := ll.openLogFile(); err != nil { - log.Error().Caller().Msgf("cannot open log file: %s", err) + return nil, fmt.Errorf("cannot open log file: %w", err) + } + + if config.FTWConfig.RunMode == config.DefaultRunMode && ll.logFile == nil { + return nil, errNoLogFile } - return ll + return ll, nil } // WithStartMarker sets the start marker for the log file diff --git a/waflog/waflog_test.go b/waflog/waflog_test.go index 6e7979e..9677d80 100644 --- a/waflog/waflog_test.go +++ b/waflog/waflog_test.go @@ -12,7 +12,8 @@ func TestNewFTWLogLines(t *testing.T) { err := config.NewConfigFromEnv() assert.NoError(t, err) - ll := NewFTWLogLines() + // Don't call NewFTWLogLines to avoid opening the file. + ll := &FTWLogLines{} // Loop through each option for _, opt := range []FTWLogOption{ WithStartMarker([]byte("#")),