-
Notifications
You must be signed in to change notification settings - Fork 402
Restrict the root-only tests to ./storage #2891
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
5306dd0
to
d415557
Compare
What is the motivation? I had it like this originally before I moved the logic into the Mekefile but then it seemed to complicated for me. In particular how is this going to work like that, the makefile uses |
Great point about the (Originally this started as a hack for other reasons), but ultimately it’s a bit unclean, and a bit of a waste, that we test+compile all of those sub packages where we know the filter will not match anything. |
Yeah fair enough, if the compile time is noticeable then this seems like an easy win indeed. And I guess the reduced log spam is also nice |
At the point of making it even more complicated I think the best thing would be to add yet another variable like |
The filter already assumes that subpackage, by grepping for a package-private function name, anyway. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
d415557
to
f5c53d5
Compare
I’m not so worried about the compile / run time, with #2837 — if that ever lands. But “conceptually more precise”, “shorter logs” and “probably faster” seems worth doing, even if I started this for all the wrong reasons (to work around an unrelated bug in my work). |
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.
LGTM
For the record, tests are now 6.5–7 minutes, down from 8.5–10 minutes in the previous commit. |
The filter already assumes that subpackage, by grepping for a package-private function name, anyway.
Cc: @Luap99