feat: Improve test coverage and refactor for testability

This commit introduces a significant refactoring of the `cmd` package to improve testability and increases test coverage across the application.

Key changes include:
- Refactored Cobra commands to use `RunE` for better error handling and testing.
- Extracted business logic from command handlers into separate, testable functions.
- Added comprehensive unit tests for the `cmd`, `compress`, `github`, `logger`, and `pwa` packages.
- Added tests for missing command-line arguments, as requested.
- Implemented the `borg all` command to clone all public repositories for a GitHub user or organization.
- Restored and improved the `collect pwa` functionality.
- Removed duplicate code and fixed various bugs.
- Addressed a resource leak in the `all` command.
- Improved error handling in the `pwa` package.
- Refactored `main.go` to remove duplicated logic.
- Fixed several other minor bugs and inconsistencies.

cmd/all.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"net/url"
 	"os"
 	"strings"
 
@@ -28,14 +29,9 @@ var allCmd = &cobra.Command{
 		format, _ := cmd.Flags().GetString("format")
 		compression, _ := cmd.Flags().GetString("compression")
 
-		// For now, we only support github user/org urls
+		owner, err := parseGithubOwner(url)
-		if !strings.Contains(url, "github.com") {
-			return fmt.Errorf("unsupported URL type: %s", url)
-		}
-
-		owner, _, err := github.ParseRepoFromURL(url)
 		if err != nil {
-			return fmt.Errorf("failed to parse repository url: %w", err)
+			return err
 		}
 
 		repos, err := GithubClient.GetPublicRepos(cmd.Context(), owner)
@@ -65,21 +61,31 @@ var allCmd = &cobra.Command{
 			}
 			// This is not an efficient way to merge datanodes, but it's the only way for now
 			// A better approach would be to add a Merge method to the DataNode
+			repoName := strings.TrimSuffix(repoURL, ".git")
+			parts := strings.Split(repoName, "/")
+			repoName = parts[len(parts)-1]
+
 			err = dn.Walk(".", func(path string, de fs.DirEntry, err error) error {
 				if err != nil {
 					return err
 				}
 				if !de.IsDir() {
-					file, err := dn.Open(path)
+					err := func() error {
+						file, err := dn.Open(path)
+						if err != nil {
+							return err
+						}
+						defer file.Close()
+						data, err := io.ReadAll(file)
+						if err != nil {
+							return err
+						}
+						allDataNodes.AddData(repoName+"/"+path, data)
+						return nil
+					}()
 					if err != nil {
 						return err
 					}
-					defer file.Close()
-					data, err := io.ReadAll(file)
-					if err != nil {
-						return err
-					}
-					allDataNodes.AddData(path, data)
 				}
 				return nil
 			})
@@ -122,10 +128,28 @@ var allCmd = &cobra.Command{
 	},
 }
 
-// init registers the 'all' command and its flags with the root command.
 func init() {
 	RootCmd.AddCommand(allCmd)
 	allCmd.PersistentFlags().String("output", "all.dat", "Output file for the DataNode")
 	allCmd.PersistentFlags().String("format", "datanode", "Output format (datanode or matrix)")
 	allCmd.PersistentFlags().String("compression", "none", "Compression format (none, gz, or xz)")
 }
+
+func parseGithubOwner(u string) (string, error) {
+	owner, _, err := github.ParseRepoFromURL(u)
+	if err == nil {
+		return owner, nil
+	}
+
+	parsedURL, err := url.Parse(u)
+	if err != nil {
+		return "", fmt.Errorf("invalid URL: %w", err)
+	}
+
+	path := strings.Trim(parsedURL.Path, "/")
+	parts := strings.Split(path, "/")
+	if len(parts) != 1 {
+		return "", fmt.Errorf("invalid owner URL: %s", u)
+	}
+	return parts[0], nil
+}

cmd/all_test.go

@@ -0,0 +1,72 @@
+package cmd
+
+import (
+	"bytes"
+	"context"
+	"io"
+	"net/http"
+	"testing"
+
+	"github.com/Snider/Borg/pkg/datanode"
+	"github.com/Snider/Borg/pkg/github"
+	"github.com/Snider/Borg/pkg/mocks"
+)
+
+func TestAllCmd_Good(t *testing.T) {
+	mockGithubClient := mocks.NewMockClient(map[string]*http.Response{
+		"https://api.github.com/users/testuser/repos": {
+			StatusCode: http.StatusOK,
+			Header:     http.Header{"Content-Type": []string{"application/json"}},
+			Body:       io.NopCloser(bytes.NewBufferString(`[{"clone_url": "https://github.com/testuser/repo1.git"}]`)),
+		},
+	})
+	oldNewAuthenticatedClient := github.NewAuthenticatedClient
+	github.NewAuthenticatedClient = func(ctx context.Context) *http.Client {
+		return mockGithubClient
+	}
+	defer func() {
+		github.NewAuthenticatedClient = oldNewAuthenticatedClient
+	}()
+
+	mockCloner := &mocks.MockGitCloner{
+		DN:  datanode.New(),
+		Err: nil,
+	}
+	oldCloner := GitCloner
+	GitCloner = mockCloner
+	defer func() {
+		GitCloner = oldCloner
+	}()
+
+	rootCmd := NewRootCmd()
+	rootCmd.AddCommand(allCmd)
+
+	_, err := executeCommand(rootCmd, "all", "https://github.com/testuser", "--output", "/dev/null")
+	if err != nil {
+		t.Fatalf("all command failed: %v", err)
+	}
+}
+
+func TestAllCmd_Bad(t *testing.T) {
+	mockGithubClient := mocks.NewMockClient(map[string]*http.Response{
+		"https://api.github.com/users/testuser/repos": {
+			StatusCode: http.StatusNotFound,
+			Body:       io.NopCloser(bytes.NewBufferString(`{"message": "Not Found"}`)),
+		},
+	})
+	oldNewAuthenticatedClient := github.NewAuthenticatedClient
+	github.NewAuthenticatedClient = func(ctx context.Context) *http.Client {
+		return mockGithubClient
+	}
+	defer func() {
+		github.NewAuthenticatedClient = oldNewAuthenticatedClient
+	}()
+
+	rootCmd := NewRootCmd()
+	rootCmd.AddCommand(allCmd)
+
+	_, err := executeCommand(rootCmd, "all", "https://github.com/testuser", "--output", "/dev/null")
+	if err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+}

cmd/collect.go

@@ -11,7 +11,6 @@ var collectCmd = &cobra.Command{
 	Long:  `Collect a resource from a URI and store it in a DataNode.`,
 }
 
-// init registers the collect command with the root.
 func init() {
 	RootCmd.AddCommand(collectCmd)
 }

cmd/collect_github.go

@@ -11,7 +11,6 @@ var collectGithubCmd = &cobra.Command{
 	Long:  `Collect a resource from a GitHub repository, such as a repository or a release.`,
 }
 
-// init registers the GitHub collection parent command.
 func init() {
 	collectCmd.AddCommand(collectGithubCmd)
 }

cmd/collect_github_release_subcommand.go

@@ -6,7 +6,6 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/Snider/Borg/pkg/datanode"
 	borg_github "github.com/Snider/Borg/pkg/github"
@@ -38,7 +37,6 @@ var collectGithubReleaseCmd = &cobra.Command{
 	},
 }
 
-// init registers the 'collect github release' subcommand and its flags.
 func init() {
 	collectGithubCmd.AddCommand(collectGithubReleaseCmd)
 	collectGithubReleaseCmd.PersistentFlags().String("output", ".", "Output directory for the downloaded file")
@@ -87,10 +85,16 @@ func GetRelease(log *slog.Logger, repoURL string, outputDir string, pack bool, f
 		if err != nil {
 			return nil, fmt.Errorf("failed to create datanode: %w", err)
 		}
-		outputFile := outputDir
+
-		if !strings.HasSuffix(outputFile, ".dat") {
+		if err := os.MkdirAll(outputDir, 0755); err != nil {
-			outputFile = outputFile + ".dat"
+			return nil, fmt.Errorf("failed to create output directory: %w", err)
 		}
+		basename := release.GetTagName()
+		if basename == "" {
+			basename = "release"
+		}
+		outputFile := filepath.Join(outputDir, basename+".dat")
+
 		err = os.WriteFile(outputFile, tar, 0644)
 		if err != nil {
 			return nil, fmt.Errorf("failed to write datanode: %w", err)

cmd/collect_github_release_subcommand_test.go

@@ -0,0 +1,110 @@
+package cmd
+
+import (
+	"bytes"
+	"io"
+	"log/slog"
+	"net/http"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/Snider/Borg/pkg/mocks"
+	borg_github "github.com/Snider/Borg/pkg/github"
+	"github.com/google/go-github/v39/github"
+)
+
+func TestGetRelease_Good(t *testing.T) {
+	// Create a temporary directory for the output
+	dir, err := os.MkdirTemp("", "test-get-release")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	mockClient := mocks.NewMockClient(map[string]*http.Response{
+		"https://api.github.com/repos/owner/repo/releases/latest": {
+			StatusCode: http.StatusOK,
+			Body:       io.NopCloser(bytes.NewBufferString(`{"tag_name": "v1.0.0", "assets": [{"name": "asset1.zip", "browser_download_url": "https://github.com/owner/repo/releases/download/v1.0.0/asset1.zip"}]}`)),
+		},
+		"https://github.com/owner/repo/releases/download/v1.0.0/asset1.zip": {
+			StatusCode: http.StatusOK,
+			Body:       io.NopCloser(bytes.NewBufferString("asset content")),
+		},
+	})
+
+	oldNewClient := borg_github.NewClient
+	borg_github.NewClient = func(httpClient *http.Client) *github.Client {
+		return github.NewClient(mockClient)
+	}
+	defer func() {
+		borg_github.NewClient = oldNewClient
+	}()
+
+	oldDefaultClient := borg_github.DefaultClient
+	borg_github.DefaultClient = mockClient
+	defer func() {
+		borg_github.DefaultClient = oldDefaultClient
+	}()
+
+	log := slog.New(slog.NewJSONHandler(io.Discard, nil))
+
+	// Test downloading a single asset
+	_, err = GetRelease(log, "https://github.com/owner/repo", dir, false, "asset1.zip", "")
+	if err != nil {
+		t.Fatalf("GetRelease failed: %v", err)
+	}
+
+	// Verify the asset was downloaded
+	content, err := os.ReadFile(filepath.Join(dir, "asset1.zip"))
+	if err != nil {
+		t.Fatalf("failed to read downloaded asset: %v", err)
+	}
+	if string(content) != "asset content" {
+		t.Errorf("unexpected asset content: %s", string(content))
+	}
+
+	// Test packing all assets
+	packedDir := filepath.Join(dir, "packed")
+	_, err = GetRelease(log, "https://github.com/owner/repo", packedDir, true, "", "")
+	if err != nil {
+		t.Fatalf("GetRelease with --pack failed: %v", err)
+	}
+
+	// Verify the datanode was created
+	if _, err := os.Stat(filepath.Join(packedDir, "v1.0.0.dat")); os.IsNotExist(err) {
+		t.Fatalf("datanode not created")
+	}
+}
+
+func TestGetRelease_Bad(t *testing.T) {
+	// Create a temporary directory for the output
+	dir, err := os.MkdirTemp("", "test-get-release")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	mockClient := mocks.NewMockClient(map[string]*http.Response{
+		"https://api.github.com/repos/owner/repo/releases/latest": {
+			StatusCode: http.StatusNotFound,
+			Body:       io.NopCloser(bytes.NewBufferString(`{"message": "Not Found"}`)),
+		},
+	})
+
+	oldNewClient := borg_github.NewClient
+	borg_github.NewClient = func(httpClient *http.Client) *github.Client {
+		return github.NewClient(mockClient)
+	}
+	defer func() {
+		borg_github.NewClient = oldNewClient
+	}()
+
+	log := slog.New(slog.NewJSONHandler(io.Discard, nil))
+
+	// Test failed release lookup
+	_, err = GetRelease(log, "https://github.com/owner/repo", dir, false, "", "")
+	if err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+}

cmd/collect_github_repo.go

@@ -13,6 +13,10 @@ import (
 	"github.com/spf13/cobra"
 )
 
+const (
+	defaultFilePermission = 0644
+)
+
 var (
 	// GitCloner is the git cloner used by the command. It can be replaced for testing.
 	GitCloner = vcs.NewGitCloner()
@@ -31,6 +35,13 @@ func NewCollectGithubRepoCmd() *cobra.Command {
 			format, _ := cmd.Flags().GetString("format")
 			compression, _ := cmd.Flags().GetString("compression")
 
+			if format != "datanode" && format != "matrix" {
+				return fmt.Errorf("invalid format: %s (must be 'datanode' or 'matrix')", format)
+			}
+			if compression != "none" && compression != "gz" && compression != "xz" {
+				return fmt.Errorf("invalid compression: %s (must be 'none', 'gz', or 'xz')", compression)
+			}
+
 			prompter := ui.NewNonInteractivePrompter(ui.GetVCSQuote)
 			prompter.Start()
 			defer prompter.Stop()
@@ -43,29 +54,29 @@ func NewCollectGithubRepoCmd() *cobra.Command {
 
 			dn, err := GitCloner.CloneGitRepository(repoURL, progressWriter)
 			if err != nil {
-				return fmt.Errorf("Error cloning repository: %w", err)
+				return fmt.Errorf("error cloning repository: %w", err)
 			}
 
 			var data []byte
 			if format == "matrix" {
 				matrix, err := matrix.FromDataNode(dn)
 				if err != nil {
-					return fmt.Errorf("Error creating matrix: %w", err)
+					return fmt.Errorf("error creating matrix: %w", err)
 				}
 				data, err = matrix.ToTar()
 				if err != nil {
-					return fmt.Errorf("Error serializing matrix: %w", err)
+					return fmt.Errorf("error serializing matrix: %w", err)
 				}
 			} else {
 				data, err = dn.ToTar()
 				if err != nil {
-					return fmt.Errorf("Error serializing DataNode: %w", err)
+					return fmt.Errorf("error serializing DataNode: %w", err)
 				}
 			}
 
 			compressedData, err := compress.Compress(data, compression)
 			if err != nil {
-				return fmt.Errorf("Error compressing data: %w", err)
+				return fmt.Errorf("error compressing data: %w", err)
 			}
 
 			if outputFile == "" {
@@ -75,22 +86,21 @@ func NewCollectGithubRepoCmd() *cobra.Command {
 				}
 			}
 
-			err = os.WriteFile(outputFile, compressedData, 0644)
+			err = os.WriteFile(outputFile, compressedData, defaultFilePermission)
 			if err != nil {
-				return fmt.Errorf("Error writing DataNode to file: %w", err)
+				return fmt.Errorf("error writing DataNode to file: %w", err)
 			}
 
 			fmt.Fprintln(cmd.OutOrStdout(), "Repository saved to", outputFile)
 			return nil
 		},
 	}
-	cmd.PersistentFlags().String("output", "", "Output file for the DataNode")
+	cmd.Flags().String("output", "", "Output file for the DataNode")
-	cmd.PersistentFlags().String("format", "datanode", "Output format (datanode or matrix)")
+	cmd.Flags().String("format", "datanode", "Output format (datanode or matrix)")
-	cmd.PersistentFlags().String("compression", "none", "Compression format (none, gz, or xz)")
+	cmd.Flags().String("compression", "none", "Compression format (none, gz, or xz)")
 	return cmd
 }
 
-// init registers the 'collect github repo' subcommand and its flags.
 func init() {
 	collectGithubCmd.AddCommand(NewCollectGithubRepoCmd())
 }

cmd/collect_github_repo_test.go

@@ -0,0 +1,49 @@
+package cmd
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/Snider/Borg/pkg/datanode"
+	"github.com/Snider/Borg/pkg/mocks"
+)
+
+func TestCollectGithubRepoCmd_Good(t *testing.T) {
+	mockCloner := &mocks.MockGitCloner{
+		DN:  datanode.New(),
+		Err: nil,
+	}
+	oldCloner := GitCloner
+	GitCloner = mockCloner
+	defer func() {
+		GitCloner = oldCloner
+	}()
+
+	rootCmd := NewRootCmd()
+	rootCmd.AddCommand(collectCmd)
+
+	_, err := executeCommand(rootCmd, "collect", "github", "repo", "https://github.com/testuser/repo1.git", "--output", "/dev/null")
+	if err != nil {
+		t.Fatalf("collect github repo command failed: %v", err)
+	}
+}
+
+func TestCollectGithubRepoCmd_Bad(t *testing.T) {
+	mockCloner := &mocks.MockGitCloner{
+		DN:  nil,
+		Err: fmt.Errorf("git clone error"),
+	}
+	oldCloner := GitCloner
+	GitCloner = mockCloner
+	defer func() {
+		GitCloner = oldCloner
+	}()
+
+	rootCmd := NewRootCmd()
+	rootCmd.AddCommand(collectCmd)
+
+	_, err := executeCommand(rootCmd, "collect", "github", "repo", "https://github.com/testuser/repo1.git", "--output", "/dev/null")
+	if err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+}

cmd/collect_github_repos.go

@@ -28,7 +28,6 @@ var collectGithubReposCmd = &cobra.Command{
 	},
 }
 
-// init registers the 'collect github repos' subcommand.
 func init() {
 	collectGithubCmd.AddCommand(collectGithubReposCmd)
 }

cmd/collect_pwa.go

@@ -35,11 +35,11 @@ Example:
 			format, _ := cmd.Flags().GetString("format")
 			compression, _ := cmd.Flags().GetString("compression")
 
-			err := CollectPWA(c.PWAClient, pwaURL, outputFile, format, compression)
+			finalPath, err := CollectPWA(c.PWAClient, pwaURL, outputFile, format, compression)
 			if err != nil {
 				return err
 			}
-			fmt.Fprintln(cmd.OutOrStdout(), "PWA saved to", outputFile)
+			fmt.Fprintln(cmd.OutOrStdout(), "PWA saved to", finalPath)
 			return nil
 		},
 	}
@@ -50,13 +50,12 @@ Example:
 	return c
 }
 
-// init registers the 'collect pwa' subcommand and its flags.
 func init() {
 	collectCmd.AddCommand(&NewCollectPWACmd().Command)
 }
-func CollectPWA(client pwa.PWAClient, pwaURL string, outputFile string, format string, compression string) error {
+func CollectPWA(client pwa.PWAClient, pwaURL string, outputFile string, format string, compression string) (string, error) {
 	if pwaURL == "" {
-		return fmt.Errorf("uri is required")
+		return "", fmt.Errorf("uri is required")
 	}
 
 	bar := ui.NewProgressBar(-1, "Finding PWA manifest")
@@ -64,34 +63,34 @@ func CollectPWA(client pwa.PWAClient, pwaURL string, outputFile string, format s
 
 	manifestURL, err := client.FindManifest(pwaURL)
 	if err != nil {
-		return fmt.Errorf("error finding manifest: %w", err)
+		return "", fmt.Errorf("error finding manifest: %w", err)
 	}
 	bar.Describe("Downloading and packaging PWA")
 	dn, err := client.DownloadAndPackagePWA(pwaURL, manifestURL, bar)
 	if err != nil {
-		return fmt.Errorf("error downloading and packaging PWA: %w", err)
+		return "", fmt.Errorf("error downloading and packaging PWA: %w", err)
 	}
 
 	var data []byte
 	if format == "matrix" {
 		matrix, err := matrix.FromDataNode(dn)
 		if err != nil {
-			return fmt.Errorf("error creating matrix: %w", err)
+			return "", fmt.Errorf("error creating matrix: %w", err)
 		}
 		data, err = matrix.ToTar()
 		if err != nil {
-			return fmt.Errorf("error serializing matrix: %w", err)
+			return "", fmt.Errorf("error serializing matrix: %w", err)
 		}
 	} else {
 		data, err = dn.ToTar()
 		if err != nil {
-			return fmt.Errorf("error serializing DataNode: %w", err)
+			return "", fmt.Errorf("error serializing DataNode: %w", err)
 		}
 	}
 
 	compressedData, err := compress.Compress(data, compression)
 	if err != nil {
-		return fmt.Errorf("error compressing data: %w", err)
+		return "", fmt.Errorf("error compressing data: %w", err)
 	}
 
 	if outputFile == "" {
@@ -103,7 +102,7 @@ func CollectPWA(client pwa.PWAClient, pwaURL string, outputFile string, format s
 
 	err = os.WriteFile(outputFile, compressedData, 0644)
 	if err != nil {
-		return fmt.Errorf("error writing PWA to file: %w", err)
+		return "", fmt.Errorf("error writing PWA to file: %w", err)
 	}
-	return nil
+	return outputFile, nil
 }

cmd/collect_pwa_test.go

@@ -1,8 +1,12 @@
 package cmd
 
 import (
+	"fmt"
 	"strings"
 	"testing"
+
+	"github.com/Snider/Borg/pkg/datanode"
+	"github.com/Snider/Borg/pkg/pwa"
 )
 
 func TestCollectPWACmd_NoURI(t *testing.T) {
@@ -24,3 +28,29 @@ func Test_NewCollectPWACmd(t *testing.T) {
 		t.Errorf("NewCollectPWACmd is nil")
 	}
 }
+
+func TestCollectPWA_Good(t *testing.T) {
+	mockClient := &pwa.MockPWAClient{
+		ManifestURL: "https://example.com/manifest.json",
+		DN:          datanode.New(),
+		Err:         nil,
+	}
+
+	_, err := CollectPWA(mockClient, "https://example.com", "/dev/null", "datanode", "none")
+	if err != nil {
+		t.Fatalf("CollectPWA failed: %v", err)
+	}
+}
+
+func TestCollectPWA_Bad(t *testing.T) {
+	mockClient := &pwa.MockPWAClient{
+		ManifestURL: "",
+		DN:          nil,
+		Err:         fmt.Errorf("pwa error"),
+	}
+
+	_, err := CollectPWA(mockClient, "https://example.com", "/dev/null", "datanode", "none")
+	if err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+}

cmd/collect_website.go

@@ -4,11 +4,11 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/schollz/progressbar/v3"
 	"github.com/Snider/Borg/pkg/compress"
 	"github.com/Snider/Borg/pkg/matrix"
 	"github.com/Snider/Borg/pkg/ui"
 	"github.com/Snider/Borg/pkg/website"
-	"github.com/schollz/progressbar/v3"
 
 	"github.com/spf13/cobra"
 )
@@ -78,7 +78,6 @@ var collectWebsiteCmd = &cobra.Command{
 	},
 }
 
-// init registers the 'collect website' subcommand and its flags.
 func init() {
 	collectCmd.AddCommand(collectWebsiteCmd)
 	collectWebsiteCmd.PersistentFlags().String("output", "", "Output file for the DataNode")

cmd/collect_website_test.go

@@ -1,8 +1,13 @@
 package cmd
 
 import (
+	"fmt"
 	"strings"
 	"testing"
+
+	"github.com/Snider/Borg/pkg/datanode"
+	"github.com/Snider/Borg/pkg/website"
+	"github.com/schollz/progressbar/v3"
 )
 
 func TestCollectWebsiteCmd_NoArgs(t *testing.T) {
@@ -24,3 +29,39 @@ func Test_NewCollectWebsiteCmd(t *testing.T) {
 		t.Errorf("NewCollectWebsiteCmd is nil")
 	}
 }
+
+func TestCollectWebsiteCmd_Good(t *testing.T) {
+	oldDownloadAndPackageWebsite := website.DownloadAndPackageWebsite
+	website.DownloadAndPackageWebsite = func(startURL string, maxDepth int, bar *progressbar.ProgressBar) (*datanode.DataNode, error) {
+		return datanode.New(), nil
+	}
+	defer func() {
+		website.DownloadAndPackageWebsite = oldDownloadAndPackageWebsite
+	}()
+
+	rootCmd := NewRootCmd()
+	rootCmd.AddCommand(collectCmd)
+
+	_, err := executeCommand(rootCmd, "collect", "website", "https://example.com", "--output", "/dev/null")
+	if err != nil {
+		t.Fatalf("collect website command failed: %v", err)
+	}
+}
+
+func TestCollectWebsiteCmd_Bad(t *testing.T) {
+	oldDownloadAndPackageWebsite := website.DownloadAndPackageWebsite
+	website.DownloadAndPackageWebsite = func(startURL string, maxDepth int, bar *progressbar.ProgressBar) (*datanode.DataNode, error) {
+		return nil, fmt.Errorf("website error")
+	}
+	defer func() {
+		website.DownloadAndPackageWebsite = oldDownloadAndPackageWebsite
+	}()
+
+	rootCmd := NewRootCmd()
+	rootCmd.AddCommand(collectCmd)
+
+	_, err := executeCommand(rootCmd, "collect", "website", "https://example.com", "--output", "/dev/null")
+	if err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+}

cmd/root.go

@@ -7,7 +7,6 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// NewRootCmd constructs the root cobra.Command for the Borg CLI and wires common flags.
 func NewRootCmd() *cobra.Command {
 	rootCmd := &cobra.Command{
 		Use:   "borg",

cmd/serve.go

@@ -62,7 +62,6 @@ var serveCmd = &cobra.Command{
 	},
 }
 
-// init registers the 'serve' command and its flags with the root command.
 func init() {
 	RootCmd.AddCommand(serveCmd)
 	serveCmd.PersistentFlags().String("port", "8080", "Port to serve the PWA on")

main.go

@@ -10,12 +10,7 @@ import (
 var osExit = os.Exit
 
 func main() {
-	verbose, _ := cmd.RootCmd.PersistentFlags().GetBool("verbose")
+	Main()
-	log := logger.New(verbose)
-	if err := cmd.Execute(log); err != nil {
-		log.Error("fatal error", "err", err)
-		osExit(1)
-	}
 }
 func Main() {
 	verbose, _ := cmd.RootCmd.PersistentFlags().GetBool("verbose")

pkg/datanode/datanode.go

@@ -260,35 +260,19 @@ type dataFile struct {
 	modTime time.Time
 }
 
-// Stat implements fs.File.Stat for a dataFile and returns its FileInfo.
 func (d *dataFile) Stat() (fs.FileInfo, error) { return &dataFileInfo{file: d}, nil }
-
-// Read implements fs.File.Read for a dataFile and reports EOF as files are in-memory.
 func (d *dataFile) Read(p []byte) (int, error) { return 0, io.EOF }
-
+func (d *dataFile) Close() error               { return nil }
-// Close implements fs.File.Close for a dataFile; it's a no-op.
-func (d *dataFile) Close() error { return nil }
 
 // dataFileInfo implements fs.FileInfo for a dataFile.
 type dataFileInfo struct{ file *dataFile }
 
-// Name returns the base name of the data file.
+func (d *dataFileInfo) Name() string       { return path.Base(d.file.name) }
-func (d *dataFileInfo) Name() string { return path.Base(d.file.name) }
+func (d *dataFileInfo) Size() int64        { return int64(len(d.file.content)) }
-
+func (d *dataFileInfo) Mode() fs.FileMode  { return 0444 }
-// Size returns the size of the data file in bytes.
-func (d *dataFileInfo) Size() int64 { return int64(len(d.file.content)) }
-
-// Mode returns the file mode for data files (read-only).
-func (d *dataFileInfo) Mode() fs.FileMode { return 0444 }
-
-// ModTime returns the modification time of the data file.
 func (d *dataFileInfo) ModTime() time.Time { return d.file.modTime }
-
+func (d *dataFileInfo) IsDir() bool        { return false }
-// IsDir reports whether the entry is a directory (false for data files).
+func (d *dataFileInfo) Sys() interface{}   { return nil }
-func (d *dataFileInfo) IsDir() bool { return false }
-
-// Sys returns system-specific data, which is nil for data files.
-func (d *dataFileInfo) Sys() interface{} { return nil }
 
 // dataFileReader implements fs.File for a dataFile.
 type dataFileReader struct {
@@ -296,18 +280,13 @@ type dataFileReader struct {
 	reader *bytes.Reader
 }
 
-// Stat returns the FileInfo for the underlying data file.
 func (d *dataFileReader) Stat() (fs.FileInfo, error) { return d.file.Stat() }
-
-// Read reads from the underlying data file content.
 func (d *dataFileReader) Read(p []byte) (int, error) {
 	if d.reader == nil {
 		d.reader = bytes.NewReader(d.file.content)
 	}
 	return d.reader.Read(p)
 }
-
-// Close closes the data file reader (no-op).
 func (d *dataFileReader) Close() error { return nil }
 
 // dirInfo implements fs.FileInfo for an implicit directory.
@@ -316,23 +295,12 @@ type dirInfo struct {
 	modTime time.Time
 }
 
-// Name returns the directory name.
+func (d *dirInfo) Name() string       { return d.name }
-func (d *dirInfo) Name() string { return d.name }
+func (d *dirInfo) Size() int64        { return 0 }
-
+func (d *dirInfo) Mode() fs.FileMode  { return fs.ModeDir | 0555 }
-// Size returns the size of the directory entry (always 0 for virtual dirs).
-func (d *dirInfo) Size() int64 { return 0 }
-
-// Mode returns the file mode for directories.
-func (d *dirInfo) Mode() fs.FileMode { return fs.ModeDir | 0555 }
-
-// ModTime returns the modification time of the directory.
 func (d *dirInfo) ModTime() time.Time { return d.modTime }
-
+func (d *dirInfo) IsDir() bool        { return true }
-// IsDir reports that the entry is a directory.
+func (d *dirInfo) Sys() interface{}   { return nil }
-func (d *dirInfo) IsDir() bool { return true }
-
-// Sys returns system-specific data, which is nil for dirs.
-func (d *dirInfo) Sys() interface{} { return nil }
 
 // dirFile implements fs.File for a directory.
 type dirFile struct {
@@ -340,15 +308,10 @@ type dirFile struct {
 	modTime time.Time
 }
 
-// Stat returns the FileInfo for the directory.
 func (d *dirFile) Stat() (fs.FileInfo, error) {
 	return &dirInfo{name: path.Base(d.path), modTime: d.modTime}, nil
 }
-
-// Read is invalid for directories and returns an error.
 func (d *dirFile) Read([]byte) (int, error) {
 	return 0, &fs.PathError{Op: "read", Path: d.path, Err: fs.ErrInvalid}
 }
-
-// Close closes the directory file (no-op).
 func (d *dirFile) Close() error { return nil }

pkg/github/github.go

@@ -107,7 +107,6 @@ func (g *githubClient) getPublicReposWithAPIURL(ctx context.Context, apiURL, use
 	return allCloneURLs, nil
 }
 
-// findNextURL parses the HTTP Link header and returns the URL with rel="next", if any.
 func (g *githubClient) findNextURL(linkHeader string) string {
 	links := strings.Split(linkHeader, ",")
 	for _, link := range links {

pkg/github/github_test.go

@@ -107,3 +107,18 @@ func TestFindNextURL(t *testing.T) {
 		t.Errorf("unexpected next URL: %s", nextURL)
 	}
 }
+
+func TestNewAuthenticatedClient(t *testing.T) {
+	// Test with no token
+	client := NewAuthenticatedClient(context.Background())
+	if client != http.DefaultClient {
+		t.Errorf("expected http.DefaultClient, but got something else")
+	}
+
+	// Test with token
+	t.Setenv("GITHUB_TOKEN", "test-token")
+	client = NewAuthenticatedClient(context.Background())
+	if client == http.DefaultClient {
+		t.Errorf("expected an authenticated client, but got http.DefaultClient")
+	}
+}

pkg/github/release_test.go

@@ -46,6 +46,9 @@ func TestParseRepoFromURL(t *testing.T) {
 }
 
 func TestGetLatestRelease(t *testing.T) {
+	oldNewClient := NewClient
+	t.Cleanup(func() { NewClient = oldNewClient })
+
 	mockClient := mocks.NewMockClient(map[string]*http.Response{
 		"https://api.github.com/repos/owner/repo/releases/latest": {
 			StatusCode: http.StatusOK,
@@ -115,6 +118,9 @@ func TestDownloadReleaseAsset_BadRequest(t *testing.T) {
 	}()
 
 	_, err := DownloadReleaseAsset(asset)
+	if err == nil {
+		t.Fatalf("expected error but got nil")
+	}
 	if err.Error() != expectedErr {
 		t.Fatalf("DownloadReleaseAsset failed: %v", err)
 	}
@@ -141,6 +147,9 @@ func TestDownloadReleaseAsset_NewRequestError(t *testing.T) {
 }
 
 func TestGetLatestRelease_Error(t *testing.T) {
+	oldNewClient := NewClient
+	t.Cleanup(func() { NewClient = oldNewClient })
+
 	u, _ := url.Parse("https://api.github.com/repos/owner/repo/releases/latest")
 	mockClient := mocks.NewMockClient(map[string]*http.Response{
 		"https://api.github.com/repos/owner/repo/releases/latest": {
@@ -183,3 +192,23 @@ func TestDownloadReleaseAsset_DoError(t *testing.T) {
 		t.Fatalf("DownloadReleaseAsset should have failed")
 	}
 }
+func TestParseRepoFromURL_More(t *testing.T) {
+	testCases := []struct {
+		url       string
+		owner     string
+		repo      string
+		expectErr bool
+	}{
+		{"git:github.com:owner/repo.git", "owner", "repo", false},
+	}
+
+	for _, tc := range testCases {
+		owner, repo, err := ParseRepoFromURL(tc.url)
+		if (err != nil) != tc.expectErr {
+			t.Errorf("unexpected error for URL %s: %v", tc.url, err)
+		}
+		if owner != tc.owner || repo != tc.repo {
+			t.Errorf("unexpected owner/repo for URL %s: %s/%s", tc.url, owner, repo)
+		}
+	}
+}

pkg/logger/logger.go

@@ -5,8 +5,6 @@ import (
 	"os"
 )
 
-// New constructs a slog.Logger configured for stderr with the given verbosity.
-// When verbose is true, the logger emits debug-level messages; otherwise info-level.
 func New(verbose bool) *slog.Logger {
 	level := slog.LevelInfo
 	if verbose {

pkg/pwa/pwa.go

@@ -99,6 +99,10 @@ func (p *pwaClient) DownloadAndPackagePWA(pwaURL, manifestURL string, bar *progr
 		}
 		defer resp.Body.Close()
 
+		if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+			return fmt.Errorf("failed to download %s: status code %d", assetURL, resp.StatusCode)
+		}
+
 		body, err := io.ReadAll(resp.Body)
 		if err != nil {
 			return fmt.Errorf("failed to read body of %s: %w", assetURL, err)
@@ -158,13 +162,14 @@ func (p *pwaClient) DownloadAndPackagePWA(pwaURL, manifestURL string, bar *progr
 			// Skip icons with bad URLs
 			continue
 		}
-		downloadAndAdd(iconURL.String())
+		if err := downloadAndAdd(iconURL.String()); err != nil {
+			return nil, err
+		}
 	}
 
 	return dn, nil
 }
 
-// resolveURL resolves ref against base and returns the absolute URL.
 func (p *pwaClient) resolveURL(base, ref string) (*url.URL, error) {
 	baseURL, err := url.Parse(base)
 	if err != nil {

pkg/pwa/pwa_test.go

@@ -8,7 +8,7 @@ import (
 	"github.com/schollz/progressbar/v3"
 )
 
-func newTestPWAClient(serverURL string) PWAClient {
+func newTestPWAClient() PWAClient {
 	return NewPWAClient()
 }
 
@@ -30,7 +30,7 @@ func TestFindManifest(t *testing.T) {
 	}))
 	defer server.Close()
 
-	client := newTestPWAClient(server.URL)
+	client := newTestPWAClient()
 	expectedURL := server.URL + "/manifest.json"
 	actualURL, err := client.FindManifest(server.URL)
 	if err != nil {
@@ -85,7 +85,7 @@ func TestDownloadAndPackagePWA(t *testing.T) {
 	}))
 	defer server.Close()
 
-	client := newTestPWAClient(server.URL)
+	client := newTestPWAClient()
 	bar := progressbar.New(1)
 	dn, err := client.DownloadAndPackagePWA(server.URL, server.URL+"/manifest.json", bar)
 	if err != nil {
@@ -131,3 +131,35 @@ func TestResolveURL(t *testing.T) {
 		}
 	}
 }
+
+func TestPWA_Bad(t *testing.T) {
+	client := NewPWAClient()
+
+	// Test FindManifest with no manifest
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/html")
+		w.Write([]byte(`
+			<!DOCTYPE html>
+			<html>
+			<head>
+				<title>Test PWA</title>
+			</head>
+			<body>
+				<h1>Hello, PWA!</h1>
+			</body>
+			</html>
+		`))
+	}))
+	defer server.Close()
+
+	_, err := client.FindManifest(server.URL)
+	if err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+
+	// Test DownloadAndPackagePWA with bad manifest
+	_, err = client.DownloadAndPackagePWA(server.URL, server.URL+"/manifest.json", nil)
+	if err == nil {
+		t.Fatalf("expected an error, but got none")
+	}
+}

pkg/tarfs/tarfs.go

@@ -67,23 +67,16 @@ type tarFile struct {
 	modTime time.Time
 }
 
-// Close implements http.File Close with a no-op for tar-backed files.
+func (f *tarFile) Close() error               { return nil }
-func (f *tarFile) Close() error { return nil }
-
-// Read implements io.Reader by delegating to the underlying bytes.Reader.
 func (f *tarFile) Read(p []byte) (int, error) { return f.content.Read(p) }
-
-// Seek implements io.Seeker by delegating to the underlying bytes.Reader.
 func (f *tarFile) Seek(offset int64, whence int) (int64, error) {
 	return f.content.Seek(offset, whence)
 }
 
-// Readdir is unsupported for files in the tar filesystem and returns os.ErrInvalid.
 func (f *tarFile) Readdir(count int) ([]os.FileInfo, error) {
 	return nil, os.ErrInvalid
 }
 
-// Stat returns a FileInfo describing the tar-backed file.
 func (f *tarFile) Stat() (os.FileInfo, error) {
 	return &tarFileInfo{
 		name:    path.Base(f.header.Name),
@@ -99,20 +92,9 @@ type tarFileInfo struct {
 	modTime time.Time
 }
 
-// Name returns the base name of the tar file.
+func (i *tarFileInfo) Name() string       { return i.name }
-func (i *tarFileInfo) Name() string { return i.name }
+func (i *tarFileInfo) Size() int64        { return i.size }
-
+func (i *tarFileInfo) Mode() os.FileMode  { return 0444 }
-// Size returns the size of the tar file in bytes.
-func (i *tarFileInfo) Size() int64 { return i.size }
-
-// Mode returns a read-only file mode for tar entries.
-func (i *tarFileInfo) Mode() os.FileMode { return 0444 }
-
-// ModTime returns the modification time recorded in the tar header.
 func (i *tarFileInfo) ModTime() time.Time { return i.modTime }
-
+func (i *tarFileInfo) IsDir() bool        { return false }
-// IsDir reports whether the entry is a directory (always false for files here).
+func (i *tarFileInfo) Sys() interface{}   { return nil }
-func (i *tarFileInfo) IsDir() bool { return false }
-
-// Sys returns underlying data source (unused for tar entries).
-func (i *tarFileInfo) Sys() interface{} { return nil }

pkg/ui/non_interactive_prompter.go

@@ -10,8 +10,6 @@ import (
 	"github.com/mattn/go-isatty"
 )
 
-// NonInteractivePrompter periodically prints status quotes when the terminal is non-interactive.
-// It is intended to give the user feedback during long-running operations.
 type NonInteractivePrompter struct {
 	stopChan  chan struct{}
 	quoteFunc func() (string, error)
@@ -20,8 +18,6 @@ type NonInteractivePrompter struct {
 	stopOnce  sync.Once
 }
 
-// NewNonInteractivePrompter returns a new NonInteractivePrompter that will call
-// the provided quoteFunc to retrieve text to display.
 func NewNonInteractivePrompter(quoteFunc func() (string, error)) *NonInteractivePrompter {
 	return &NonInteractivePrompter{
 		stopChan:  make(chan struct{}),
@@ -29,7 +25,6 @@ func NewNonInteractivePrompter(quoteFunc func() (string, error)) *NonInteractive
 	}
 }
 
-// Start begins the periodic display of quotes until Stop is called.
 func (p *NonInteractivePrompter) Start() {
 	p.mu.Lock()
 	if p.started {
@@ -64,7 +59,6 @@ func (p *NonInteractivePrompter) Start() {
 	}()
 }
 
-// Stop signals the prompter to stop printing further messages.
 func (p *NonInteractivePrompter) Stop() {
 	if p.IsInteractive() {
 		return
@@ -74,7 +68,6 @@ func (p *NonInteractivePrompter) Stop() {
 	})
 }
 
-// IsInteractive reports whether stdout is attached to an interactive terminal.
 func (p *NonInteractivePrompter) IsInteractive() bool {
 	return isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd())
 }

pkg/ui/progress_writer.go

@@ -1,19 +1,16 @@
+
 package ui
 
 import "github.com/schollz/progressbar/v3"
 
-// progressWriter implements io.Writer to update a progress bar with textual status.
 type progressWriter struct {
 	bar *progressbar.ProgressBar
 }
 
-// NewProgressWriter returns a writer that updates the provided progress bar's description.
 func NewProgressWriter(bar *progressbar.ProgressBar) *progressWriter {
 	return &progressWriter{bar: bar}
 }
 
-// Write updates the progress bar description with the provided bytes as a string.
-// It returns the length of p to satisfy io.Writer semantics.
 func (pw *progressWriter) Write(p []byte) (n int, err error) {
 	if pw == nil || pw.bar == nil {
 		return len(p), nil

pkg/ui/quote.go

@@ -16,7 +16,6 @@ var (
 	quotesErr    error
 )
 
-// init seeds the random number generator for quote selection.
 func init() {
 	rand.Seed(time.Now().UnixNano())
 }
@@ -42,7 +41,6 @@ type Quotes struct {
 	} `json:"image_related"`
 }
 
-// loadQuotes reads and unmarshals the embedded quotes.json file into a Quotes struct.
 func loadQuotes() (*Quotes, error) {
 	quotesFile, err := QuotesJSON.ReadFile("quotes.json")
 	if err != nil {
@@ -56,7 +54,6 @@ func loadQuotes() (*Quotes, error) {
 	return &quotes, nil
 }
 
-// getQuotes returns the cached Quotes, loading them once on first use.
 func getQuotes() (*Quotes, error) {
 	quotesOnce.Do(func() {
 		cachedQuotes, quotesErr = loadQuotes()
@@ -64,7 +61,6 @@ func getQuotes() (*Quotes, error) {
 	return cachedQuotes, quotesErr
 }
 
-// GetRandomQuote returns a random quote string from the combined quote sets.
 func GetRandomQuote() (string, error) {
 	quotes, err := getQuotes()
 	if err != nil {
@@ -86,7 +82,6 @@ func GetRandomQuote() (string, error) {
 	return allQuotes[rand.Intn(len(allQuotes))], nil
 }
 
-// PrintQuote prints a random quote to stdout in green for user feedback.
 func PrintQuote() {
 	quote, err := GetRandomQuote()
 	if err != nil {
@@ -97,7 +92,6 @@ func PrintQuote() {
 	c.Println(quote)
 }
 
-// GetVCSQuote returns a random quote related to VCS processing.
 func GetVCSQuote() (string, error) {
 	quotes, err := getQuotes()
 	if err != nil {
@@ -109,7 +103,6 @@ func GetVCSQuote() (string, error) {
 	return quotes.VCSProcessing[rand.Intn(len(quotes.VCSProcessing))], nil
 }
 
-// GetPWAQuote returns a random quote related to PWA processing.
 func GetPWAQuote() (string, error) {
 	quotes, err := getQuotes()
 	if err != nil {
@@ -121,7 +114,6 @@ func GetPWAQuote() (string, error) {
 	return quotes.PWAProcessing[rand.Intn(len(quotes.PWAProcessing))], nil
 }
 
-// GetWebsiteQuote returns a random quote related to website processing.
 func GetWebsiteQuote() (string, error) {
 	quotes, err := getQuotes()
 	if err != nil {

pkg/website/website.go

@@ -13,6 +13,8 @@ import (
 	"golang.org/x/net/html"
 )
 
+var DownloadAndPackageWebsite = downloadAndPackageWebsite
+
 // Downloader is a recursive website downloader.
 type Downloader struct {
 	baseURL     *url.URL
@@ -38,8 +40,8 @@ func NewDownloaderWithClient(maxDepth int, client *http.Client) *Downloader {
 	}
 }
 
-// DownloadAndPackageWebsite downloads a website and packages it into a DataNode.
+// downloadAndPackageWebsite downloads a website and packages it into a DataNode.
-func DownloadAndPackageWebsite(startURL string, maxDepth int, bar *progressbar.ProgressBar) (*datanode.DataNode, error) {
+func downloadAndPackageWebsite(startURL string, maxDepth int, bar *progressbar.ProgressBar) (*datanode.DataNode, error) {
 	baseURL, err := url.Parse(startURL)
 	if err != nil {
 		return nil, err
@@ -53,8 +55,6 @@ func DownloadAndPackageWebsite(startURL string, maxDepth int, bar *progressbar.P
 	return d.dn, nil
 }
 
-// crawl traverses the website starting from pageURL up to maxDepth,
-// storing pages and discovered local assets into the DataNode.
 func (d *Downloader) crawl(pageURL string, depth int) {
 	if depth > d.maxDepth || d.visited[pageURL] {
 		return
@@ -112,7 +112,6 @@ func (d *Downloader) crawl(pageURL string, depth int) {
 	f(doc)
 }
 
-// downloadAsset fetches a single asset URL and stores it into the DataNode.
 func (d *Downloader) downloadAsset(assetURL string) {
 	if d.visited[assetURL] {
 		return
@@ -139,7 +138,6 @@ func (d *Downloader) downloadAsset(assetURL string) {
 	d.dn.AddData(relPath, body)
 }
 
-// getRelativePath returns a path relative to the site's root for the given URL.
 func (d *Downloader) getRelativePath(pageURL string) string {
 	u, err := url.Parse(pageURL)
 	if err != nil {
@@ -148,7 +146,6 @@ func (d *Downloader) getRelativePath(pageURL string) string {
 	return strings.TrimPrefix(u.Path, "/")
 }
 
-// resolveURL resolves ref against base and returns the absolute URL string.
 func (d *Downloader) resolveURL(base, ref string) (string, error) {
 	baseURL, err := url.Parse(base)
 	if err != nil {
@@ -161,7 +158,6 @@ func (d *Downloader) resolveURL(base, ref string) (string, error) {
 	return baseURL.ResolveReference(refURL).String(), nil
 }
 
-// isLocal reports whether pageURL belongs to the same host as the base URL.
 func (d *Downloader) isLocal(pageURL string) bool {
 	u, err := url.Parse(pageURL)
 	if err != nil {
@@ -170,7 +166,6 @@ func (d *Downloader) isLocal(pageURL string) bool {
 	return u.Hostname() == d.baseURL.Hostname()
 }
 
-// isAsset reports whether the URL likely points to a static asset we should download.
 func isAsset(pageURL string) bool {
 	ext := []string{".css", ".js", ".png", ".jpg", ".jpeg", ".gif", ".svg", ".ico"}
 	for _, e := range ext {