From a1abbc55a2962cf26884599ffe7e23ac49d346c4 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Wed, 11 Apr 2018 13:30:02 +0200 Subject: [PATCH] refactoring all tests but handlers and process --- app/config/config_test.go | 26 +++++------ app/context/context_test.go | 30 ++++++------- app/converter/converter_test.go | 76 +++++++++++++++++++-------------- app/converter/file/file_test.go | 11 +++-- app/handlers.go | 1 + app/http/http_test.go | 10 ++--- 6 files changed, 84 insertions(+), 70 deletions(-) diff --git a/app/config/config_test.go b/app/config/config_test.go index 4d44e73c..a30df9b1 100644 --- a/app/config/config_test.go +++ b/app/config/config_test.go @@ -6,70 +6,72 @@ import ( ) func TestNewAppConfig(t *testing.T) { + var path string + // case 1: uses an empty configuration file path. if _, err := NewAppConfig(""); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Error("AppConfig should not have been instantiated by using an empty configuration file path") } // case 2: uses a broken configuration file. - path, _ := filepath.Abs("../../_tests/configurations/broken-gotenberg.yml") + path, _ = filepath.Abs("../../_tests/configurations/broken-gotenberg.yml") if _, err := NewAppConfig(path); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Errorf("AppConfig should not have been instantiated with '%s'", path) } // case 3: uses a configuration file with a wrong logging level. path, _ = filepath.Abs("../../_tests/configurations/wrong-logging-level-gotenberg.yml") if _, err := NewAppConfig(path); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Errorf("AppConfig should not have been instantiated with '%s'", path) } // case 4: uses a configuration file with a wrong logging format. path, _ = filepath.Abs("../../_tests/configurations/wrong-logging-format-gotenberg.yml") if _, err := NewAppConfig(path); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Errorf("AppConfig should not have been instantiated with '%s'", path) } // case 5: uses a configuration file with a wrong markdown command template. path, _ = filepath.Abs("../../_tests/configurations/wrong-markdown-command-template-gotenberg.yml") if _, err := NewAppConfig(path); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Errorf("AppConfig should not have been instantiated with '%s'", path) } // case 6: uses a configuration file with a wrong HTML command template. path, _ = filepath.Abs("../../_tests/configurations/wrong-html-command-template-gotenberg.yml") if _, err := NewAppConfig(path); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Errorf("AppConfig should not have been instantiated with '%s'", path) } // case 7: uses a configuration file with a wrong Office command template. path, _ = filepath.Abs("../../_tests/configurations/wrong-office-command-template-gotenberg.yml") if _, err := NewAppConfig(path); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Errorf("AppConfig should not have been instantiated with '%s'", path) } // case 8: uses a configuration file with a wrong merge command template. path, _ = filepath.Abs("../../_tests/configurations/wrong-merge-command-template-gotenberg.yml") if _, err := NewAppConfig(path); err == nil { - t.Error("AppConfig should not have been instantiated!") + t.Errorf("AppConfig should not have been instantiated with '%s'", path) } // case 9: uses a correct configuration file. path, _ = filepath.Abs("../../_tests/configurations/gotenberg.yml") if _, err := NewAppConfig(path); err != nil { - t.Error("AppConfig should have been instantiated!") + t.Errorf("AppConfig should have been instantiated with '%s'", path) } } func TestWrongLoggingLevelError(t *testing.T) { err := &wrongLoggingLevelError{} if err.Error() != wrongLoggingLevelErrorMessage { - t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), wrongLoggingLevelErrorMessage) + t.Errorf("Error returned a wrong message: got '%s' want '%s'", err.Error(), wrongLoggingLevelErrorMessage) } } func TestWrongLoggingFormatError(t *testing.T) { err := &wrongLoggingFormatError{} if err.Error() != wrongLoggingFormatErrorMessage { - t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), wrongLoggingFormatErrorMessage) + t.Errorf("Error returned a wrong message: got '%s' want '%s'", err.Error(), wrongLoggingFormatErrorMessage) } } diff --git a/app/context/context_test.go b/app/context/context_test.go index 3d5a60b1..17def1dd 100644 --- a/app/context/context_test.go +++ b/app/context/context_test.go @@ -11,7 +11,7 @@ import ( func TestWithConverter(t *testing.T) { req := WithConverter(httptest.NewRequest(http.MethodPost, "/", nil), &converter.Converter{}) if c, _ := req.Context().Value(converterKey).(*converter.Converter); c == nil { - t.Errorf("Context returned a wrong converter: got %v want not nil", c) + t.Errorf("Context returned a wrong converter: got '%v' want not nil", c) } } @@ -20,13 +20,21 @@ func TestGetConverter(t *testing.T) { // case 1: uses a request without a converter entry in its context. if _, err := GetConverter(req); err == nil { - t.Error("Context should not have a converter entry!") + t.Error("Context should not have a converter entry") } // case 2: uses a request with a converter entry in its context. req = WithConverter(req, &converter.Converter{}) if _, err := GetConverter(req); err != nil { - t.Error("Context should have a converter entry!") + t.Error("Context should have a converter entry") + } +} + +func TestWithResultFilePath(t *testing.T) { + filePath := "file.pdf" + req := WithResultFilePath(httptest.NewRequest(http.MethodPost, "/", nil), filePath) + if path, _ := req.Context().Value(resultFilePathKey).(string); path != filePath { + t.Errorf("Context returned a wrong result file path: got '%s' want '%s'", path, filePath) } } @@ -35,34 +43,26 @@ func TestGetResultFilePath(t *testing.T) { // case 1: uses a request without a result file path entry in its context. if _, err := GetResultFilePath(req); err == nil { - t.Error("Context should not have a result file path entry!") + t.Error("Context should not have a result file path entry") } // case 2: uses a request with a result file path entry in its context. req = WithResultFilePath(req, "file.pdf") if _, err := GetResultFilePath(req); err != nil { - t.Error("Context should have a converter entry!") - } -} - -func TestWithResultFilePath(t *testing.T) { - filePath := "file.pdf" - req := WithResultFilePath(httptest.NewRequest(http.MethodPost, "/", nil), filePath) - if path, _ := req.Context().Value(resultFilePathKey).(string); path != filePath { - t.Errorf("Context returned a wrong converter: got %s want %s", path, filePath) + t.Error("Context should have a result file path entry") } } func TestConverterNotFoundError(t *testing.T) { err := &converterNotFoundError{} if err.Error() != converterNotFoundErrorMessage { - t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), converterNotFoundErrorMessage) + t.Errorf("Error returned a wrong message: got '%s' want '%s'", err.Error(), converterNotFoundErrorMessage) } } func TestResultFilePathNotFoundError(t *testing.T) { err := &resultFilePathNotFoundError{} if err.Error() != resultFilePathNotFoundErrorMessage { - t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), resultFilePathNotFoundErrorMessage) + t.Errorf("Error returned a wrong message: got '%s' want '%s'", err.Error(), resultFilePathNotFoundErrorMessage) } } diff --git a/app/converter/converter_test.go b/app/converter/converter_test.go index 3ee44214..18697c86 100644 --- a/app/converter/converter_test.go +++ b/app/converter/converter_test.go @@ -43,78 +43,91 @@ func makeRequest(filesPaths ...string) *http.Request { return req } +func loadCommandConfigs(configurationFilePath string) { + path, _ := filepath.Abs(configurationFilePath) + c, _ := config.NewAppConfig(path) + process.Load(c.CommandsConfig) +} + func TestNewConverter(t *testing.T) { + var ( + path string + oPath string + ) + // case 1: uses a request with a single file. - path, _ := filepath.Abs("../../_tests/file.docx") + path, _ = filepath.Abs("../../_tests/file.docx") if _, err := NewConverter(makeRequest(path)); err != nil { - t.Error("Converter should have been instantiated!") + t.Errorf("Converter should have been instantiated with '%s'", path) } // case 2: uses a request with wrong file type. path, _ = filepath.Abs("../../_tests/configurations/gotenberg.yml") if _, err := NewConverter(makeRequest(path)); err == nil { - t.Error("Converter should not have been instantiated!") + t.Errorf("Converter should not have been instantiated with '%s'", path) } // case 3: uses a request with two files. - oPath, _ := filepath.Abs("../../_tests/file.docx") path, _ = filepath.Abs("../../_tests/file.pdf") - if _, err := NewConverter(makeRequest(oPath, path)); err != nil { - t.Error("Converter should have been instantiated!") + oPath, _ = filepath.Abs("../../_tests/file.docx") + if _, err := NewConverter(makeRequest(path, oPath)); err != nil { + t.Errorf("Converter should have been instantiated with '%s' and '%s'", path, oPath) } - // case 4: uses a request with one Office file and one wrong file type. - oPath, _ = filepath.Abs("../../_tests/file.docx") + // case 4: uses a request with one Office file type and one wrong file type. path, _ = filepath.Abs("../../_tests/configurations/gotenberg.yml") - if _, err := NewConverter(makeRequest(oPath, path)); err == nil { - t.Error("Converter should not have been instantiated!") + oPath, _ = filepath.Abs("../../_tests/file.docx") + if _, err := NewConverter(makeRequest(path, oPath)); err == nil { + t.Errorf("Converter should not have been instantiated with '%s' and '%s'", path, oPath) } // case 5: uses a request with no file. if _, err := NewConverter(makeRequest()); err == nil { - t.Error("Converter should not have been instantiated!") + t.Error("Converter should not have been instantiated with no file") } } func TestConvert(t *testing.T) { - path, _ := filepath.Abs("../../_tests/configurations/gotenberg.yml") - appConfig, _ := config.NewAppConfig(path) - process.Load(appConfig.CommandsConfig) + var ( + path string + oPath string + c *Converter + ) + + loadCommandConfigs("../../_tests/configurations/gotenberg.yml") // case 1: uses a request with a single file. path, _ = filepath.Abs("../../_tests/file.docx") - c, _ := NewConverter(makeRequest(path)) + c, _ = NewConverter(makeRequest(path)) if _, err := c.Convert(); err != nil { - t.Error("Converter should have been able to convert an Office document to PDF!") + t.Errorf("Converter should have been able to convert '%s' to PDF", path) } // case 2: uses a request with two files. - oPath, _ := filepath.Abs("../../_tests/file.docx") path, _ = filepath.Abs("../../_tests/file.pdf") - c, _ = NewConverter(makeRequest(oPath, path)) + oPath, _ = filepath.Abs("../../_tests/file.docx") + c, _ = NewConverter(makeRequest(path, oPath)) if _, err := c.Convert(); err != nil { - t.Error("Converter should have been able to convert two files to PDF and merge them!") + t.Errorf("Converter should have been able to convert '%s' and '%s' to PDF", path, oPath) } + loadCommandConfigs("../../_tests/configurations/timeout-gotenberg.yml") + // case 3: uses a request with a single file and a configuration with an unsuitable timeout for the conversion commands. - path, _ = filepath.Abs("../../_tests/configurations/timeout-gotenberg.yml") - appConfig, _ = config.NewAppConfig(path) - process.Load(appConfig.CommandsConfig) path, _ = filepath.Abs("../../_tests/file.docx") c, _ = NewConverter(makeRequest(path)) if _, err := c.Convert(); err == nil { - t.Error("Converter should not have been able to convert an Office document to PDF!") + t.Errorf("Converter should not have been able to convert '%s' to PDF", path) } + loadCommandConfigs("../../_tests/configurations/merge-timeout-gotenberg.yml") + // case 4: uses a request with two files and a configuration with an unsuitable timeout for the merge command. - path, _ = filepath.Abs("../../_tests/configurations/merge-timeout-gotenberg.yml") - appConfig, _ = config.NewAppConfig(path) - process.Load(appConfig.CommandsConfig) - oPath, _ = filepath.Abs("../../_tests/file.docx") path, _ = filepath.Abs("../../_tests/file.pdf") - c, _ = NewConverter(makeRequest(oPath, path)) + oPath, _ = filepath.Abs("../../_tests/file.docx") + c, _ = NewConverter(makeRequest(path, oPath)) if _, err := c.Convert(); err == nil { - t.Error("Converter should not have been able to merge PDF!") + t.Errorf("Converter should not have been able to merge '%s' and '%s' into PDF", path, oPath) } } @@ -122,14 +135,13 @@ func TestClear(t *testing.T) { path, _ := filepath.Abs("../../_tests/file.docx") c, _ := NewConverter(makeRequest(path)) if err := c.Clear(); err != nil { - t.Error("Converter should have been able to clear itself!") + t.Error("Converter should have been able to clear itself") } } func TestNoFileToConvertError(t *testing.T) { err := &noFileToConvertError{} - if err.Error() != noFileToConvertErrorMessage { - t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), noFileToConvertErrorMessage) + t.Errorf("Error returned a wrong message: got '%s' want '%s'", err.Error(), noFileToConvertErrorMessage) } } diff --git a/app/converter/file/file_test.go b/app/converter/file/file_test.go index 36d8943b..5323c4e2 100644 --- a/app/converter/file/file_test.go +++ b/app/converter/file/file_test.go @@ -12,17 +12,17 @@ func TestNewFile(t *testing.T) { workingDir := "test" os.Mkdir(workingDir, 0666) - // case 2: uses a wrong file name. + // case 1: uses a wrong file name. if _, err := NewFile(workingDir, new(bytes.Buffer), "file.yml"); err == nil { - t.Error("File should not have been instantiated!") + t.Error("File should not have been instantiated with an empty buffer") } - // case 3: uses a reader from a correct file type. + // case 2: uses a reader from a correct file type. filePath, _ := filepath.Abs("../../../_tests/file.pdf") r, _ := os.Open(filePath) defer r.Close() if _, err := NewFile(workingDir, r, "file.pdf"); err != nil { - t.Error("File should have been instantiated!") + t.Errorf("File should have been instantiated using a reader from '%s'", filePath) } os.RemoveAll(workingDir) @@ -31,8 +31,7 @@ func TestFileTypeNotFoundError(t *testing.T) { fileName := "file.wp" err := &fileTypeNotFoundError{fileName: fileName} expected := fmt.Sprintf("File type was not found for '%s'", fileName) - if err.Error() != expected { - t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), expected) + t.Errorf("Error returned a wrong message: got '%s' want '%s'", err.Error(), expected) } } diff --git a/app/handlers.go b/app/handlers.go index 29245cef..61534c60 100644 --- a/app/handlers.go +++ b/app/handlers.go @@ -60,6 +60,7 @@ func convertHandler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c, err := converter.NewConverter(r) if err != nil { + // TODO bad request if no file to convert http.Error(w, err.Error(), http.StatusInternalServerError) logger.Error(err) diff --git a/app/http/http_test.go b/app/http/http_test.go index 958c227e..e5bdbd3a 100644 --- a/app/http/http_test.go +++ b/app/http/http_test.go @@ -12,20 +12,20 @@ func TestCheckAuthorizedContentType(t *testing.T) { // case 1: uses a request without a content type entry in its header. if err := CheckAuthorizedContentType(req.Header); err == nil { - t.Error("It should not have been able to retrieve an authorized content type from header!") + t.Error("Function should not have been able to retrieve an authorized content type from request's header") } // case 2: uses a request with a content type entry in its header. req.Header.Set("Content-Type", string(MultipartFormDataContentType)) if err := CheckAuthorizedContentType(req.Header); err != nil { - t.Error("It should have been able to retrieve an authorized content type from header!") + t.Error("Function should have been able to retrieve an authorized content type from request's header") } } func TestNotAuthorizedContentTypeError(t *testing.T) { err := ¬AuthorizedContentTypeError{} - message := fmt.Sprintf("Accepted value for 'Content-Type': %s", MultipartFormDataContentType) - if err.Error() != message { - t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), message) + expected := fmt.Sprintf("Accepted value for 'Content-Type': %s", MultipartFormDataContentType) + if err.Error() != expected { + t.Errorf("Error returned a wrong message: got '%s' want '%s'", err.Error(), expected) } }