-
-
Notifications
You must be signed in to change notification settings - Fork 765
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 --only-pending
CLI option
#3068
base: main
Are you sure you want to change the base?
Conversation
@@ -2196,15 +2219,27 @@ def run_suite_hooks(hook_description, hooks) | |||
end | |||
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.
Gathering the intersection of files here is where I found it getting messy. Specifically if there were ever to be more flags similar to these set, it might be worth extracting some of this functionality.
def configure_only_pending(options) | ||
options[:only_pending] = true | ||
add_tag_filter(options, :inclusion_filter, :last_run_status, 'pending') | ||
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.
These could probably be collapsed to a configure_only(options, :pending)
configure_only(options, :failures)
type call
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.
@JonRowe , as a tradeoff here, I'm supplying three arguments. This accounts for the difference between 'failures' and 'failed'. If the terms were the same like 'pending', then I could just construct the symbol like
options["only_#{type}"] = true
My other thought was some sort of constant mapping.
@@ -2196,15 +2219,27 @@ def run_suite_hooks(hook_description, hooks) | |||
end | |||
end | |||
|
|||
def get_files_to_run(paths) |
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.
Why the extra method here?
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.
@JonRowe , I was thinking the method #get_files_to_run was getting too large, so I wanted to break it up. Maybe that's premature, since nothing else calls #get_files.
As requested on issue #3066, this work adds the CLI option filter
--only-pending
. This is similar to--only-failures
.If
example_status_persistence_file_path
is not configured, it will fail with a message.If both this new flag and
--only-failures
are set, it will fail with a message.