From 9e8ccfe78e80feb07c235b88471bdbacff151b46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20P=C4=99czkowski?= Date: Thu, 13 Jun 2024 21:19:04 +0200 Subject: [PATCH] Improve code readability #1 Improve mostly common_utils_test.go --- internal/commands/common_utils_test.go | 132 +++++++++++-------------- internal/commands/status_cmd.go | 10 +- internal/commands/status_cmd_test.go | 115 +++++++++++++++------ internal/commands/sync_cmd_test.go | 9 +- internal/config/config_file_parse.go | 6 +- internal/grm/app.go | 12 +-- internal/grm/app_test.go | 4 +- 7 files changed, 166 insertions(+), 122 deletions(-) diff --git a/internal/commands/common_utils_test.go b/internal/commands/common_utils_test.go index d58cf29..505f9be 100644 --- a/internal/commands/common_utils_test.go +++ b/internal/commands/common_utils_test.go @@ -2,8 +2,8 @@ package commands import ( "fmt" - "gitlab.com/revalus/grm/internal/config" "os" + "path" "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/osfs" @@ -13,12 +13,9 @@ import ( "github.com/go-git/go-git/v5/storage/filesystem" ) -type TestSetup struct { - rootFS billy.Filesystem - baseRepository struct { - fileSystem billy.Filesystem - repo *git.Repository - } +type TestSetupPaths struct { + baseTestDirectory string + baseTestRepository string } func checkErrorDuringPreparation(err error) { @@ -28,111 +25,94 @@ func checkErrorDuringPreparation(err error) { } } -func createTmpDir() string { - - baseForTMPDir := fmt.Sprintf("%v/grmTest", os.TempDir()) - if _, ok := os.Stat(baseForTMPDir); ok != nil { - err := os.Mkdir(baseForTMPDir, 0777) +func createRepositoryForTest() string { + systemTMPDirectoryWithTestPath := fmt.Sprintf("%v/grmTest", os.TempDir()) + if _, ok := os.Stat(systemTMPDirectoryWithTestPath); ok != nil { + err := os.Mkdir(systemTMPDirectoryWithTestPath, 0777) checkErrorDuringPreparation(err) } - - tempDir, err := os.MkdirTemp(baseForTMPDir, "*") + temporaryDirPath, err := os.MkdirTemp(systemTMPDirectoryWithTestPath, "*") checkErrorDuringPreparation(err) - - return tempDir + return temporaryDirPath } -func getTestSetup() TestSetup { - - tmpDir := createTmpDir() - - baseFileSystem := osfs.New(tmpDir) - - initRepositoryFileSystem, err := baseFileSystem.Chroot("worktree") +// prepareRepositoryDirectories - prepare directories for file (rootRepositoryDirectory) and git metadata (gitMetadataDirectory) +func prepareRepositoryDirectories(dirName string) (billy.Filesystem, billy.Filesystem) { + rootRepositoryDirectory := osfs.New(dirName) + gitMetadataDirectory, err := rootRepositoryDirectory.Chroot(".git") checkErrorDuringPreparation(err) - directoryForGitMetadata, err := initRepositoryFileSystem.Chroot(".git") + return rootRepositoryDirectory, gitMetadataDirectory +} + +func prepareBasicRepository() TestSetupPaths { + + temporaryDirPath := createRepositoryForTest() + + // Create an interface of abstraction over filesystem to provide tests over multiple systems + // baseTestsDirectory - provides to main directory where new directories might be created + baseTestsDirectory := osfs.New(temporaryDirPath) + rootRepositoryDirectory, gitMetadataDirectory := prepareRepositoryDirectories(baseTestsDirectory.Root() + "/base_repository") + + repository, err := git.Init(filesystem.NewStorage( + gitMetadataDirectory, + cache.NewObjectLRUDefault()), + rootRepositoryDirectory, + ) checkErrorDuringPreparation(err) - repository, err := git.Init(filesystem.NewStorage(directoryForGitMetadata, cache.NewObjectLRUDefault()), initRepositoryFileSystem) + testFile, err := rootRepositoryDirectory.Create("TestFile.txt") checkErrorDuringPreparation(err) - fileForFirstCommit, err := initRepositoryFileSystem.Create("TestFile.txt") - checkErrorDuringPreparation(err) - - _, err = fileForFirstCommit.Write([]byte("foo-conent")) + _, err = testFile.Write([]byte("foo-conent")) checkErrorDuringPreparation(err) repositoryWorkTree, err := repository.Worktree() checkErrorDuringPreparation(err) - repositoryWorkTree.Add(fileForFirstCommit.Name()) - _, err = repositoryWorkTree.Commit("First commit", &git.CommitOptions{}) - + _, err = repositoryWorkTree.Add(testFile.Name()) checkErrorDuringPreparation(err) - return TestSetup{ - baseRepository: struct { - fileSystem billy.Filesystem - repo *git.Repository - }{ - fileSystem: initRepositoryFileSystem, - repo: repository, - }, - rootFS: baseFileSystem, + _, err = repositoryWorkTree.Commit("First commit", &git.CommitOptions{}) + checkErrorDuringPreparation(err) + + return TestSetupPaths{ + baseTestDirectory: baseTestsDirectory.Root(), + baseTestRepository: rootRepositoryDirectory.Root(), } } func makeCommit(wk *git.Worktree, commitMessage string) { - _, err := wk.Commit(commitMessage, &git.CommitOptions{}) checkErrorDuringPreparation(err) - } -func getFSForLocalRepo(dirName string, baseFileSystem billy.Filesystem) (billy.Filesystem, *filesystem.Storage) { - fsForLocalRepo, err := baseFileSystem.Chroot(dirName) - checkErrorDuringPreparation(err) - fsForMetadata, err := fsForLocalRepo.Chroot(".git") - checkErrorDuringPreparation(err) - storageForTestRepo := filesystem.NewStorage(fsForMetadata, cache.NewObjectLRUDefault()) - return fsForLocalRepo, storageForTestRepo -} +// createAndCloneRepository - create sub-repository with cloned base repository required to verify sync command +func createAndCloneRepository(repositoryName string, paths TestSetupPaths) *git.Repository { -func getBaseForTestingSyncCommand() (StatusChecker, *git.Repository, config.RepositoryConfig, TestSetup) { - tmpDirWithInitialRepository := getTestSetup() - dirNameForLocalRepository := "testRepo" - fsForLocalRepo, storageForTestRepo := getFSForLocalRepo(dirNameForLocalRepository, tmpDirWithInitialRepository.rootFS) + baseGitRepository, gitMetadataDirectory := prepareRepositoryDirectories( + path.Join(paths.baseTestDirectory, repositoryName), + ) - fakeLocalRepository, err := git.Clone(storageForTestRepo, fsForLocalRepo, &git.CloneOptions{ - URL: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), + storageForSubRepository := filesystem.NewStorage(gitMetadataDirectory, cache.NewObjectLRUDefault()) + fakeLocalRepository, err := git.Clone(storageForSubRepository, baseGitRepository, &git.CloneOptions{ + URL: paths.baseTestRepository, }) checkErrorDuringPreparation(err) - - sc := StatusChecker{ - workspace: tmpDirWithInitialRepository.rootFS.Root(), - } - - repoCfg := config.RepositoryConfig{ - Name: "test", - Src: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), - Dest: dirNameForLocalRepository, - } - - return sc, fakeLocalRepository, repoCfg, tmpDirWithInitialRepository + return fakeLocalRepository } -func getBaseForTestingSyncMultipleRemote() (StatusChecker, *git.Repository, config.RepositoryConfig) { - sc, fakeLocalRepository, repoCfg, tmpDirWithInitialRepository := getBaseForTestingSyncCommand() +func addLocalRepositoryAsAFakeRemoteRepository(repository *git.Repository, baseTestRepositoryPath string) error { - fakeLocalRepository.CreateRemote(&gitcfg.RemoteConfig{ + _, err := repository.CreateRemote(&gitcfg.RemoteConfig{ Name: "subremote", - URLs: []string{tmpDirWithInitialRepository.baseRepository.fileSystem.Root()}, + URLs: []string{baseTestRepositoryPath}, }) - - fakeLocalRepository.Fetch(&git.FetchOptions{ + if err != nil { + return err + } + return repository.Fetch(&git.FetchOptions{ RemoteName: "subremote", }) - return sc, fakeLocalRepository, repoCfg } diff --git a/internal/commands/status_cmd.go b/internal/commands/status_cmd.go index b514878..e559dfb 100644 --- a/internal/commands/status_cmd.go +++ b/internal/commands/status_cmd.go @@ -6,6 +6,7 @@ import ( "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" "gitlab.com/revalus/grm/internal/config" + "path" "sort" ) @@ -20,7 +21,7 @@ func NewStatusChecker(workspace string) StatusChecker { } func findNumberOfCommitDiffs(srcCommit *object.Commit, dstCommit *object.Commit) int { - + // This function is a helper function to get only five latest items, based on given commit getFiveElementsFromHashes := func(commit *object.Commit, hashedSlice *[]string) *object.Commit { var err error @@ -36,6 +37,7 @@ func findNumberOfCommitDiffs(srcCommit *object.Commit, dstCommit *object.Commit) return commit } + // Compare diff between sources by hash list (the same hash list must be present to assume the end of changes) getRangeDiff := func(listFist, listSecond []string) (int, bool) { diffRange := 0 @@ -54,6 +56,8 @@ func findNumberOfCommitDiffs(srcCommit *object.Commit, dstCommit *object.Commit) baseCommitHashes := []string{} destCommitHashes := []string{} + + // Try to find all differences, limit only to five last changes to avoid reading whole repository at once for { if srcCommit != nil { @@ -80,8 +84,8 @@ func (sc StatusChecker) Command(repoCfg config.RepositoryConfig) CommandStatus { Error: false, } - destPath := fmt.Sprintf("%v/%v", sc.workspace, repoCfg.Dest) - repo, err := git.PlainOpen(destPath) + repositoryPath := path.Join(sc.workspace, repoCfg.Dest) + repo, err := git.PlainOpen(repositoryPath) if err != nil { cmdStatus.Error = true diff --git a/internal/commands/status_cmd_test.go b/internal/commands/status_cmd_test.go index a92e8a3..9bd3242 100644 --- a/internal/commands/status_cmd_test.go +++ b/internal/commands/status_cmd_test.go @@ -2,7 +2,10 @@ package commands import ( "fmt" + "github.com/go-git/go-git/v5/plumbing/cache" + "github.com/go-git/go-git/v5/storage/filesystem" "gitlab.com/revalus/grm/internal/config" + "path" "testing" "github.com/go-git/go-billy/v5/memfs" @@ -12,10 +15,10 @@ import ( ) func TestIfBranchesAreEqual(t *testing.T) { - tmpDirWithInitialRepository := getTestSetup() + pathsToTest := prepareBasicRepository() fakeLocalRepo, err := git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ - URL: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), + URL: pathsToTest.baseTestRepository, }) checkErrorDuringPreparation(err) @@ -42,9 +45,9 @@ func TestIfBranchesAreEqual(t *testing.T) { func TestIfCurrentBranchIsDifferent(t *testing.T) { - tmpDirWithInitialRepository := getTestSetup() + pathsToTest := prepareBasicRepository() fakeLocalRepo, err := git.Clone(memory.NewStorage(), memfs.New(), &git.CloneOptions{ - URL: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), + URL: pathsToTest.baseTestRepository, }) checkErrorDuringPreparation(err) @@ -85,28 +88,21 @@ func TestIfCurrentBranchIsDifferent(t *testing.T) { result = findNumberOfCommitDiffs(currentBranchCommit, remoteBranchCommit) if result != 15 { - t.Errorf("Expected to get 5 changes, instead of this got %v", result) + t.Errorf("Expected to get 15 changes, instead of this got %v", result) } } func TestCommandRepositoryDoesNotExists(t *testing.T) { - tmpDirWithInitialRepository := getTestSetup() - fsForLocalRepo, storageForTestRepo := getFSForLocalRepo("noMatterValue", tmpDirWithInitialRepository.rootFS) - - _, err := git.Clone(storageForTestRepo, fsForLocalRepo, &git.CloneOptions{ - URL: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), - }) - checkErrorDuringPreparation(err) - + pathsToTest := prepareBasicRepository() sc := StatusChecker{ - workspace: tmpDirWithInitialRepository.rootFS.Root(), + workspace: pathsToTest.baseTestDirectory, } repoCfg := config.RepositoryConfig{ Name: "test", - Src: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), - Dest: tmpDirWithInitialRepository.rootFS.Root(), + Src: pathsToTest.baseTestRepository, + Dest: pathsToTest.baseTestDirectory, } repoStatus := sc.Command(repoCfg) @@ -125,12 +121,15 @@ func TestCommandRepositoryDoesNotExists(t *testing.T) { func TestCommandRepositoryNoRemoteBranch(t *testing.T) { - tmpDirWithInitialRepository := getTestSetup() - dirNameForLocalRepository := "testRepo" - fsForLocalRepo, storageForTestRepo := getFSForLocalRepo(dirNameForLocalRepository, tmpDirWithInitialRepository.rootFS) + pathsToTest := prepareBasicRepository() + dirNameForLocalRepository := "sub-repository" + fsForLocalRepo, gitMetadataDirectory := prepareRepositoryDirectories( + path.Join(pathsToTest.baseTestDirectory, dirNameForLocalRepository), + ) + storageForTestRepo := filesystem.NewStorage(gitMetadataDirectory, cache.NewObjectLRUDefault()) fakeLocalRepository, err := git.Clone(storageForTestRepo, fsForLocalRepo, &git.CloneOptions{ - URL: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), + URL: pathsToTest.baseTestRepository, }) checkErrorDuringPreparation(err) @@ -138,12 +137,12 @@ func TestCommandRepositoryNoRemoteBranch(t *testing.T) { checkErrorDuringPreparation(err) sc := StatusChecker{ - workspace: tmpDirWithInitialRepository.rootFS.Root(), + workspace: pathsToTest.baseTestDirectory, } repoCfg := config.RepositoryConfig{ Name: "test", - Src: tmpDirWithInitialRepository.baseRepository.fileSystem.Root(), + Src: pathsToTest.baseTestRepository, Dest: dirNameForLocalRepository, } @@ -164,7 +163,21 @@ func TestCommandRepositoryNoRemoteBranch(t *testing.T) { func TestCommandAllCorrectWithoutChanges(t *testing.T) { - sc, _, repoCfg, _ := getBaseForTestingSyncCommand() + pathsToTest := prepareBasicRepository() + subRepositoryDirectoryName := "sub-repository" + + // Get new empty repository to compare with base repository + createAndCloneRepository(subRepositoryDirectoryName, pathsToTest) + + sc := StatusChecker{ + workspace: pathsToTest.baseTestDirectory, + } + + repoCfg := config.RepositoryConfig{ + Name: "test", + Src: pathsToTest.baseTestRepository, + Dest: subRepositoryDirectoryName, + } repoStatus := sc.Command(repoCfg) expectedMessage := "branch master - ( | origin | \u21910 \u21930 )" @@ -182,7 +195,19 @@ func TestCommandAllCorrectWithoutChanges(t *testing.T) { } func TestCommandAllCorrectWithOneChange(t *testing.T) { - sc, fakeLocalRepository, repoCfg, _ := getBaseForTestingSyncCommand() + pathsToTest := prepareBasicRepository() + subRepositoryDirectoryName := "sub-repository" + fakeLocalRepository := createAndCloneRepository(subRepositoryDirectoryName, pathsToTest) + + sc := StatusChecker{ + workspace: pathsToTest.baseTestDirectory, + } + + repoCfg := config.RepositoryConfig{ + Name: "test", + Src: pathsToTest.baseTestRepository, + Dest: subRepositoryDirectoryName, + } fakeLocalWorkTree, err := fakeLocalRepository.Worktree() checkErrorDuringPreparation(err) @@ -207,9 +232,26 @@ func TestCommandAllCorrectWithOneChange(t *testing.T) { } } -func TestCommandMultiRemoteNoChanges(t *testing.T) { +func TestCommandMultiRemotesNoChanges(t *testing.T) { + pathsToTest := prepareBasicRepository() + subRepositoryDirectoryName := "sub-repository" + fakeLocalRepository := createAndCloneRepository(subRepositoryDirectoryName, pathsToTest) + + sc := StatusChecker{ + workspace: pathsToTest.baseTestDirectory, + } + + repoCfg := config.RepositoryConfig{ + Name: "test", + Src: pathsToTest.baseTestRepository, + Dest: subRepositoryDirectoryName, + } + + err := addLocalRepositoryAsAFakeRemoteRepository(fakeLocalRepository, pathsToTest.baseTestRepository) + if err != nil { + t.Errorf("Unexpected error %v", err) + } - sc, _, repoCfg := getBaseForTestingSyncMultipleRemote() repoStatus := sc.Command(repoCfg) expectedMessage := "branch master - ( | origin | \u21910 \u21930 ) - ( | subremote | \u21910 \u21930 )" @@ -225,8 +267,25 @@ func TestCommandMultiRemoteNoChanges(t *testing.T) { } } -func TestCommandMultiRemoteWithOneChange(t *testing.T) { - sc, fakeLocalRepository, repoCfg := getBaseForTestingSyncMultipleRemote() +func TestCommandMultiRemotesWithOneChange(t *testing.T) { + pathsToTest := prepareBasicRepository() + subRepositoryDirectoryName := "sub-repository" + fakeLocalRepository := createAndCloneRepository(subRepositoryDirectoryName, pathsToTest) + + sc := StatusChecker{ + workspace: pathsToTest.baseTestDirectory, + } + + repoCfg := config.RepositoryConfig{ + Name: "test", + Src: pathsToTest.baseTestRepository, + Dest: subRepositoryDirectoryName, + } + + err := addLocalRepositoryAsAFakeRemoteRepository(fakeLocalRepository, pathsToTest.baseTestRepository) + if err != nil { + t.Errorf("Unexpected error %v", err) + } fakeLocalWorkTree, err := fakeLocalRepository.Worktree() checkErrorDuringPreparation(err) diff --git a/internal/commands/sync_cmd_test.go b/internal/commands/sync_cmd_test.go index 93bad1e..758e95d 100644 --- a/internal/commands/sync_cmd_test.go +++ b/internal/commands/sync_cmd_test.go @@ -16,14 +16,14 @@ func TestSyncInit(t *testing.T) { func TestSyncCommand(t *testing.T) { - testSetup := getTestSetup() + pathsToTest := prepareBasicRepository() sync := Synchronizer{ - workspace: testSetup.rootFS.Root(), + workspace: pathsToTest.baseTestDirectory, } cfg := config.RepositoryConfig{ - Src: fmt.Sprintf("file://%v", testSetup.baseRepository.fileSystem.Root()), + Src: fmt.Sprintf("file://%v", pathsToTest.baseTestRepository), Dest: "awesome-go", } @@ -32,10 +32,11 @@ func TestSyncCommand(t *testing.T) { t.Errorf("Unexpected error: %v", cloneStatus.Message) } - info, err := os.Stat(fmt.Sprintf("%v/awesome-go/.git", testSetup.rootFS.Root())) + info, err := os.Stat(fmt.Sprintf("%v/awesome-go/.git", pathsToTest.baseTestDirectory)) if err != nil { t.Errorf("Unexpected error: %v", err.Error()) } + if !info.IsDir() { t.Errorf("Expected that the selected path is dir") } diff --git a/internal/config/config_file_parse.go b/internal/config/config_file_parse.go index b875bf5..fd385ed 100644 --- a/internal/config/config_file_parse.go +++ b/internal/config/config_file_parse.go @@ -41,9 +41,9 @@ func GetRepositoryConfig(data []byte, fileExtension string) (Configuration, erro return Configuration{}, errors.New(errorMessage) } if repo.Name == "" { - splittedGit := strings.Split(repo.Src, "/") - nameWithExcention := splittedGit[len(splittedGit)-1] - name := strings.Split(nameWithExcention, ".")[0] + splitGit := strings.Split(repo.Src, "/") + nameWithExtension := splitGit[len(splitGit)-1] + name := strings.Split(nameWithExtension, ".")[0] config.Repositories[index].Name = name } if repo.Dest == "" { diff --git a/internal/grm/app.go b/internal/grm/app.go index 7d667af..5369e09 100644 --- a/internal/grm/app.go +++ b/internal/grm/app.go @@ -61,18 +61,18 @@ func (g *GitRepositoryManager) Run(w io.Writer) int { exitCode := 0 if len(g.cliArguments.LimitToTags) != 0 { - err := g.limitTags() + err := g.limitRepositoriesToTags() if err != nil { echo.ErrorfMsg(err.Error()) - exitCode = 1 + return 1 } } if g.cliArguments.LimitToName != "" { - err := g.limitName() + err := g.limitRepositoryToName() if err != nil { echo.ErrorfMsg(err.Error()) - exitCode = 1 + return 1 } } @@ -108,7 +108,7 @@ func describeStatus(status commands.CommandStatus) { } } -func (g *GitRepositoryManager) limitTags() error { +func (g *GitRepositoryManager) limitRepositoriesToTags() error { limitedTagsTmp := []config.RepositoryConfig{} for _, item := range g.configuration.Repositories { @@ -123,7 +123,7 @@ func (g *GitRepositoryManager) limitTags() error { return nil } -func (g *GitRepositoryManager) limitName() error { +func (g *GitRepositoryManager) limitRepositoryToName() error { for _, item := range g.configuration.Repositories { if g.cliArguments.LimitToName == item.Name { g.configuration.Repositories = []config.RepositoryConfig{item} diff --git a/internal/grm/app_test.go b/internal/grm/app_test.go index 8ee728c..b7f6c26 100644 --- a/internal/grm/app_test.go +++ b/internal/grm/app_test.go @@ -163,7 +163,7 @@ func TestLimitTags(t *testing.T) { } echo.Color(false) echo.Output(emt) - grm.limitTags() + grm.limitRepositoriesToTags() grm.runCommand(fakeCommand) } @@ -192,7 +192,7 @@ func TestLimitName(t *testing.T) { } echo.Color(false) echo.Output(emt) - grm.limitName() + grm.limitRepositoryToName() grm.runCommand(fakeCommand) } func TestRunWithNotExistingNameInLimit(t *testing.T) {