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
Added --changed option to "brew tests" dev cmd #13158
Added --changed option to "brew tests" dev cmd #13158
Conversation
77ded1d
to
46ea73a
Compare
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.
Great work so far!
Library/Homebrew/dev-cmd/tests.rb
Outdated
raise UsageError, "The --only= argument requires a valid file or folder name!" if args.only | ||
|
||
if args.changed? | ||
opoo "Either no files have been changed from the master branch " \ | ||
"or no tests are associated with the changed files!" | ||
return | ||
end |
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.
raise UsageError, "The --only= argument requires a valid file or folder name!" if args.only | |
if args.changed? | |
opoo "Either no files have been changed from the master branch " \ | |
"or no tests are associated with the changed files!" | |
return | |
end | |
if args.only | |
raise UsageError, "The --only= argument requires a valid file or folder name!" | |
elsif args.changed? | |
opoo "Either no files have been changed from the master branch " \ | |
"or no tests are associated with the changed files!" | |
return | |
end |
Also: does the latter want to maybe be a UsageError
too? Wondering if any other cases need handled, too.
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.
I can't think of any other use cases. It would also be possible to split that last warning into two messages. If brew tests --changed
is called and no files have been changed, for whatever reason, that's probably a usage error. The other case where no tests directly correspond with the changed files isn't really a usage error imo.
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.
I decided to split those two up into a usage error and a warning. Also, the decision to use a guard clause was in response to the linter.
This option allows the user to run tests on all files that have been changed from the master branch.
46ea73a
to
b7d8822
Compare
Thanks again @apainintheneck! |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?This option allows the user to run tests on only the files that have been changed in comparison with the master branch. It slots in between the
brew tests
andbrew tests --only=filename
dev commands.Resolves #13146.