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

Proposal: Enforce Explanatory Comments with // coverage-ignore (similar to nolintlint) #173

Open
tino-alfaneti opened this issue Mar 15, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@tino-alfaneti
Copy link

The tool as it is now provides flexibility which is good, however, it can lead to unclear intentions when blocks of code are being deliberately ignored, without explanation for the next person who will look into that codebase. Similar to how golangci-lint's nolintlint feature requires an accompanying comment, I propose adding an optional feature to enforce that a descriptive comment must follow any // coverage-ignore.

Proposed Enhancement


Option 1 - Mandatory Comment Requirement

When a user adds a // coverage-ignore annotation, the tool should enforce that an additional explanatory comment is provided. For example:

  `// coverage-ignore // calls actual external API`

Option 2 - Configurable Enforcement

Allow users to toggle this enforcement on or off through a configuration flag ( maybe a command-line flag or configuration file option), much like nolintlint. This would let other developers adopt the feature gradually (or not at all).


For any option, it might be a plus to provide a way for users to specify custom patterns or expected keywords for the explanatory comment to better fit their project's guidelines.

Benefits

  1. Improved Code Transparency
    Enforcing an explanation when ignoring coverage helps maintain clarity about why certain lines or blocks of code are excluded from testing.

  2. Better Code Quality
    By requiring a justification, developers are encouraged to scrutinize each // coverage-ignore usage, reducing potential abuse and encouraging better testing practices.

  3. No Inconsistency Is Introduced :)
    This mimicks the behavior of other established tools like golangci-lint and creates a familiar pattern for developers, making the adoption of this tool easier and more standardized (arguably).

  4. Facilitates Code Reviews
    Clear reasons for coverage exclusions can make code reviews more effective, allowing reviewers to quickly understand any intentional omissions in testing.

@vladopajic
Copy link
Owner

hey @tino-alfaneti

thanks for making proposal. it very interesting and looks quite nice for go-test-coverage to have it.

some inputs:

  • defiantly have it as part of configuration so users can turn it on/off (off by default)
  • having "custom patterns or expected keywords" - i am not sure about this, for first draft it would be enough just to require string part after // coverage-ignore to no be empty.
  • report should include list of locations where comments are not provided (filename + line number)

feel free to contribute with PR. i can help to promptly review it.

@vladopajic vladopajic added the enhancement New feature or request label Mar 15, 2025
@tino-alfaneti
Copy link
Author

Thank you @vladopajic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants