Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

services/git:bugfix - error git diff breaking parse and invalid filepath #838

Merged
merged 1 commit into from
Dec 3, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions internal/services/formatters/service.go
Original file line number Diff line number Diff line change
@@ -143,10 +143,14 @@ func (s *Service) RemoveSrcFolderFromPath(path string) string {
if path == "" || len(path) <= 4 || !strings.Contains(path[:4], "src") {
return path
}

path = strings.Replace(filepath.ToSlash(path), "/src/", "", 1)

if runtime.GOOS == "windows" {
return strings.Replace(path, `src\`, "", 1)
return filepath.FromSlash(path)
}
return strings.Replace(path, "src/", "", 1)

return path
}

func (s *Service) GetCodeWithMaxCharacters(code string, column int) string {
7 changes: 6 additions & 1 deletion internal/services/formatters/service_test.go
Original file line number Diff line number Diff line change
@@ -423,9 +423,14 @@ func TestService_GetCodeWithMaxCharacters(t *testing.T) {
func TestRemoveSrcFolderFromPath(t *testing.T) {
t.Run("should return path without src prefix", func(t *testing.T) {
monitorController := NewFormatterService(&analysis.Analysis{}, testutil.NewDockerMock(), &config.Config{})
result := monitorController.RemoveSrcFolderFromPath(filepath.Join("src", "something"))
result := monitorController.RemoveSrcFolderFromPath(filepath.Join("/", "src", "something"))
assert.Equal(t, filepath.Base("something"), result)
})
t.Run("should return path without src prefix", func(t *testing.T) {
monitorController := NewFormatterService(&analysis.Analysis{}, testutil.NewDockerMock(), &config.Config{})
result := monitorController.RemoveSrcFolderFromPath(filepath.Join("/", "src", "something", "test"))
assert.Equal(t, filepath.Join("something", "test"), result)
})
t.Run("should return path without diff when src is after 4 index position", func(t *testing.T) {
monitorController := NewFormatterService(&analysis.Analysis{}, testutil.NewDockerMock(), &config.Config{})
result := monitorController.RemoveSrcFolderFromPath(filepath.Join("something", "src"))
16 changes: 7 additions & 9 deletions internal/services/git/git.go
Original file line number Diff line number Diff line change
@@ -80,10 +80,12 @@ func (g *Git) executeCMD(line, filePath string) ([]byte, error) {
stderr := bytes.NewBufferString("")

// NOTE: Here we use ^ as json double quotes to work properly on all platforms.
// The --no-patch flag suppress diff output, avoiding parse errors.
cmd := exec.Command(
"git",
"log",
"-1",
"--no-patch",
`--format={
^^^^^author^^^^^: ^^^^^%an^^^^^,
^^^^^email^^^^^:^^^^^%ae^^^^^,
@@ -110,13 +112,15 @@ func (g *Git) executeCMD(line, filePath string) ([]byte, error) {
}

func (g *Git) parseOutput(output []byte) (author commitauthor.CommitAuthor) {
output = g.getCleanOutput(output)
output = g.replaceCarets(output)

if err := json.Unmarshal(output, &author); err != nil {
logger.LogErrorWithLevel(
messages.MsgErrorGitCommitAuthorsParseOutput+string(output), err,
)
return g.newCommitAuthorNotFound()
}

return author
}

@@ -144,14 +148,8 @@ func (g *Git) parseLineStringToNumber(line string) string {
return strconv.Itoa(num)
}

func (g *Git) getCleanOutput(output []byte) []byte {
// Output from git log contains the diff changes
// so we need to extract only the json output data.
if idx := bytes.LastIndex(output, []byte("}")); idx >= 0 {
return bytes.ReplaceAll(output[:idx+1], []byte("^^^^^"), []byte(`"`))
}
logger.LogWarn(fmt.Sprintf("Could not to clean git blame output: %s", output))
return []byte("")
Comment on lines -147 to -154
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a closer look at the code and saw this function with some unnecessary stuff. Changed to just relace the carets.

func (g *Git) replaceCarets(output []byte) []byte {
return bytes.ReplaceAll(output, []byte("^^^^^"), []byte(`"`))
}

func (g *Git) existsGitFolderInPath() bool {
8 changes: 8 additions & 0 deletions internal/services/git/git_test.go
Original file line number Diff line number Diff line change
@@ -110,6 +110,14 @@ func TestGetCommitAuthor(t *testing.T) {
t.Run("Should return a new service", func(t *testing.T) {
assert.NotEmpty(t, New(&config.Config{}))
})

t.Run("Should not return git diff in output", func(t *testing.T) {
bytes, err := service.executeCMD("1", "README.md")

assert.NoError(t, err)
assert.NotEmpty(t, bytes)
assert.NotContains(t, string(bytes), "diff --git")
})
}

func TestRepositoryIsShallow(t *testing.T) {