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

Sonar: Class members annotated with "@VisibleForTesting" should not be accessed from production code #923

Closed
sleberknight opened this issue Mar 24, 2023 · 0 comments · Fixed by #924
Assignees
Labels
code cleanup Fix issues reported by Sonar or any other code analysis tools
Milestone

Comments

@sleberknight
Copy link
Member

Sonar found a case where we are using a method annotated by @VisibleForTesting in production code, specifically two places in ProcessHelper which are calling the package-private Processes#getPidOrThrow method.

Sonar thinks this is critical but I think it's pretty minor, though I concur that it needs to be addressed.

However, the fix here is to simply remove the annotation, because the usages are perfectly valid within the same package.

@sleberknight sleberknight added the code cleanup Fix issues reported by Sonar or any other code analysis tools label Mar 24, 2023
@sleberknight sleberknight added this to the 2.6.0 milestone Mar 24, 2023
@sleberknight sleberknight self-assigned this Mar 24, 2023
sleberknight added a commit that referenced this issue Mar 24, 2023
Usage of this package-private method from ProcessHelper
in the same package is a valid usage, so removing the
VisibleForTesting annotation.

Closes #923
chrisrohr pushed a commit that referenced this issue Mar 24, 2023
Usage of this package-private method from ProcessHelper
in the same package is a valid usage, so removing the
VisibleForTesting annotation.

Closes #923
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Fix issues reported by Sonar or any other code analysis tools
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant