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

TST: set --allow-unrelated-histories in the mk_push_target setup for Windows #5855

Merged
merged 1 commit into from Aug 3, 2021

Conversation

adswa
Copy link
Member

@adswa adswa commented Aug 2, 2021

The test helper mk_push_target creates a sibling dataset by
creating an unrelated dataset and adding it as a sibling. On
Windows, this was not straightforward, as the datasets had
unrelated histories due to the initial adjusted branch commit
in the target dataset. Until git-annex version 20210720, a git
annex sync would establish a shared history - more recent versions
of git-annex will fail to sync unrelated histories unless
--allow-unrelated-histories is set.

With this patch, the tests that started to fail on Windows in #5811 pass again.

Fixes #5811.

@adswa adswa requested a review from yarikoptic August 2, 2021 09:07
@adswa adswa added the semver-internal Changes only affect the internal API label Aug 2, 2021
@codecov
Copy link

codecov bot commented Aug 2, 2021

Codecov Report

Merging #5855 (c59d3db) into maint (7f543d0) will decrease coverage by 60.74%.
The diff coverage is 32.27%.

❗ Current head c59d3db differs from pull request most recent head e21dde0. Consider uploading reports for the commit e21dde0 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##            maint    #5855       +/-   ##
===========================================
- Coverage   90.28%   29.54%   -60.75%     
===========================================
  Files         300      297        -3     
  Lines       42370    42338       -32     
===========================================
- Hits        38252    12507    -25745     
- Misses       4118    29831    +25713     
Impacted Files Coverage Δ
datalad/__main__.py 0.00% <0.00%> (-75.00%) ⬇️
datalad/cmdline/tests/test_helpers.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/cmdline/tests/test_main.py 0.00% <0.00%> (-98.62%) ⬇️
datalad/config.py 69.69% <0.00%> (-27.03%) ⬇️
datalad/consts.py 96.96% <ø> (-3.04%) ⬇️
datalad/core/distributed/tests/test_clone.py 0.00% <0.00%> (-97.24%) ⬇️
datalad/core/distributed/tests/test_push.py 0.00% <0.00%> (-99.12%) ⬇️
datalad/core/local/tests/test_create.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/core/local/tests/test_diff.py 0.00% <0.00%> (-99.54%) ⬇️
datalad/core/local/tests/test_save.py 0.00% <0.00%> (-96.71%) ⬇️
... and 395 more

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 7f543d0...e21dde0. Read the comment docs.

@adswa
Copy link
Member Author

adswa commented Aug 2, 2021

The test failure looks unrelated to me:

======================================================================
ERROR: datalad.tests.test_utils_cached_dataset.test_get_cached_dataset
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/tests/utils.py", line 189, in _wrap_skip_if_no_network
    return func(*args, **kwargs)
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/tests/utils.py", line 739, in _wrap_with_tempfile
    return t(*(arg + (filename,)), **kw)
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/tests/test_utils_cached_dataset.py", line 120, in test_get_cached_dataset
    ds = get_cached_dataset(url, version, keys)
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/tests/utils_cached_dataset.py", line 117, in get_cached_dataset
    for c in ds.repo.fetch(DEFAULT_REMOTE)):
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/support/gitrepo.py", line 1897, in fetch
    return list(
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/support/gitrepo.py", line 1908, in fetch_
    yield from self._fetch_push_helper(
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/support/gitrepo.py", line 2081, in _fetch_push_helper
    out = self._git_runner.run(
  File "/home/appveyor/dlvenv/lib/python3.8/site-packages/datalad/cmd.py", line 168, in run
    raise CommandError(
datalad.support.exceptions.CommandError: CommandError: 'git -c diff.ignoreSubmodules=none fetch --verbose --progress dl-test-remote' failed with exitcode 128 under /home/appveyor/DLTMP/datalad_temp_test_get_cached_datasetdvfszxbw/https___github.com_datalad_testrepo--minimalds [err: 'fatal: unable to access 'https://github.com/datalad/testrepo--minimalds/': Failed to connect to github.com port 443: Connection timed out']

@adswa adswa added platform-windows Issue concerned with Windows tests Add or improve existing tests labels Aug 2, 2021
@yarikoptic
Copy link
Member

I wonder if such a special treatment would only hide an issue which otherwise would eventually hit the users. I still think it should be cleared up with @joeyh since I still feel that it might be just a regression and will be fixed on annex side.

Some jobs stalled in our tests (not a good sign :-/ stalls happened recently in other PRs as well), restarted a 12259.1 on travis

@yarikoptic
Copy link
Member

oh, just spotted the comment you @adswa left regarding git-annex change http://source.git-annex.branchable.com/?p=source.git;a=commit;h=3d50b47dede6e4e94232718f4c460cd19147dcfb which exposes that option within git-annex and thus may be the change in behavior is intended. But then it seems it would be for us in probably create-sibling*s to handle/do whenever we do an initial sync call, not just in the tests

@adswa
Copy link
Member Author

adswa commented Aug 2, 2021

I wonder if such a special treatment would only hide an issue which otherwise would eventually hit the users

I wondered this, too. But this test setup is arguably a quite special case - creating a new dataset and adding it as a sibling to an existing dataset. And it is only on Windows where this involves unifying two unrelated histories. Maybe I don't have enough imagination, but I would think that a typical workflow would rather involve a clone with sibling/remote add or a create-sibling?

@adswa
Copy link
Member Author

adswa commented Aug 2, 2021

But then it seems it would be for us in probably create-sibling*s to handle/do whenever we do an initial sync call, not just in the tests

Yes, this may well be. But this PR here just wants to make this test helper functional again

@yarikoptic
Copy link
Member

And it is only on Windows where this involves unifying two unrelated histories.

isn't it for any remote which would be on a crippled FS (external usb stick), thus adjusted branches etc?
thought to check -- but git-annex seems to fail its own tests ATM on a regular and crippled FS on linux https://github.com/datalad/git-annex/actions/runs/1088733834 (but passes with crippled HOME, odd)

@adswa
Copy link
Member Author

adswa commented Aug 2, 2021

isn't it for any remote which would be on a crippled FS (external usb stick), thus adjusted branches etc?

oh yes, probably! Sorry, I keep forgetting about crippled FSs

@mih
Copy link
Member

mih commented Aug 2, 2021

... But then it seems it would be for us in probably create-sibling*s to handle/do whenever we do an initial sync call, not just in the tests

Is create-sibling now functional on windows? The reason for these test helpers was that datalad is not capable to do this on windows. I am all in favor of fixing the helpers, and leaving bigger plans aside.

Filed #5857 to make things a bit more obvious.

…Windows

The test helper mk_push_target creates a sibling dataset by
creating an unrelated dataset and adding it as a sibling. On
Windows, this was not straightforward, as the datasets had
unrelated histories due to the initial adjusted branch commit
in the target dataset. Until git-annex version 20210720, a git
annex sync would establish a shared history - more recent versions
of git-annex will fail to sync unrelated histories unless
--allow-unrelated-histories is set.
@adswa adswa changed the base branch from master to maint August 3, 2021 14:04
@adswa
Copy link
Member Author

adswa commented Aug 3, 2021

rebased to maint

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.

This looks good to me. Thx!

@mih
Copy link
Member

mih commented Aug 3, 2021

The failing linting is not the fault of this PR.

@mih mih merged commit 85e2e96 into datalad:maint Aug 3, 2021
@adswa adswa deleted the tst-pushtarget branch August 3, 2021 15:10
@yarikoptic yarikoptic removed the semver-internal Changes only affect the internal API label Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-windows Issue concerned with Windows tests Add or improve existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

a number of push tests (4?) started to fail at 20210720 on windows with some recent annex changes
3 participants