From ba3e2eaa6f8cc5372c750ec6595ab68d6b612c31 Mon Sep 17 00:00:00 2001 From: beastieboy Date: Sun, 13 Nov 2022 10:10:04 +0000 Subject: [PATCH] Improve test coverage, fix various error handling bugs (#40) - [x] Bring test coverage back up to around 90%. - [x] Fix various bugs in error handling - [x] Remove panic behaviour when templates can't be found Closes #39 Co-authored-by: Nicolas Herry Reviewed-on: https://www.beastieboy.net/git/beastieboy/marmotte/pulls/40 --- gopher/dynamic.go | 16 +++--- gopher/dynamic_test.go | 98 +++++++++++++++++++++++++++++++++++++ gopher/errorcodes.go | 8 --- gopher/files_test.go | 26 ++++++++++ gopher/transformers_test.go | 64 +++++++++++++++++++++++- testdata/attic/broken.gt | 1 + testdata/attic/hello.gt | 3 +- testdata/errors-empty/.keep | 0 8 files changed, 197 insertions(+), 19 deletions(-) create mode 100644 gopher/dynamic_test.go create mode 100644 testdata/attic/broken.gt create mode 100644 testdata/errors-empty/.keep diff --git a/gopher/dynamic.go b/gopher/dynamic.go index 9e4f762..c5390f9 100644 --- a/gopher/dynamic.go +++ b/gopher/dynamic.go @@ -20,23 +20,23 @@ type DynamicContext struct { } func RunTemplate(dc *DynamicContext, p string) ([]string, error) { - var err error + // read the template found in the FullPath - t := template.Must(template.ParseFiles(p)) - if t != nil { + if t, err := template.ParseFiles(p); err != nil { + return []string{}, err + } else { var buf bytes.Buffer - if err := t.Execute(io.Writer(&buf), dc); err == nil { - return []string{buf.String()}, err - } else { + if err = t.Execute(io.Writer(&buf), dc); err != nil { log.Error(). Err(err). Str("Template Path", p). Msg("Could not generate dynamic content via template") + return []string{}, err + } else { + return []string{buf.String()}, err } } - - return []string{}, err } diff --git a/gopher/dynamic_test.go b/gopher/dynamic_test.go new file mode 100644 index 0000000..3e446c8 --- /dev/null +++ b/gopher/dynamic_test.go @@ -0,0 +1,98 @@ +package gopher + + +import ( + "testing" + "os" + "time" + "bytes" + "text/template" + + "github.com/google/go-cmp/cmp" +) + +func TestRunTemplate(t *testing.T) { + d, _ := os.Getwd() + c := Context { Root: d + "/../testdata" } + + p := "/attic/hello.gt" + + rq, _ := NewRequest(c, p) + rs := NewResponse(c, *rq) + + dc := DynamicContext { + ServerContext: c, + RequestInfo: *rq, + ResponseInfo: *rs, + ErrorInfo: GopherErrorInfo{}, + CurrentTime: time.Now(), + } + + lines, _ := os.ReadFile(rq.FullPath) + + tmpl, err := template.New("test").Parse(string(lines)) + var b bytes.Buffer + err = tmpl.Execute(&b, dc) + expected := []string{b.String()} + + got, err := RunTemplate(&dc, rq.FullPath); + + if err != nil { + t.Errorf("Received an unexpected error when running a template with path %s; %s", p, err.Error()) + } + + if ! cmp.Equal(got, expected) { + t.Errorf("Unexpected return value when generating a template: %s", cmp.Diff(expected, got)) + } + + p = "/attic/broken.gt" + + rq, _ = NewRequest(c, p) + rs = NewResponse(c, *rq) + + dc = DynamicContext { + ServerContext: c, + RequestInfo: *rq, + ResponseInfo: *rs, + ErrorInfo: GopherErrorInfo{}, + CurrentTime: time.Now(), + } + + expected = []string{} + + got, err = RunTemplate(&dc, rq.FullPath); + + if err == nil { + t.Errorf("Failed to received an error when running a broken template with path %s", p) + } + + if ! cmp.Equal(got, expected) { + t.Errorf("Unexpected return value when generating a broken template: %s", cmp.Diff(expected, got)) + } + + p = "/attic/non-existent.gt" + + rq, _ = NewRequest(c, p) + rs = NewResponse(c, *rq) + + dc = DynamicContext { + ServerContext: c, + RequestInfo: *rq, + ResponseInfo: *rs, + ErrorInfo: GopherErrorInfo{}, + CurrentTime: time.Now(), + } + + expected = []string{} + + got, err = RunTemplate(&dc, rq.FullPath); + + if err == nil { + t.Errorf("Failed to received an error when running a non-existent template with path %s", p) + } + + if ! cmp.Equal(got, expected) { + t.Errorf("Unexpected return value when generating a non-existent template: %s", cmp.Diff(expected, got)) + } + +} diff --git a/gopher/errorcodes.go b/gopher/errorcodes.go index 036612c..5e22088 100644 --- a/gopher/errorcodes.go +++ b/gopher/errorcodes.go @@ -1,9 +1,5 @@ package gopher -import ( - "fmt" -) - type GopherErrorCode string const GopherErrorNoError GopherErrorCode = "00" @@ -11,10 +7,6 @@ const GopherErrorSelectorNotFound GopherErrorCode = "10" const GopherErrorWrongPermissions GopherErrorCode = "20" const GopherErrorDataRetrieval GopherErrorCode = "30" -func (e GopherErrorCode) String() string { - return fmt.Sprint(string(e)) -} - type GopherErrorInfo struct { Selector string } diff --git a/gopher/files_test.go b/gopher/files_test.go index 71b21e4..522371c 100644 --- a/gopher/files_test.go +++ b/gopher/files_test.go @@ -5,6 +5,8 @@ import ( "os" "strings" "testing" + + "github.com/google/go-cmp/cmp" ) func TestFileType(t *testing.T) { @@ -115,3 +117,27 @@ func TestGopherMapFixLine(t *testing.T) { } } + +func TestGopherMapFixLines(t *testing.T) { + d, _ := os.Getwd() + c := Context { Host: "myhost", Port: 7777, Root: d + "/../testdata" } + + l1 := "An info line with no tab\r\n" + expected1 := "i" + strings.TrimRight(l1, "\r\n") + "\tfake.host\t1\r\n" + + l2 := "An info line\twith\ttabs\r\n" + expected2 := l2 + + l3 := "0A menu line with no server and no port\tfile.txt\r\n" + expected3 := strings.TrimRight(l3, "\r\n") + "\t" + c.Host + "\t" + fmt.Sprintf("%d", c.Port) + "\r\n" + + l4 := "0A menu line with a server and a port\tfile.txt\tsomeserver.com\t70\r\n" + expected4 := l4 + + got := GopherMapFixLines(c, []string{ l1, l2, l3, l4}) + expected := []string { expected1, expected2, expected3, expected4 } + + if ! cmp.Equal(expected, got) { + t.Errorf("Unexpected info lines from GopherMapFixLines: %s", cmp.Diff(expected, got)) + } +} diff --git a/gopher/transformers_test.go b/gopher/transformers_test.go index f89ebcd..458c10c 100644 --- a/gopher/transformers_test.go +++ b/gopher/transformers_test.go @@ -4,6 +4,7 @@ import ( "testing" "os" "path/filepath" + "fmt" "github.com/google/go-cmp/cmp" ) @@ -239,7 +240,7 @@ func TestSelectorRewriteTransformer(t *testing.T) { func TestExecutableTransformer(t *testing.T) { d, _ := os.Getwd() - c := Context { Root: d + "/../testdata", Footers: map[string][]string{"default": {"powered", "by", "marmotte"}} } + c := Context { Root: d + "/../testdata", Footers: map[string][]string{"default": {"powered", "by", "marmotte"}}, ErrorRoot: d + "/../testdata/errors" } p:= "/createtextfile.sh" rq, _ := NewRequest(c, p) _, newRq, rs, err := ExecutableTransformer(&c, *rq, &Response{}, nil) @@ -254,4 +255,65 @@ func TestExecutableTransformer(t *testing.T) { if int(got) != expected { t.Errorf("Unexpected Response type for a file created by an executable: %s, expected: %d, got: %d", newRq.FullPath, expected, got) } + + p = "/bin/ls" + rq, _ = NewRequest(c, p) + // overwrite rq.FullPath with an absolute path + rq.FullPath = "/bin/ls" + _, newRq, rs, err = ExecutableTransformer(&c, *rq, &Response{}, nil) + + // This should redirect to an error handling function, which should not fail + if err != nil { + t.Errorf("Unexpected error received when running an ExecutableTransformer for %s: %s", p, err.Error()) + } + + // The selector should become not found + expectedCode := GopherErrorSelectorNotFound + gotCode := rs.ErrorCode + + if gotCode != expectedCode { + t.Errorf("Failed to receive the expected error code in the response when running an ExecutableTransformer for %s, expected %s, got %s", p, expectedCode, gotCode) + } + +} + +func TestErrorRedirectTransformer(t *testing.T) { + d, _ := os.Getwd() + c := Context { Root: d + "/../testdata", Footers: map[string][]string{"default": {"powered", "by", "marmotte"}}, ErrorRoot: d + "/../testdata/errors-empty" } + p:= "/non-existent" + rq, _ := NewRequest(c, p) + // error in rendering pipeline, e.g. in data retrieval + rs := &Response{ErrorCode: GopherErrorDataRetrieval, ContentText: []string{"Some text retrieved but..."}} + + _, _, rs, err := ErrorRedirectTransformer(&c, *rq, rs, nil) + + if err == nil { + t.Errorf("Failed to receive an error when running an ErrorRedirectTransformer with a missing error template for %s: %s", p, err.Error()) + } + + expected := []string {fmt.Sprint("Error retrieving the error page for the request. The original error code was ", rs.ErrorCode, " for the selector ", rq.Path)} + + if ! cmp.Equal(expected, rs.ContentText) { + t.Errorf("Received unexpected ContentText is Reponse after an ErrorRedirectTransformer with a missing error template for %s: %s", p, cmp.Diff(expected, rs.ContentText)) + } + +} + +func TestDynamicRenderingTransformer(t *testing.T) { + d, _ := os.Getwd() + c := Context { Root: d + "/../testdata", Footers: map[string][]string{"default": {"powered", "by", "marmotte"}}, ErrorRoot: d + "/../testdata/errors" } + p:= "/attic/hello.gt" + rq, _ := NewRequest(c, p) + + rs := NewResponse(c, *rq) + + _, _, rs, err := DynamicRenderingTransformer(&c, *rq, rs, nil) + + if err != nil { + t.Errorf("Unexpected error received when running a DynamicRenderingTransformer for a valid template %s: %s", p, err.Error()) + } + + if len(rs.ContentText) == 0 { + t.Errorf("Unexpected empty ContentText in Response when running a DynamicRenderingTransformer for a valid template %s", p) + } } diff --git a/testdata/attic/broken.gt b/testdata/attic/broken.gt new file mode 100644 index 0000000..fc079d1 --- /dev/null +++ b/testdata/attic/broken.gt @@ -0,0 +1 @@ +This template is broken and should generate an error. {{ .NonExistentField }} \ No newline at end of file diff --git a/testdata/attic/hello.gt b/testdata/attic/hello.gt index 229000b..bb5a4b0 100644 --- a/testdata/attic/hello.gt +++ b/testdata/attic/hello.gt @@ -13,5 +13,4 @@ Current Configuration: - FullPath: {{- .RequestInfo.FullPath }} - Path: {{- .RequestInfo.Path }} - Additional parameters - - Current time: {{- .CurrentTime }} - \ No newline at end of file + - Current time: {{- .CurrentTime }} \ No newline at end of file diff --git a/testdata/errors-empty/.keep b/testdata/errors-empty/.keep new file mode 100644 index 0000000..e69de29