Repository: preslavmihaylov/todocheck
Pull request #164 (2021-08-29 17:05:20)

Extend JSON output to include linked issue & other metadata (#162)

preslavmihaylov: Could you also extend the testing framework to unmarshal the metadata field and make sure it's validated in the TODO error scenarios?
bengsparks: The newer commits now check for the `issueID` field in `metadata` part of the JSON output, and makes sure there is no such entry for malformed TODOs. @preslavmihaylov 

Patch:
diff --git a/checker/checker.go b/checker/checker.go
index fcfff85..053b570 100644
--- a/checker/checker.go
+++ b/checker/checker.go
@@ -49,9 +49,9 @@ func (c *Checker) Check(
 
 	switch status {
 	case taskstatus.Closed:
-		return checkererrors.IssueClosedErr(filename, lines, linecnt), nil
+		return checkererrors.IssueClosedErr(filename, lines, linecnt, taskID), nil
 	case taskstatus.NonExistent:
-		return checkererrors.IssueNonExistentErr(filename, lines, linecnt), nil
+		return checkererrors.IssueNonExistentErr(filename, lines, linecnt, taskID), nil
 	}
 
 	return nil, nil
diff --git a/checker/errors/errors.go b/checker/errors/errors.go
index f633b08..f8b2f3a 100644
--- a/checker/errors/errors.go
+++ b/checker/errors/errors.go
@@ -23,20 +23,23 @@ type TODO struct {
 	filename string
 	lines    []string
 	linecnt  int
+	metadata map[string]string
 }
 
 // ToJSON converts the todo error into json format
 func (err *TODO) ToJSON() ([]byte, error) {
 	res := &struct {
-		Type     string `json:"type"`
-		Filename string `json:"filename"`
-		Line     int    `json:"line"`
-		Message  string `json:"message"`
+		Type     string            `json:"type"`
+		Filename string            `json:"filename"`
+		Line     int               `json:"line"`
+		Message  string            `json:"message"`
+		Metadata map[string]string `json:"metadata"`
 	}{
 		Type:     string(err.errType),
 		Filename: err.filename,
 		Line:     err.linecnt,
 		Message:  "",
+		Metadata: err.metadata,
 	}
 
 	if err.errType == TODOErrTypeMalformed {
@@ -67,26 +70,33 @@ func MalformedTODOErr(filename string, lines []string, linecnt int) *TODO {
 		filename: filename,
 		lines:    lines,
 		linecnt:  linecnt,
+		metadata: make(map[string]string),
 	}
 }
 
 // IssueClosedErr when referenced todo issue is closed
-func IssueClosedErr(filename string, lines []string, linecnt int) *TODO {
+func IssueClosedErr(filename string, lines []string, linecnt int, issueID string) *TODO {
 	return &TODO{
 		errType:  TODOErrTypeIssueClosed,
 		filename: filename,
 		lines:    lines,
 		linecnt:  linecnt,
+		metadata: map[string]string{
+			"issueID": issueID,
+		},
 	}
 }
 
 // IssueNonExistentErr when referenced todo issue doesn't exist
-func IssueNonExistentErr(filename string, lines []string, linecnt int) *TODO {
+func IssueNonExistentErr(filename string, lines []string, linecnt int, issueID string) *TODO {
 	return &TODO{
 		errType:  TODOErrTypeNonExistentIssue,
 		filename: filename,
 		lines:    lines,
 		linecnt:  linecnt,
+		metadata: map[string]string{
+			"issueID": issueID,
+		},
 	}
 }
 


Test patch:
diff --git a/testing/scenariobuilder/todoerr_scenario.go b/testing/scenariobuilder/todoerr_scenario.go
index ee9d399..e99b43c 100644
--- a/testing/scenariobuilder/todoerr_scenario.go
+++ b/testing/scenariobuilder/todoerr_scenario.go
@@ -12,11 +12,14 @@ type TodoErrScenario struct {
 	sourceFile    string
 	sourceLineNum int
 	contents      []string
+	metadata      map[string]string
 }
 
 // NewTodoErr returns a new todo err scenario
 func NewTodoErr() *TodoErrScenario {
-	return &TodoErrScenario{}
+	return &TodoErrScenario{
+		metadata: map[string]string{},
+	}
 }
 
 // WithType specifies the expected todo err type for the given scenario
@@ -48,6 +51,21 @@ func (s *TodoErrScenario) ExpectLine(line string) *TodoErrScenario {
 	return s
 }
 
+// WithJSONMetadata extends existing metadata with a multiple of key value pairs
+// expected within the `metadata` field of the TODOs
+func (s *TodoErrScenario) WithJSONMetadata(metadata map[string]string) *TodoErrScenario {
+	for k, v := range metadata {
+		s.metadata[k] = v
+	}
+	return s
+}
+
+// WithJSONMetadataEntry stores a single key-value pair that is expected in the `metadata` field
+func (s *TodoErrScenario) WithJSONMetadataEntry(key string, value string) *TodoErrScenario {
+	s.metadata[key] = value
+	return s
+}
+
 func (s *TodoErrScenario) String() string {
 	str := fmt.Sprintf("ERROR: %s", s.errType)
 	for i := 0; i < len(s.contents); i++ {
@@ -62,10 +80,32 @@ func (s *TodoErrScenario) String() string {
 }
 
 type TodoErrForJSON struct {
-	Type     string `json:"type"`
-	Filename string `json:"filename"`
-	Line     int    `json:"line"`
-	Message  string `json:"message"`
+	Type     string            `json:"type"`
+	Filename string            `json:"filename"`
+	Line     int               `json:"line"`
+	Message  string            `json:"message"`
+	Metadata map[string]string `json:"metadata"`
+}
+
+func metadatasMatch(this, other *TodoErrForJSON) bool {
+	if len(this.Metadata) != len(other.Metadata) {
+		return false
+	}
+	for k, v := range this.Metadata {
+		if other.Metadata[k] != v {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (s *TodoErrForJSON) equals(other *TodoErrForJSON) bool {
+	return s.Type == other.Type &&
+		s.Filename == other.Filename &&
+		s.Line == other.Line &&
+		s.Message == other.Message &&
+		metadatasMatch(s, other)
 }
 
 func (s *TodoErrScenario) ToTodoErrForJSON() *TodoErrForJSON {
@@ -74,6 +114,7 @@ func (s *TodoErrScenario) ToTodoErrForJSON() *TodoErrForJSON {
 		Filename: s.sourceFile,
 		Line:     s.sourceLineNum,
 		Message:  "",
+		Metadata: s.metadata,
 	}
 
 	if s.errType == errors.TODOErrTypeMalformed {
diff --git a/testing/scenariobuilder/validations.go b/testing/scenariobuilder/validations.go
index be25481..a2d9048 100644
--- a/testing/scenariobuilder/validations.go
+++ b/testing/scenariobuilder/validations.go
@@ -61,7 +61,7 @@ func validateJSONTodoErrs(programOutput string, scenarios []*TodoErrScenario) va
 		for _, elem := range elements {
 			removeIdx := -1
 			for j, expected := range scenarioJSONObjs {
-				if elem == *expected {
+				if elem.equals(expected) {
 					removeIdx = j
 					break
 				}
diff --git a/testing/scenarios/annotated_todos/main.go b/testing/scenarios/annotated_todos/main.go
index 368eb8a..984166b 100644
--- a/testing/scenarios/annotated_todos/main.go
+++ b/testing/scenarios/annotated_todos/main.go
@@ -20,3 +20,6 @@ package main
  * TODO J456:
  * This issue doesn't exist
  */
+
+/* This is a malformed TODO:
+ */
diff --git a/testing/test_configs/no_issue_tracker.yaml b/testing/test_configs/no_issue_tracker.yaml
index 23f1629..4f21131 100644
--- a/testing/test_configs/no_issue_tracker.yaml
+++ b/testing/test_configs/no_issue_tracker.yaml
@@ -1,4 +1,4 @@
-origin: http://myjira.com
+origin: http://127.0.0.1:34335
 issue_tracker: JIRA
 auth: 
   type: none
diff --git a/testing/todocheck_test.go b/testing/todocheck_test.go
index fa04ace..8d22e65 100644
--- a/testing/todocheck_test.go
+++ b/testing/todocheck_test.go
@@ -156,6 +156,12 @@ func TestAnnotatedTodos(t *testing.T) {
 				ExpectLine(" * TODO J456:").
 				ExpectLine(" * This issue doesn't exist").
 				ExpectLine(" */")).
+		ExpectTodoErr(
+			scenariobuilder.NewTodoErr().
+				WithType(errors.TODOErrTypeMalformed).
+				WithLocation("scenarios/annotated_todos/main.go", 24).
+				ExpectLine("/* This is a malformed TODO:").
+				ExpectLine(" */")).
 		Run()
 	if err != nil {
 		t.Errorf("%s", err)
@@ -174,19 +180,27 @@ func TestAnnotatedTodosWithJSONOutput(t *testing.T) {
 		ExpectTodoErr(
 			scenariobuilder.NewTodoErr().
 				WithType(errors.TODOErrTypeIssueClosed).
-				WithLocation("scenarios/annotated_todos/main.go", 3)).
+				WithLocation("scenarios/annotated_todos/main.go", 3).
+				WithJSONMetadataEntry("issueID", "J123")).
 		ExpectTodoErr(
 			scenariobuilder.NewTodoErr().
 				WithType(errors.TODOErrTypeNonExistentIssue).
-				WithLocation("scenarios/annotated_todos/main.go", 7)).
+				WithLocation("scenarios/annotated_todos/main.go", 7).
+				WithJSONMetadataEntry("issueID", "J456")).
 		ExpectTodoErr(
 			scenariobuilder.NewTodoErr().
 				WithType(errors.TODOErrTypeIssueClosed).
-				WithLocation("scenarios/annotated_todos/main.go", 14)).
+				WithLocation("scenarios/annotated_todos/main.go", 14).
+				WithJSONMetadataEntry("issueID", "J123")).
 		ExpectTodoErr(
 			scenariobuilder.NewTodoErr().
 				WithType(errors.TODOErrTypeNonExistentIssue).
-				WithLocation("scenarios/annotated_todos/main.go", 19)).
+				WithLocation("scenarios/annotated_todos/main.go", 19).
+				WithJSONMetadataEntry("issueID", "J456")).
+		ExpectTodoErr(
+			scenariobuilder.NewTodoErr().
+				WithType(errors.TODOErrTypeMalformed).
+				WithLocation("scenarios/annotated_todos/main.go", 24)).
 		Run()
 	if err != nil {
 		t.Errorf("%s", err)