feat: configurable regexp for coverage-ignore

[tino-alfaneti]

Enforce Explanatory Comments with // coverage-ignore. When a user adds a // coverage-ignore annotation, the tool should enforce that an additional explanatory comment is provided.

[vladopajic]

• feature should be tested with unit tests on all levels

  • `pkg/testcoverage/config_test.go``
    • TestConfigYamlParse
  • pkg/testcoverage/coverage/cover_test.go
    • new test that will check this functionality with different values used (similar to test Test_findAnnotations but in this test second argument of FindAnnotations must be used with different values)
  • pkg/testcoverage/check_test.go
    • a case in func TestCheck(t *testing.T) where non default value of IgnoreTextRegex is used

• IgnoreTextRegex does not seem suitable name why not CoverageIgnoreRegex ?

[vladopajic]

Enforce Explanatory Comments with // coverage-ignore. When a user adds a // coverage-ignore annotation, the tool should enforce that an additional explanatory comment is provided.

currently this pr just changes annotation from // coverage-ignore to whatever user wants. but this pr does not enforce users to add explanatory comments.

on the second toughs; i don't really think anyone needs to change annotations from // coverage-ignore to // ignore, for example. and in addition to this we should not let people change this because it introduces variation in standard unnecessarily. also it complicates repo code, again unnecessarily.

---

i am closing this pr because configurable regexp for coverage-ignore is not really desirable as per above.

[tino-alfaneti]

Enforce Explanatory Comments with // coverage-ignore. When a user adds a // coverage-ignore annotation, the tool should enforce that an additional explanatory comment is provided.

currently this pr just changes annotation from // coverage-ignore to whatever user wants. but this pr does not enforce users to add explanatory comments.

on the second toughs; i don't really think anyone needs to change annotations from // coverage-ignore to // ignore, for example. and in addition to this we should not let people change this because it introduces variation in standard unnecessarily. also it complicates repo code, again unnecessarily.

i am closing this pr because configurable regexp for coverage-ignore is not really desirable as per above.

I see what you mean. Let me look into it again and find a better solution. Thank you

[vladopajic]

@tino-alfaneti thanks for understand and sorry for lost effort.

---

if you want to work on feature that enforces comments here are hits in direction:

• config option should be bool: ForceAnnotationComment
• findAnnotations func shoud be like this

  func findAnnotations(source []byte, forceComment bool) (valid []extent, withoutComment []extent, error) {
       ...
       
       !if strings.Contains(c.Text(), IgnoreText) {
          continue // does not have annotation continue to next comment
       }
       if forceComment {
          if hasComment(c.Text()) { // does annotation have comment? "// coverage-ignore // can't test this edge case" <- true
             valid = append(valid, newExtent(fset, c)) // add current extent to valid (has comment)
          } else {
              withoutComment = append(withoutComment, newExtent(fset, c)) // current annotation does not have comment
          }
       } else {
         valid = append(valid, newExtent(fset, c)) // add current extent to valid 
       }
       
       ..
  }

  • note: findAnnotations must return withoutComment []extent because this will mark positions in the code where annotation was added but comment was not (when ForceAnnotationComment was used).
• coverageForFile should return Stats type with new property AnnotationsWithoutComments []int which will be set using withoutComment value (returned by findAnnotations).
• finally Stats.AnnotationsWithoutComments should be reported somwhere in ReportForHuman and ReportForGithubAction
  • if AnnotationsWithoutComments is not empty should it only report that and fail, without showing coverage statistic? seems okay imho, since person used ForceAnnotationComment feature.
• all changes should have tests
• code should pass lint check (make lint command)

[tino-alfaneti]

No worries at all @vladopajic , I believe that no effort is wasted, as every experience is an opportunity for learning and growth. I truly appreciate the hints and guidance you've provided.