Merge pull request 'Improve code readability #1' (#3) from readability_refactor_1 into main

Reviewed-on: #3
This commit is contained in:
mpeczkowski 2024-06-16 13:59:06 +00:00
commit 9d7a64a371
10 changed files with 194 additions and 147 deletions

View File

@ -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
}

View File

@ -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
@ -52,8 +54,10 @@ func findNumberOfCommitDiffs(srcCommit *object.Commit, dstCommit *object.Commit)
return diffRange, false
}
baseCommitHashes := []string{}
destCommitHashes := []string{}
var baseCommitHashes []string
var 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

View File

@ -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)

View File

@ -1,6 +1,7 @@
package commands
import (
"errors"
"fmt"
"gitlab.com/revalus/grm/internal/config"
@ -26,15 +27,14 @@ const (
func fetchRepository(repo *git.Repository) (bool, error) {
err := repo.Fetch(&git.FetchOptions{})
if err == git.NoErrAlreadyUpToDate {
switch {
case errors.Is(err, git.NoErrAlreadyUpToDate):
return false, nil
}
if err != nil && err != git.NoErrAlreadyUpToDate {
return false, err
}
case errors.Is(err, git.NoErrAlreadyUpToDate):
return false, nil
default:
return true, nil
}
}
func cloneRepository(destPath string, repoCfg *config.RepositoryConfig) (bool, error) {
@ -62,21 +62,23 @@ func (s Synchronizer) Command(repoCfg config.RepositoryConfig) CommandStatus {
destPath := fmt.Sprintf("%v/%v", s.workspace, repoCfg.Dest)
repo, err := git.PlainOpen(destPath)
if err != nil && err == git.ErrRepositoryNotExists {
if err != nil {
if errors.Is(err, git.ErrRepositoryNotExists) {
cmdStatus.Changed, err = cloneRepository(destPath, &repoCfg)
cmdStatus.Message = syncCloned
} else if err == nil {
} else {
cmdStatus.Error = true
cmdStatus.Message = err.Error()
}
return cmdStatus
}
cmdStatus.Changed, err = fetchRepository(repo)
if cmdStatus.Changed {
cmdStatus.Message = syncFetched
} else {
cmdStatus.Message = syncUpToDate
}
} else {
cmdStatus.Error = true
cmdStatus.Message = err.Error()
return cmdStatus
}
if err != nil {
cmdStatus.Error = true

View File

@ -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")
}

View File

@ -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 == "" {

View File

@ -1,6 +1,7 @@
package config
import (
"errors"
"fmt"
"os"
"reflect"
@ -101,9 +102,9 @@ repositories:
`)
_, err := GetRepositoryConfig(exampleWrongYamlConfig, "yaml")
expectedError := fmt.Sprintf(errMissingSrcField, 0)
expectedError := errors.New(fmt.Sprintf(errMissingSrcField, 0))
if err.Error() != expectedError {
if errors.Is(err, expectedError) {
t.Errorf("Expected to get error with value %v, instead of this got: %v", expectedError, err.Error())
}
}
@ -127,7 +128,7 @@ repositories:
}
expectedError := getDuplicateFieldError("name", "example2", []int{1, 2})
if err.Error() != expectedError.Error() {
if errors.Is(err, expectedError) {
t.Errorf("Expected to get error with value %v, instead of this got: %v", expectedError.Error(), err.Error())
}
}
@ -151,7 +152,7 @@ repositories:
expectedError := getDuplicateFieldError("dest", "example", []int{1, 2})
if err.Error() != expectedError.Error() {
if errors.Is(err, expectedError) {
t.Errorf("Expected to get error with value \"%v\", instead of this got: \"%v\"", expectedError, err)
}
}

View File

@ -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}

View File

@ -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) {

View File

@ -29,7 +29,7 @@ func TestGetFileExtension(t *testing.T) {
}
func TestErrorInGetExcensionFile(t *testing.T) {
func TestErrorInGetExtensionFile(t *testing.T) {
result, err := getFileExtension("test")