diff --git a/.gitignore b/.gitignore index 2694913c..9a59743d 100755 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ coverage-all.out # File created for debugging with vscode mage_output_file.go +TODO diff --git a/dev/mage/mageutils.go b/dev/mage/mageutils.go index c0925351..902318cc 100644 --- a/dev/mage/mageutils.go +++ b/dev/mage/mageutils.go @@ -131,8 +131,7 @@ func InstallVSCodeModules() error { } if err := InstallGoDeps(vscodeDeps); err != nil { - return fmt.Errorf( - color.RedString("failed to install vscode-go dependencies: %v", err)) + return fmt.Errorf("failed to install vscode-go dependencies: %w", err) } return nil diff --git a/docs/docGeneration.go b/docs/docGeneration.go index 2df103b4..7c639e0c 100644 --- a/docs/docGeneration.go +++ b/docs/docGeneration.go @@ -5,9 +5,9 @@ import ( "bytes" "fmt" "go/ast" - "go/parser" "go/printer" "go/token" + "go/types" "os" "path/filepath" "sort" @@ -16,6 +16,7 @@ import ( gitutils "github.com/l50/goutils/v2/git" "github.com/spf13/afero" + "golang.org/x/tools/go/packages" ) // PackageDoc holds the documentation for a Go package. @@ -148,7 +149,11 @@ func generateReadmeFromTemplate(fs afero.Fs, pkgDoc *PackageDoc, path string, te repoRoot, err := gitutils.RepoRoot() if err != nil { - return fmt.Errorf("error finding repo root: %w", err) + // Fallback to current working directory if not in a git repo + repoRoot, err = os.Getwd() + if err != nil { + return fmt.Errorf("error getting current working directory: %w", err) + } } rootReadmePath := filepath.Join(repoRoot, "README.md") @@ -293,41 +298,51 @@ func directoryContainsGoFiles(fs afero.Fs, path string) (bool, error) { } func processGoFiles(fs afero.Fs, path string, repo Repo, tmplPath string, excludedPackagesMap map[string]struct{}) error { - fset := token.NewFileSet() + cfg := &packages.Config{ + Mode: packages.NeedName | packages.NeedSyntax | packages.NeedTypes | packages.NeedTypesInfo, + Dir: path, + Fset: token.NewFileSet(), + Tests: false, + } - // Create a temporary directory - tempDir, err := os.MkdirTemp("", "parserTemp") + // Use nonTestFilter to exclude test files + files, err := afero.ReadDir(fs, path) if err != nil { - return err + return fmt.Errorf("error reading directory: %w", err) } - defer os.RemoveAll(tempDir) // Ensure cleanup - // Copy files from afero to the temporary directory - aferoFiles, _ := afero.ReadDir(fs, path) - for _, file := range aferoFiles { - data, _ := afero.ReadFile(fs, filepath.Join(path, file.Name())) - if err := os.WriteFile(filepath.Join(tempDir, file.Name()), data, file.Mode()); err != nil { - return err + var goFiles []string + for _, file := range files { + if nonTestFilter(file) && strings.HasSuffix(file.Name(), ".go") { + goFiles = append(goFiles, filepath.Join(path, file.Name())) + } + } + + cfg.Overlay = make(map[string][]byte) + for _, file := range goFiles { + content, err := afero.ReadFile(fs, file) + if err != nil { + return fmt.Errorf("error reading file %s: %w", file, err) } + cfg.Overlay[file] = content } - // Point the parser to the temporary directory - pkgs, err := parser.ParseDir(fset, tempDir, nonTestFilter, parser.ParseComments) + // Use "." to represent the current package + patterns := []string{"."} + + pkgs, err := packages.Load(cfg, patterns...) if err != nil { - return err + return fmt.Errorf("error loading packages: %w", err) } for _, pkg := range pkgs { - if filepath.Base(path) == "magefiles" && pkg.Name == "main" { - // treat magefiles as a separate package for documentation - pkg.Name = "magefiles" - } - - // check if the package name is in the excluded packages list + // Ensure the package is not excluded if _, exists := excludedPackagesMap[pkg.Name]; exists { - continue // if so, skip this package + continue } - if err := generateReadmeForPackage(fs, path, fset, pkg, repo, tmplPath); err != nil { + + // Generate the README for the package + if err := generateReadmeForPackage(fs, path, pkg, repo, tmplPath); err != nil { return err } } @@ -339,15 +354,15 @@ func nonTestFilter(info os.FileInfo) bool { return !strings.HasSuffix(info.Name(), "_test.go") } -func generateReadmeForPackage(fs afero.Fs, path string, fset *token.FileSet, pkg *ast.Package, repo Repo, templatePath string) error { +func generateReadmeForPackage(fs afero.Fs, path string, pkg *packages.Package, repo Repo, templatePath string) error { pkgDoc := &PackageDoc{ PackageName: pkg.Name, GoGetPath: fmt.Sprintf("github.com/%s/%s/%s", repo.Owner, repo.Name, pkg.Name), Functions: []FunctionDoc{}, } - for _, file := range pkg.Files { - err := processFileDeclarations(fset, pkgDoc, file) + for _, file := range pkg.Syntax { + err := processFileDeclarations(pkg.Fset, pkgDoc, file, pkg.TypesInfo) if err != nil { return err } @@ -359,38 +374,36 @@ func generateReadmeForPackage(fs afero.Fs, path string, fset *token.FileSet, pkg }) return generateReadmeFromTemplate(fs, pkgDoc, filepath.Join(path, "README.md"), templatePath) - } -func processFileDeclarations(fset *token.FileSet, pkgDoc *PackageDoc, file *ast.File) error { +func processFileDeclarations(fset *token.FileSet, pkgDoc *PackageDoc, file *ast.File, info *types.Info) error { for _, decl := range file.Decls { if fn, isFn := decl.(*ast.FuncDecl); isFn { if !fn.Name.IsExported() || strings.HasPrefix(fn.Name.Name, "Test") { continue } - fnDoc, err := createFunctionDoc(fset, fn) + fnDoc, err := createFunctionDoc(fset, fn, info) if err != nil { return err } - // Using fnDoc.Name instead of fn.Name.Name - // fnDoc.Name will be a unique identifier for each method or function pkgDoc.Functions = append(pkgDoc.Functions, fnDoc) } } return nil } -func createFunctionDoc(fset *token.FileSet, fn *ast.FuncDecl) (FunctionDoc, error) { +func createFunctionDoc(fset *token.FileSet, fn *ast.FuncDecl, info *types.Info) (FunctionDoc, error) { var params, results, structName string var err error + + // Extract parameters and results if fn.Type.Params != nil { params, err = formatNode(fset, fn.Type.Params) if err != nil { return FunctionDoc{}, fmt.Errorf("error formatting function parameters: %w", err) } - } if fn.Type.Results != nil { results, err = formatNode(fset, fn.Type.Results) @@ -409,7 +422,7 @@ func createFunctionDoc(fset *token.FileSet, fn *ast.FuncDecl) (FunctionDoc, erro } signature := fmt.Sprintf("%s(%s) %s", fn.Name.Name, params, results) - signature = strings.TrimRight(signature, " ") // Trim trailing space + signature = strings.TrimRight(signature, " ") // Split the signature if it's too long const maxLineLength = 80 diff --git a/docs/docGeneration_test.go b/docs/docGeneration_test.go index 9e8a0020..8aa6712a 100644 --- a/docs/docGeneration_test.go +++ b/docs/docGeneration_test.go @@ -1,13 +1,13 @@ package docs_test import ( + "os" "path/filepath" "strings" "testing" "github.com/l50/goutils/v2/docs" "github.com/l50/goutils/v2/git" - "github.com/l50/goutils/v2/sys" "github.com/spf13/afero" ) @@ -25,16 +25,18 @@ func TestCreatePackageDocs(t *testing.T) { repo docs.Repo templatePath string excludedPkgs []string - setupFs func() afero.Fs + setupFs func(t *testing.T) (afero.Fs, string) expectErr bool expectPkgName string + expectPkgDir bool }{ { name: "valid template path", repo: repo, - setupFs: func() afero.Fs { + setupFs: func(t *testing.T) (afero.Fs, string) { fs, templatePath := initBaseFs(t) initCommonDirs(fs, templatePath) + _ = fs.MkdirAll("pkg", 0755) filesToCopy := []string{ "templates/README.md.tmpl", "templates/README.md.tmpl", @@ -43,27 +45,19 @@ func TestCreatePackageDocs(t *testing.T) { t.Fatal(err) } - if err := sys.Cd(templatePath); err != nil { - t.Fatal(err) - } - - if _, err := sys.RunCommand("git", "init"); err != nil { - t.Fatal(err) - } - - return fs + return fs, "" // Return empty string for tempDir }, templatePath: templatePath, expectErr: false, + expectPkgDir: true, }, { name: "invalid template path", repo: repo, - setupFs: func() afero.Fs { + setupFs: func(t *testing.T) (afero.Fs, string) { fs, templatePath := initBaseFs(t) initCommonDirs(fs, templatePath) - - return fs + return fs, "" }, templatePath: "nonexistent_template.tmpl", expectErr: true, @@ -71,12 +65,12 @@ func TestCreatePackageDocs(t *testing.T) { { name: "path outside root directory", repo: repo, - setupFs: func() afero.Fs { + setupFs: func(t *testing.T) (afero.Fs, string) { fs := afero.NewMemMapFs() _ = fs.MkdirAll("/Users/bob/co/opensource/asdf/pkg", 0755) - _ = afero.WriteFile(fs, templatePath, []byte("{{.PackageName}}"), 0644) + _ = afero.WriteFile(fs, templatePath, []byte("docs_test"), 0644) _ = afero.WriteFile(fs, "/Users/bob/co/opensource/asdf/pkg", []byte("module github.com/"+repo.Owner+"/"+repo.Name), 0644) - return fs + return fs, "" }, templatePath: templatePath, expectErr: true, @@ -84,12 +78,12 @@ func TestCreatePackageDocs(t *testing.T) { { name: "absolute path given", repo: repo, - setupFs: func() afero.Fs { + setupFs: func(t *testing.T) (afero.Fs, string) { fs := afero.NewMemMapFs() _ = fs.MkdirAll("/Users/bob/co/opensource/asdf/pkg", 0755) - _ = afero.WriteFile(fs, templatePath, []byte("{{.PackageName}}"), 0644) + _ = afero.WriteFile(fs, templatePath, []byte("docs_test"), 0644) _ = afero.WriteFile(fs, "/Users/bob/co/opensource/asdf/pkg", []byte("module github.com/"+repo.Owner+"/"+repo.Name), 0644) - return fs + return fs, "" }, templatePath: "/Users/bob/co/opensource/asdf/pkg", expectErr: true, @@ -98,54 +92,96 @@ func TestCreatePackageDocs(t *testing.T) { name: "excluding specific packages", repo: repo, excludedPkgs: []string{"excludedPkg1", "excludedPkg2"}, - setupFs: func() afero.Fs { + setupFs: func(t *testing.T) (afero.Fs, string) { fs := afero.NewMemMapFs() - _ = afero.WriteFile(fs, templatePath, []byte("{{.PackageName}}"), 0644) + _ = afero.WriteFile(fs, templatePath, []byte("docs_test"), 0644) _ = afero.WriteFile(fs, "go.mod", []byte("module github.com/"+repo.Owner+"/"+repo.Name), 0644) - // Write some data to simulate the packages being present _ = fs.MkdirAll("excludedPkg1", 0755) _ = fs.MkdirAll("excludedPkg2", 0755) - return fs + return fs, "" }, templatePath: templatePath, expectErr: false, }, + { + name: "exclude test files", + repo: repo, + setupFs: func(t *testing.T) (afero.Fs, string) { + fs := afero.NewOsFs() + + // Create a temporary directory + tempDir, err := os.MkdirTemp("", "testDocs") + if err != nil { + t.Fatal(err) + } + + // Ensure cleanup + t.Cleanup(func() { os.RemoveAll(tempDir) }) + + // Write files to the temp directory + _ = os.MkdirAll(filepath.Join(tempDir, "templates"), 0755) + _ = os.MkdirAll(filepath.Join(tempDir, "pkg"), 0755) + _ = os.WriteFile(filepath.Join(tempDir, "templates", "README.md.tmpl"), []byte("{{range .Functions}}{{.Name}}{{end}}"), 0644) + _ = os.WriteFile(filepath.Join(tempDir, "go.mod"), []byte("module pkg"), 0644) + _ = os.WriteFile(filepath.Join(tempDir, "pkg", "regular.go"), []byte(` +package pkg + +func RegularFunction() {} +`), 0644) + _ = os.WriteFile(filepath.Join(tempDir, "pkg", "regular_test.go"), []byte(` +package pkg + +func TestRegularFunction() {} +`), 0644) + return fs, tempDir // Return tempDir + }, + templatePath: filepath.Join("templates", "README.md.tmpl"), + expectErr: false, + expectPkgName: "RegularFunction", + expectPkgDir: true, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - fs := tc.setupFs() + fs, tempDir := tc.setupFs(t) - // Check directory explicitly: - dirExists, _ := afero.DirExists(fs, "templates") - if !dirExists { - t.Error("magefiles directory does not exist in the in-memory file system") + // Change working directory if tempDir is provided + if tempDir != "" { + changeWorkingDir(t, tempDir) + defer revertWorkingDir(t) } - if tc.expectPkgName != "" { - readmePath := filepath.Join("templates", "README.md") - content, err := afero.ReadFile(fs, readmePath) - if err == nil && !strings.Contains(string(content), tc.expectPkgName) { - t.Errorf("expected package name %s not found in generated README", tc.expectPkgName) - } + debugFileSystem(fs, t, "Before CreatePackageDocs") + + checkTemplatesDir(fs, t) + + if tc.expectPkgDir { + checkPkgDir(fs, t) } err := docs.CreatePackageDocs(fs, tc.repo, tc.templatePath, tc.excludedPkgs...) if (err != nil) != tc.expectErr { t.Errorf("CreatePackageDocs() error = %v, expectErr %v", err, tc.expectErr) } + + debugFileSystem(fs, t, "After CreatePackageDocs") + + if tc.expectPkgName != "" { + verifyReadmeContent(fs, t, tc.expectPkgName) + } }) } } func initBaseFs(t *testing.T) (afero.Fs, string) { baseFs := afero.NewOsFs() - templatePath, err := afero.TempDir(baseFs, "", "testDocs") + templateDir, err := afero.TempDir(baseFs, "", "testDocs") if err != nil { t.Fatal(err) } - return afero.NewBasePathFs(baseFs, templatePath), templatePath + return afero.NewBasePathFs(baseFs, templateDir), filepath.Join(templateDir, "templates", "README.md.tmpl") } func initCommonDirs(fs afero.Fs, path string) { @@ -176,3 +212,61 @@ func copyFileToMockFs(srcFs afero.Fs, destFs afero.Fs, srcPath, destPath string) return afero.WriteFile(destFs, destPath, content, 0644) } + +func changeWorkingDir(t *testing.T, dir string) { + err := os.Chdir(dir) + if err != nil { + t.Fatalf("Failed to change working directory: %v", err) + } +} + +func revertWorkingDir(t *testing.T) { + err := os.Chdir("..") + if err != nil { + t.Fatalf("Failed to revert working directory: %v", err) + } +} + +func debugFileSystem(fs afero.Fs, t *testing.T, message string) { + files, _ := afero.ReadDir(fs, ".") + t.Logf("%s - Files and directories in root:", message) + for _, f := range files { + t.Logf("- %s (IsDir: %v)", f.Name(), f.IsDir()) + } +} + +func checkTemplatesDir(fs afero.Fs, t *testing.T) { + dirExists, _ := afero.DirExists(fs, "templates") + if !dirExists { + t.Error("templates directory does not exist in the in-memory file system") + } +} + +func checkPkgDir(fs afero.Fs, t *testing.T) { + pkgDirExists, _ := afero.DirExists(fs, "pkg") + if !pkgDirExists { + t.Error("pkg directory does not exist in the in-memory file system") + } else { + // Debug: List files in the pkg directory + pkgFiles, _ := afero.ReadDir(fs, "pkg") + t.Logf("Files in pkg directory:") + for _, f := range pkgFiles { + t.Logf("- %s", f.Name()) + } + } +} + +func verifyReadmeContent(fs afero.Fs, t *testing.T, expectedPkgName string) { + readmePath := filepath.Join("pkg", "README.md") + content, err := afero.ReadFile(fs, readmePath) + if err != nil { + t.Errorf("Failed to read generated README: %v", err) + } else { + if !strings.Contains(string(content), expectedPkgName) { + t.Errorf("Expected function name %s not found in generated README", expectedPkgName) + } + if strings.Contains(string(content), "Test"+expectedPkgName) { + t.Errorf("Test function Test%s should not be in generated README", expectedPkgName) + } + } +} diff --git a/go.mod b/go.mod index 40723776..7ac149ea 100644 --- a/go.mod +++ b/go.mod @@ -14,12 +14,12 @@ require ( github.com/go-git/go-git/v5 v5.12.0 github.com/golang/mock v1.6.0 github.com/magefile/mage v1.15.0 - github.com/mattn/go-isatty v0.0.20 github.com/otiai10/copy v1.14.0 github.com/samber/slog-multi v1.1.0 github.com/spf13/afero v1.11.0 github.com/stretchr/testify v1.9.0 github.com/tidwall/gjson v1.17.1 + golang.org/x/tools v0.22.0 k8s.io/api v0.30.2 k8s.io/apimachinery v0.30.2 k8s.io/client-go v0.30.2 @@ -60,6 +60,7 @@ require ( github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/lib/pq v1.10.9 // indirect github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect + github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-runewidth v0.0.15 // indirect github.com/miekg/dns v1.1.61 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect @@ -76,6 +77,7 @@ require ( github.com/xeipuuv/gojsonschema v1.2.0 // indirect github.com/xlab/treeprint v1.2.0 // indirect go.starlark.net v0.0.0-20240411212711-9b43f0afd521 // indirect + golang.org/x/mod v0.18.0 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/logging/README.md b/logging/README.md index d2dfe6e1..e00858ab 100644 --- a/logging/README.md +++ b/logging/README.md @@ -285,10 +285,10 @@ error: An error if any issue occurs during initialization. --- -### NewPrettyHandler(io.Writer, PrettyHandlerOptions) +### NewPrettyHandler(io.Writer, PrettyHandlerOptions, OutputType) ```go -NewPrettyHandler(io.Writer, PrettyHandlerOptions) *PrettyHandler +NewPrettyHandler(io.Writer, PrettyHandlerOptions, OutputType) *PrettyHandler ``` NewPrettyHandler creates a new PrettyHandler with specified output @@ -299,6 +299,7 @@ log messages with optional colorization and structured formatting. out: Output writer where log messages will be written. opts: PrettyHandlerOptions for configuring the handler. +outputType: Type of output for the handler. **Returns:** diff --git a/logging/logutils.go b/logging/logutils.go index eb21ec33..2625063e 100644 --- a/logging/logutils.go +++ b/logging/logutils.go @@ -1,6 +1,7 @@ package logging import ( + "errors" "fmt" "os" "path/filepath" @@ -26,6 +27,10 @@ const ( // text output. This is useful for console output where color // coding can enhance readability. ColorOutput + + // JSONOutput indicates that the logger will produce JSON formatted + // output. This is useful for structured logging and log aggregation. + JSONOutput ) // CreateLogFile creates a log file in a 'logs' subdirectory of the @@ -110,7 +115,7 @@ func (cfg *LogConfig) ConfigureLogger() (Logger, error) { if cfg.OutputType == ColorOutput { prettyOpts := PrettyHandlerOptions{SlogOpts: *opts} - stdoutHandler = NewPrettyHandler(os.Stdout, prettyOpts) + stdoutHandler = NewPrettyHandler(os.Stdout, prettyOpts, cfg.OutputType) } else { stdoutHandler = slog.NewJSONHandler(os.Stdout, opts) } @@ -195,7 +200,7 @@ func InitLogging(cfg *LogConfig) (Logger, error) { // // error: The error created from the errMsg, after it has been logged. func LogAndReturnError(logger Logger, errMsg string) error { - err := fmt.Errorf(errMsg) + err := errors.New(errMsg) logger.Error(err) return err } diff --git a/logging/prettyhandler.go b/logging/prettyhandler.go index d02bd316..13107007 100644 --- a/logging/prettyhandler.go +++ b/logging/prettyhandler.go @@ -5,13 +5,11 @@ import ( "encoding/json" "fmt" "io" - "log" + stdlog "log" "log/slog" - "os" "time" "github.com/fatih/color" - "github.com/mattn/go-isatty" ) // PrettyHandlerOptions represents options used for configuring @@ -32,9 +30,13 @@ type PrettyHandlerOptions struct { // // Handler: The underlying slog.Handler used for logging. // l: Standard logger used for outputting log messages. +// Level: The log level for the handler. +// OutputType: The type of output for the handler. type PrettyHandler struct { slog.Handler - l *log.Logger + l *stdlog.Logger + Level slog.Level + OutputType OutputType } // NewPrettyHandler creates a new PrettyHandler with specified output @@ -45,14 +47,17 @@ type PrettyHandler struct { // // out: Output writer where log messages will be written. // opts: PrettyHandlerOptions for configuring the handler. +// outputType: Type of output for the handler. // // **Returns:** // // *PrettyHandler: A new instance of PrettyHandler. -func NewPrettyHandler(out io.Writer, opts PrettyHandlerOptions) *PrettyHandler { +func NewPrettyHandler(out io.Writer, opts PrettyHandlerOptions, outputType OutputType) *PrettyHandler { h := &PrettyHandler{ - Handler: slog.NewJSONHandler(out, &opts.SlogOpts), - l: log.New(out, "", 0), + Handler: slog.NewJSONHandler(out, &opts.SlogOpts), + l: stdlog.New(out, "", 0), // Use stdlog.New + Level: opts.SlogOpts.Level.Level(), + OutputType: outputType, } return h } @@ -70,30 +75,21 @@ func NewPrettyHandler(out io.Writer, opts PrettyHandlerOptions) *PrettyHandler { // // error: An error if any issue occurs during log handling. func (h *PrettyHandler) Handle(ctx context.Context, r slog.Record) error { + if r.Level < h.Level { + return nil // Skip log records below the minimum level + } + fields, err := h.parseLogRecord(r) if err != nil { - return err // Return error if JSON is invalid or any other error occurs in parsing + return err } - if h.outputToFile() { + if h.OutputType == JSONOutput { return h.outputJSON(fields) } - return h.outputFormatted(fields, r.Level) } -// outputToFile determines if the output is being written to a file -// rather than a terminal, in which case it returns true. -// -// **Returns:** -// -// bool: True if output is to a file, false otherwise. -func (h *PrettyHandler) outputToFile() bool { - _, isFile := h.l.Writer().(*os.File) - isTerminal := isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()) - return isFile && !isTerminal -} - // outputJSON marshals the log fields into JSON format and outputs // them using the logger. This is used when logging to a file or // non-terminal output. @@ -127,9 +123,34 @@ func (h *PrettyHandler) outputJSON(fields map[string]interface{}) error { // // error: An error if formatting or output fails. func (h *PrettyHandler) outputFormatted(fields map[string]interface{}, level slog.Level) error { - finalLogMsg := fmt.Sprintf("[%s] [%s] %s", fields["time"], h.colorizeBasedOnLevel(level), fields["msg"]) - h.l.Println(finalLogMsg) - return nil + timeStr := fields["time"].(string) + msg := fields["msg"].(string) + + var levelColor string + if h.OutputType == ColorOutput { + switch level { + case slog.LevelDebug: + levelColor = "\033[36m" // Cyan + case slog.LevelInfo: + levelColor = "\033[32m" // Green + case slog.LevelWarn: + levelColor = "\033[33m" // Yellow + case slog.LevelError: + levelColor = "\033[31m" // Red + default: + levelColor = "\033[0m" // Default + } + } + + resetColor := "\033[0m" + if h.OutputType != ColorOutput { + levelColor = "" + resetColor = "" + } + + formattedMsg := fmt.Sprintf("%s %s%s%s: %s\n", timeStr, levelColor, level.String(), resetColor, msg) + _, err := h.l.Writer().Write([]byte(formattedMsg)) + return err } // parseLogRecord parses the given slog.Record into a map of log fields. @@ -162,27 +183,6 @@ func (h *PrettyHandler) parseLogRecord(r slog.Record) (map[string]interface{}, e return fields, nil } -// colorizeBasedOnLevel applies color to the given log level string -// based on its severity. -// -// **Parameters:** -// -// level: Log level to be colorized. -// -// **Returns:** -// -// string: The colorized log level string. -func (h *PrettyHandler) colorizeBasedOnLevel(level slog.Level) string { - // Create a new color object based on the log level - colorAttr := determineColorAttribute(level) - c := color.New(colorAttr) - - // Apply color only to the level part - coloredLevel := c.Sprint(level.String()) - - return coloredLevel -} - // determineColorAttribute returns the color attribute corresponding // to the given log level. // diff --git a/logging/prettyhandler_test.go b/logging/prettyhandler_test.go index 5134518e..dbb84e55 100644 --- a/logging/prettyhandler_test.go +++ b/logging/prettyhandler_test.go @@ -46,24 +46,25 @@ func TestPrettyHandlerHandle(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - // Create a buffer to capture the output var buf strings.Builder - prettyHandler := logging.NewPrettyHandler(&buf, logging.PrettyHandlerOptions{}) + opts := logging.PrettyHandlerOptions{ + SlogOpts: slog.HandlerOptions{ + Level: tc.level, + }, + } + prettyHandler := logging.NewPrettyHandler(&buf, opts, logging.ColorOutput) - // Create a log record record := slog.Record{ Level: tc.level, Time: time.Now(), Message: tc.msg, } - // Call the handle method err := prettyHandler.Handle(context.Background(), record) if err != nil { t.Fatalf("Handle() error = %v", err) } - // Check if the output contains the expected msg if !strings.Contains(buf.String(), tc.expected) { t.Fatalf("Expected to find '%s' in the output, got '%s'", tc.expected, buf.String()) } @@ -97,19 +98,22 @@ func TestPrettyHandlerParseLogRecord(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - prettyHandler := logging.NewPrettyHandler(io.Discard, logging.PrettyHandlerOptions{}) + opts := logging.PrettyHandlerOptions{ + SlogOpts: slog.HandlerOptions{ + Level: tc.record.Level, + }, + } + prettyHandler := logging.NewPrettyHandler(io.Discard, opts, logging.JSONOutput) - // Create a log record record := slog.Record{ Level: tc.record.Level, Time: time.Now(), Message: tc.record.Message, } - // Call the handle method err := prettyHandler.Handle(context.Background(), record) if (err != nil) != tc.expectError { - t.Errorf("parseLogRecord() for %s expected error: %v, got: %v", tc.name, tc.expectError, err) + t.Errorf("Handle() for %s expected error: %v, got: %v", tc.name, tc.expectError, err) } }) } @@ -117,41 +121,56 @@ func TestPrettyHandlerParseLogRecord(t *testing.T) { func TestPrettyHandlerColorization(t *testing.T) { testCases := []struct { - name string - level slog.Level + name string + level slog.Level + outputType logging.OutputType + expectColor bool }{ { - name: "Info Level Color", - level: slog.LevelInfo, + name: "Info Level Color", + level: slog.LevelInfo, + outputType: logging.ColorOutput, + expectColor: true, }, { - name: "Error Level Color", - level: slog.LevelError, + name: "Error Level Color", + level: slog.LevelError, + outputType: logging.ColorOutput, + expectColor: true, + }, + { + name: "JSON Output No Color", + level: slog.LevelInfo, + outputType: logging.JSONOutput, + expectColor: false, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { var buf strings.Builder - prettyHandler := logging.NewPrettyHandler(&buf, logging.PrettyHandlerOptions{}) + opts := logging.PrettyHandlerOptions{ + SlogOpts: slog.HandlerOptions{ + Level: tc.level, + }, + } + prettyHandler := logging.NewPrettyHandler(&buf, opts, tc.outputType) - // Create a log record record := slog.Record{ Level: tc.level, Time: time.Now(), Message: "test message", } - // Call the handle method err := prettyHandler.Handle(context.Background(), record) if err != nil { t.Errorf("Handle() error = %v", err) } - // Ensure the output does not contain ANSI color codes output := buf.String() - if strings.Contains(output, "\u001b[") { - t.Errorf("Output should not contain color codes, got '%s'", output) + hasColorCodes := strings.Contains(output, "\u001b[") + if hasColorCodes != tc.expectColor { + t.Errorf("Color codes presence (%v) does not match expectation (%v) for output type %v", hasColorCodes, tc.expectColor, tc.outputType) } }) }