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

Add DefaultIgnoreFunctionSet #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sywhang
Copy link
Contributor

@sywhang sywhang commented Jul 24, 2024

This exposes a variable DefaultIgnoreFunctionSet, which can be used to set a default list of functions to ignore for goleak.

Note that this mechanism is intended for library or tool authors who cannot directly call Ignore* Options to set up how their consumers are running the tests. They should be able to use linkname to set this variable to some well-known list of goroutines that leak if they really want to exempt their goroutines from getting continously flagged by goleak.

Fixes #119.

This exposes a variable `DefaultIgnoreFunctionSet`, which can be used
to set a default list of functions to ignore for goleak.

Note that this mechanism is intended for library or tool authors who
cannot directly call Ignore* Options to set up how their consumers are
running the tests. They should be able to use linkname to set this
variable to some well-known list of goroutines that leak if they really
want to exempt their goroutines from getting continously flagged by
goleak.

Fixes uber-go#119.
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.17%. Comparing base (898a938) to head (974cb98).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #127      +/-   ##
==========================================
+ Coverage   99.14%   99.17%   +0.02%     
==========================================
  Files           5        5              
  Lines         234      242       +8     
==========================================
+ Hits          232      240       +8     
  Misses          1        1              
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fmeum
Copy link

fmeum commented Jul 24, 2024

I would still consider this useful, but since bazel-contrib/rules_go@0f1e0ae, rules_go doesn't need it anymore. Cc @linzhp

@abhinav
Copy link
Collaborator

abhinav commented Jul 24, 2024

They should be able to use linkname to set this variable to some well-known list of goroutines that leak if they really want to exempt their goroutines from getting continously flagged by goleak.

Will that still be possible following golang/go#67401?

// by their consumers.
// Unless you are in such a situation, you should be using [IgnoreAnyFunction],
// [IgnoreCurrent], or [IgnoreTopFunction] instead.
var DefaultIgnoreFunctionSet string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk with making this an exported variable here is that libraries can't easily cooperate on setting this. It'll take one library to do goleak.DefaultIgnoreFunctionSet = "foo" to override other exceptions.

@sywhang
Copy link
Contributor Author

sywhang commented Jul 24, 2024

since rules_go no longer needs this, I think it's better to repurpose this feature specifically for library authors and expose a top-level function to append to ignorelist. Will work on that change and fix this PR

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

Successfully merging this pull request may close these issues.

default ignorelist entry for rules_go SIGTERM handler
3 participants