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

rerun: Adjusted branch tweaks #5328

Merged
merged 10 commits into from
Jan 15, 2021
Merged

rerun: Adjusted branch tweaks #5328

merged 10 commits into from
Jan 15, 2021

Conversation

kyleam
Copy link
Contributor

@kyleam kyleam commented Jan 13, 2021

This series was prompted by gh-4335. It tweaks rerun and several tests in test_rerun.py to be compatible with adjusted branches in scenarios that don't involve --onto and --branch. The remaining scenarios are skipped, and rerun now aborts with an informative message.

test_rerun.py no longer shows any failures when I execute it under tools/eval_under_testloopfs.

Prefer eq_() over plain assert to get a more informative failure
message.
test_rerun_ambiguous_revision_file() uses `branch` when it calls
rerun(), but that's not an essential piece of the test.  With an
upcoming commit, rerun() will abort when `branch` is used with an
adjusted branch, so drop the use of `branch` so that this test is
still executed for file systems where git-annex defaults to an
adjusted branch.
Lots of spots in test_rerun look at the commit message of the last
commit, but that's of course not appropriate when on a _synced_
adjusted branch (and save() syncs the branch after each call).

Import and use the last_commit_msg() helper from test_run, which
inspects DEFAULT_BRANCH (i.e. the corresponding branch for these test
setups).
A spot in test_run_inputs_outputs() resets to a few commits back, but
doing so isn't strictly required for the test, and 'git reset' does
not play well with adjusted branches.

Drop the 'git reset' call and adjust the downstream assertion
accordingly.
For a synced adjusted state, HEAD doesn't mean the same thing as it
does in the adjusted state.  Use DEFAULT_BRANCH instead.
Several spots in test_rerun default to using what is effectively HEAD,
and, for a synced adjusted state, HEAD doesn't mean the same thing as
it does in the adjusted state.  Use DEFAULT_BRANCH instead.
Using --onto with rerun() doesn't work on adjusted branch, and an
upcoming commit will make rerun() aborts in this case.  The rerun()
merge functionality depends on selecting a new base with --onto, so
skip all of these tests on file systems where git-annex defaults to
using an adjusted branch.
The revision for rerun() defaults to HEAD, meaning to rerun the
command from last commit or, if since is given, rerun the commands in
since..HEAD.  But on an adjusted branch, the corresponding branch is
more likely to be appropriate than HEAD, especially because changes
are propagated back to the corresponding branch each save().
rerun() depends on being able to check out revisions and replay old
commits for --onto and to create a new branch for --branch.  On a file
system where git-annex sets annex.crippledfilesystem to true, doing so
is not conceptually straightforward and would require additional
guards and special handling.  Even on file systems where
annex.crippledfilesystem is _not_ set to true, being on an adjusted
branch would require some care (e.g., not allowing the created branch
to land on an adjusted branch and verifying that the range does not
include an adjusted branch commit).

Regardless of whether any of the above limitations are seen as worth
fixing, rerun() should make the current situation clear to the user.
@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #5328 (8be4694) into master (0b26f63) will increase coverage by 0.01%.
The diff coverage is 97.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5328      +/-   ##
==========================================
+ Coverage   90.35%   90.37%   +0.01%     
==========================================
  Files         300      300              
  Lines       41911    41929      +18     
==========================================
+ Hits        37870    37892      +22     
+ Misses       4041     4037       -4     
Impacted Files Coverage Δ
datalad/interface/tests/test_rerun.py 99.61% <94.73%> (-0.39%) ⬇️
datalad/interface/rerun.py 96.74% <100.00%> (+0.12%) ⬆️
datalad/interface/tests/test_rerun_merges.py 100.00% <100.00%> (ø)
datalad/downloaders/base.py 81.05% <0.00%> (+0.35%) ⬆️
datalad/downloaders/tests/test_http.py 91.68% <0.00%> (+1.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b26f63...8be4694. Read the comment docs.

Copy link
Member

@mih mih left a comment

Choose a reason for hiding this comment

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

Great! Thx @kyleam. I will now have another look on actual routine operation of rerun on Windows ;-)

@mih mih merged commit cc850d1 into datalad:master Jan 15, 2021
@kyleam kyleam deleted the rerun-adjusted branch January 15, 2021 14:32
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.

None yet

2 participants