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

Use configured branches when no commit/range is specified #430

Merged
merged 2 commits into from
Jun 10, 2016

Conversation

philpep
Copy link
Contributor

@philpep philpep commented Jun 8, 2016

Fix behavior of run, profiling and continuous commands that were using
get_hash_from_master() instead of configured branches when no commit was
specified.

In particular when only one branch was configured and was not "master",
asv tried to use the "master" branch.

Add (and refactor) specific test.

Closes #364
Alternative to #387

From 160s to 60s by running only one benchmark and reinstall a backup of
initial results and skipping existing commit (for ALL spec).

Also remove unneeded --skip-existing-{succesful,failed} in first commit
run.
@philpep philpep force-pushed the fix-no-spec-ignore-conf-branches branch from b7e237c to 6d56c44 Compare June 8, 2016 13:55
Fix behavior of run, profiling and continuous commands that were using
get_hash_from_master() instead of configured branches when no commit was
specified.

In particular when only one branch was configured and was not "master",
asv tried to use the "master" branch.

Add (and refactor) specific test.

Closes airspeed-velocity#364
@philpep philpep force-pushed the fix-no-spec-ignore-conf-branches branch from 6d56c44 to 0381e59 Compare June 8, 2016 13:56
@philpep philpep changed the title Use configured branches in run, profiling and continuous commands when no commit/range is specified Use configured branches when no commit/range is specified Jun 8, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 70.402% when pulling 0381e59 on philpep:fix-no-spec-ignore-conf-branches into c360f01 on spacetelescope:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 70.402% when pulling 0381e59 on philpep:fix-no-spec-ignore-conf-branches into c360f01 on spacetelescope:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 70.402% when pulling 0381e59 on philpep:fix-no-spec-ignore-conf-branches into c360f01 on spacetelescope:master.


assert set(result_files) == expected

for branches, expected_commits in (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style nitpick: I'd prefer to write multiline for assignments as

branches_and_commits = (
    ...
)
for branches, expected_commits in branches_and_commits:
...

@pv pv merged commit b432040 into airspeed-velocity:master Jun 10, 2016
@pv
Copy link
Collaborator

pv commented Jun 10, 2016

LGTM, merged.

@philpep philpep deleted the fix-no-spec-ignore-conf-branches branch June 15, 2016 07:10
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.

3 participants