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

Allow asv run to consume list of hashes from a file #768

Merged
merged 7 commits into from
Nov 27, 2018

Conversation

eteq
Copy link
Contributor

@eteq eteq commented Oct 29, 2018

This PR enables the listing of hashes asv should run on by providing a file. Currently the format is intentionally limited to "one hash per line". The user says they want to use said file by giving "HASHFILE:xxx.yyy" as the range parameter to asv run. While not strictly a "range", this seemed ok because "ANY", "EXISTING", etc were already options, so this seems not too much farther from "range" than those.

The use case for this is wanting to compare a specific hand-picked set of commits to so how performance on a particular feature evolves with major known (also probably #561, if I understand the suggestion there correctly). To try to also address the use case #562 is going for, I added "stdin" or "-" as options, in which case stdin is treated as the "hash file", although if that's too hacky I could drop that commit.

I know I could do one of the tricks suggested in #562 instead of this, but those are a lot harder to grok, and also seem to preclude the use of some of the nice multi-commit features of asv run like --parallel or --interleave-processes.

Happy to add docs/etc if need be - just "testing the waters" for now, as I found it quite frustrating to not have an easy way to just run on a list of known hashes.

Closes #562

Copy link
Collaborator

@pv pv left a comment

Choose a reason for hiding this comment

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

Looks mainly ok.

Needs docs at least in the option help= text, and you can add it to test_workflow.py:test_run_spec.

Note also the CI currently has sporadic failures in some tests due to some conda issues unrelated to this.

asv/commands/run.py Outdated Show resolved Hide resolved
asv/commands/run.py Outdated Show resolved Hide resolved
@eteq
Copy link
Contributor Author

eteq commented Nov 27, 2018

Thanks @pv - I've pushed up a commit that I think addresses your inline review comments and adds to the test you suggested as well as some docs describing this feature in the range argument as well as the "using.rst" file.

@pv pv merged commit 986b091 into airspeed-velocity:master Nov 27, 2018
@pv
Copy link
Collaborator

pv commented Nov 27, 2018

thanks, merged

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.

2 participants