-
Notifications
You must be signed in to change notification settings - Fork 1.4k
pkg/cover: allow paths to be excluded from stats #5804
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
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
23ff349 to
8be0bc6
Compare
|
Hi quic-likaid. I'd like to better understand your motivation. |
|
Hi @tarasmadan , We have a lot of modules enabled on an arm64 target , but only some of them are reachable due to hardware configuration. If we have following paths: Say we want to exclude With the patch it becomes: which is hopefully clearer. |
8be0bc6 to
8336cc9
Compare
|
re-created the commit to trigger CLA check. (failed again..) |
|
Does it mean you have your own, manually crafted subsystem definitions and you want to have more flexible syntax to describe them? |
Yes. |
8336cc9 to
f380f3d
Compare
f380f3d to
e970627
Compare
|
@jiangenj did you mean to push all those commits here or was it by mistake? |
e970627 to
0c9f0b4
Compare
Thanks, it's by mistake. |
0c9f0b4 to
02a17df
Compare
|
@quic-likaid still facing with the cla/google issue, perhaps it's your account setting. |
24cb834 to
679ffb7
Compare
tarasmadan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you plan to test it?
I did look at the existing tests before committing. It seems to me that the test ( I'm using the change in our internal setup, and it works fine. |
679ffb7 to
7e4c4eb
Compare
|
Added a new test. |
Head branch was pushed to by a user without write access
Some sub paths may not be covered due to hardware configuration, or lack
of interest. This patch allows them to be excluded from the stats. This
can be convenient if the excluded paths are deep in the hierarchy:
{
"name": "sound",
"path": [
"techpack/audio",
"-techpack/audio/asoc/aaa/bbb"
"-techpack/audio/asoc/aaa/ccc"
]
}
fe30bda to
1f53a76
Compare
|
Hi @tarasmadan , I've resolved a merge conflict. Would you please review? Thanks. |
|
Hi @tarasmadan, this PR stucks in "3 workflows awaiting approval". Would you let me know what other approvals I need to get? Thanks. |
Some sub paths may not be covered due to hardware configuration, or lack of interest. This patch allows them to be excluded from the stats. This can be convenient if the excluded paths are deep in the hierarchy:
Before sending a pull request, please review Contribution Guidelines:
https://github.com/google/syzkaller/blob/master/docs/contributing.md