github-annotations-with-api (#452)

Tufin · Dec 10, 2023 · 2410920 · 2410920
1 parent 0cc2ea9
commit 2410920
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 38 deletions.
diff --git a/checker/api_change.go b/checker/api_change.go
@@ -37,10 +37,10 @@ func (c ApiChange) MatchIgnore(ignorePath, ignoreLine string, l Localizer) bool
 	if ignorePath == "" {
 		return false
 	}
-	x := c.GetUncolorizedText(l)
+
 	return ignorePath == strings.ToLower(c.Path) &&
 		strings.Contains(ignoreLine, strings.ToLower(c.Operation+" "+c.Path)) &&
-		strings.Contains(ignoreLine, strings.ToLower(x))
+		strings.Contains(ignoreLine, strings.ToLower(c.GetUncolorizedText(l)))
 }
 
 func (c ApiChange) GetId() string {

diff --git a/formatters/format_githubactions.go b/formatters/format_githubactions.go
@@ -61,9 +61,7 @@ func (f GitHubActionsFormatter) RenderBreakingChanges(changes checker.Changes, o
 			params = append(params, "endLine="+strconv.Itoa(change.GetSourceLineEnd()+1))
 		}
 
-		// all annotated messages must be one-line, due to GitHub Actions limitations
-		message := strings.ReplaceAll(change.GetUncolorizedText(f.Localizer), "\n", "%0A")
-
+		message := fmt.Sprintf("at %s, in API %s %s %s", change.GetSource(), change.GetOperation(), change.GetPath(), change.GetUncolorizedText(f.Localizer))
 		buf.WriteString(fmt.Sprintf("::%s %s::%s\n", githubActionsSeverity[change.GetLevel()], strings.Join(params, ","), message))
 	}
 

diff --git a/formatters/format_githubactions_test.go b/formatters/format_githubactions_test.go
@@ -1,6 +1,7 @@
 package formatters_test
 
 import (
+	"net/http"
 	"os"
 	"testing"
 
@@ -22,62 +23,81 @@ func TestGithubActionsLookup(t *testing.T) {
 
 func TestGitHubActionsFormatter_RenderBreakingChanges_OneFailure(t *testing.T) {
 	testChanges := checker.Changes{
-		checker.ComponentChange{
-			Id:    "change_id",
-			Level: checker.ERR,
+		checker.ApiChange{
+			Id:        "change_id",
+			Level:     checker.ERR,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
 	}
 
 	// check output
 	output, err := gitHubFormatter.RenderBreakingChanges(testChanges, formatters.NewRenderOpts())
 	assert.NoError(t, err)
-	expectedOutput := "::error title=change_id::This is a breaking change.\n"
+	expectedOutput := "::error title=change_id::at openapi.yaml, in API GET /api/test This is a breaking change.\n"
 	assert.Equal(t, expectedOutput, string(output))
 }
 
 func TestGitHubActionsFormatter_RenderBreakingChanges_MultipleLevels(t *testing.T) {
 	testChanges := checker.Changes{
-		checker.ComponentChange{
-			Id:    "change_id",
-			Level: checker.ERR,
+		checker.ApiChange{
+			Id:        "change_id",
+			Level:     checker.ERR,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
-		checker.ComponentChange{
-			Id:    "warning_id",
-			Level: checker.WARN,
+		checker.ApiChange{
+			Id:        "warning_id",
+			Level:     checker.WARN,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
-		checker.ComponentChange{
-			Id:    "notice_id",
-			Level: checker.INFO,
+		checker.ApiChange{
+			Id:        "notice_id",
+			Level:     checker.INFO,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
 	}
 
 	// check output
 	output, err := gitHubFormatter.RenderBreakingChanges(testChanges, formatters.NewRenderOpts())
 	assert.NoError(t, err)
-	expectedOutput := "::error title=change_id::This is a breaking change.\n::warning title=warning_id::This is a warning.\n::notice title=notice_id::This is a notice.\n"
+	expectedOutput := "::error title=change_id::at openapi.yaml, in API GET /api/test This is a breaking change.\n::warning title=warning_id::at openapi.yaml, in API GET /api/test This is a warning.\n::notice title=notice_id::at openapi.yaml, in API GET /api/test This is a notice.\n"
 	assert.Equal(t, expectedOutput, string(output))
 }
 
 func TestGitHubActionsFormatter_RenderBreakingChanges_MultilineText(t *testing.T) {
+	t.Skip("messages should not contain \n so this case should never happen")
 	testChanges := checker.Changes{
-		checker.ComponentChange{
-			Id:    "change_two_lines_id",
-			Level: checker.ERR,
+		checker.ApiChange{
+			Id:        "change_two_lines_id",
+			Level:     checker.ERR,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
 	}
 
 	// check output
 	output, err := gitHubFormatter.RenderBreakingChanges(testChanges, formatters.NewRenderOpts())
 	assert.NoError(t, err)
-	expectedOutput := "::error title=change_two_lines_id::This is a breaking change.%0AThis is a second line.\n"
+	expectedOutput := "::error title=change_two_lines_id::at openapi.yaml, in API GET /api/test This is a breaking change.%0AThis is a second line.\n"
 	assert.Equal(t, expectedOutput, string(output))
 }
 
 func TestGitHubActionsFormatter_RenderBreakingChanges_FileLocation(t *testing.T) {
 	testChanges := checker.Changes{
-		checker.ComponentChange{
+		checker.ApiChange{
 			Id:              "change_id",
 			Level:           checker.ERR,
+			Operation:       http.MethodGet,
+			Path:            "/api/test",
+			Source:          "openapi.yaml",
 			SourceFile:      "openapi.json",
 			SourceLine:      20,
 			SourceLineEnd:   25,
@@ -89,7 +109,7 @@ func TestGitHubActionsFormatter_RenderBreakingChanges_FileLocation(t *testing.T)
 	// check output
 	output, err := gitHubFormatter.RenderBreakingChanges(testChanges, formatters.NewRenderOpts())
 	assert.NoError(t, err)
-	expectedOutput := "::error title=change_id,file=openapi.json,col=6,endColumn=11,line=21,endLine=26::This is a breaking change.\n"
+	expectedOutput := "::error title=change_id,file=openapi.json,col=6,endColumn=11,line=21,endLine=26::at openapi.yaml, in API GET /api/test This is a breaking change.\n"
 	assert.Equal(t, expectedOutput, string(output))
 }
 
@@ -101,29 +121,41 @@ func TestGitHubActionsFormatter_RenderBreakingChanges_JobOutputParameters(t *tes
 	_ = os.Setenv("GITHUB_OUTPUT", tempFile.Name())
 
 	testChanges := checker.Changes{
-		checker.ComponentChange{
-			Id:    "change_id",
-			Level: checker.ERR,
+		checker.ApiChange{
+			Id:        "change_id",
+			Level:     checker.ERR,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
-		checker.ComponentChange{
-			Id:    "change_id",
-			Level: checker.ERR,
+		checker.ApiChange{
+			Id:        "change_id",
+			Level:     checker.ERR,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
-		checker.ComponentChange{
-			Id:    "warning_id",
-			Level: checker.WARN,
+		checker.ApiChange{
+			Id:        "warning_id",
+			Level:     checker.WARN,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
-		checker.ComponentChange{
-			Id:    "notice_id",
-			Level: checker.INFO,
+		checker.ApiChange{
+			Id:        "notice_id",
+			Level:     checker.INFO,
+			Operation: http.MethodGet,
+			Path:      "/api/test",
+			Source:    "openapi.yaml",
 		},
 	}
 
 	// check output
 	output, err := gitHubFormatter.RenderBreakingChanges(testChanges, formatters.NewRenderOpts())
 	assert.NoError(t, err)
 	_ = os.Unsetenv("GITHUB_OUTPUT")
-	expectedOutput := "::error title=change_id::This is a breaking change.\n::error title=change_id::This is a breaking change.\n::warning title=warning_id::This is a warning.\n::notice title=notice_id::This is a notice.\n"
+	expectedOutput := "::error title=change_id::at openapi.yaml, in API GET /api/test This is a breaking change.\n::error title=change_id::at openapi.yaml, in API GET /api/test This is a breaking change.\n::warning title=warning_id::at openapi.yaml, in API GET /api/test This is a warning.\n::notice title=notice_id::at openapi.yaml, in API GET /api/test This is a notice.\n"
 	assert.Equal(t, expectedOutput, string(output))
 
 	// check job output parameters (NOTE: order of parameters is not guaranteed)