datalad icon indicating copy to clipboard operation
datalad copied to clipboard

test_repo_save fails on crippled FS

Open bpoldrack opened this issue 3 years ago • 4 comments

Failure discovered in PR #5440. Windows build only and apparently unrelated to adjusted branches. Might be that there's something wrong with get_convoluted_situation on Windows.

======================================================================
FAIL: datalad.support.tests.test_repo_save.test_gitrepo_save_all
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python39-x64\lib\site-packages\nose\case.py", line 198, in runTest
    self.test(*self.arg)
  File "C:\Python39-x64\lib\site-packages\datalad\tests\utils.py", line 757, in _wrap_with_tempfile
    return t(*(arg + (filename,)), **kw)
  File "C:\Python39-x64\lib\site-packages\datalad\support\tests\test_repo_save.py", line 72, in test_gitrepo_save_all
    _test_save_all(path, GitRepo)
  File "C:\Python39-x64\lib\site-packages\datalad\support\tests\test_repo_save.py", line 64, in _test_save_all
    assert f.match('subds_modified'), f
AssertionError: C:\DLTMP\datalad_temp_ui79yjzi\subdir\subds_added
======================================================================
FAIL: datalad.support.tests.test_repo_save.test_annexrepo_save_all
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python39-x64\lib\site-packages\nose\case.py", line 198, in runTest
    self.test(*self.arg)
  File "C:\Python39-x64\lib\site-packages\datalad\tests\utils.py", line 757, in _wrap_with_tempfile
    return t(*(arg + (filename,)), **kw)
  File "C:\Python39-x64\lib\site-packages\datalad\support\tests\test_repo_save.py", line 79, in test_annexrepo_save_all
    _test_save_all(path, AnnexRepo)
  File "C:\Python39-x64\lib\site-packages\datalad\support\tests\test_repo_save.py", line 64, in _test_save_all
    assert f.match('subds_modified'), f
AssertionError: C:\DLTMP\datalad_temp_w22aspuy\subdir\subds_added

bpoldrack avatar Mar 04 '21 11:03 bpoldrack

I briefly looked into this. Maybe status is at fault. The failure arises because the only dataset components that should be modified at this state have subds_modified as a part of their name. On Linux, this is true:

❱ datalad status                
 modified: subdir/subds_modified (dataset)
 modified: subds_modified (dataset)

On Windows it is not true - it sees quite a bunch of datasets as being modified:

C:\Users\datalad\AppData\Local\Temp\datalad_temp_q_6xom6q>datalad status --untracked all
 modified: subdir\subds_added (dataset)         <- not seen by git as modified
 modified: subdir\subds_modified (dataset)
 modified: subdir\subds_untracked (dataset)     <- not seen by git as modified
 modified: subds_added (dataset)           <- not seen by git as modified
 modified: subds_modified (dataset)
 modified: subds_untracked (dataset)       <- not seen by git as modified

C:\Users\datalad\AppData\Local\Temp\datalad_temp_q_6xom6q>datalad status subdir/subds_added
nothing to save, working tree clean

Many of them are not seen as modified by Git:

C:\Users\datalad\AppData\Local\Temp\datalad_temp_q_6xom6q>git st
On branch adjusted/dl-test-branch(unlocked)
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)
        modified:   subdir/subds_clean (new commits)
        modified:   subdir/subds_modified (modified content)
        modified:   subds_clean (new commits)
        modified:   subds_modified (modified content, untracked content)

This does not happen on Linux.

adswa avatar Oct 26 '21 07:10 adswa

I think that difference is intended, as datalad status is supposed to give a seemless status across subdatasets (if -d wasn't specified), whereas git status doesn't consider status within a submodule, but the status of it wrt to the superproject.

However, this part: modified: subdir/subds_clean (new commits) in git status is interesting. I still think it's get_convoluted_situation that at some point doesn't manage to properly account for windows/adjusted branches.

To clarify:

What I think is intended is the difference between git and datalad status, not the difference between linux and windows. Point being, that this doesn't look to me as if it's status really that's at fault here, but the created situation itself.

bpoldrack avatar Oct 26 '21 09:10 bpoldrack

However, this part: modified: subdir/subds_clean (new commits) in git status is interesting. I still think it's get_convoluted_situation that at some point doesn't manage to properly account for windows/adjusted branches.

No, this is normal. Its the adjusted branch commit on top of the registered subproject commit that leads Git to regard it as a modification. status adjusts for this.

adswa avatar Oct 26 '21 10:10 adswa

This issue is categorized, and the resulting test disabling wrong. The failure can also be triggered when running the test on a crippled FS (tools/eval_under_testloopfs).

Here is the result when running on 0.17.2 and Debian:

[0.17.2] % tools/eval_under_testloopfs python -m pytest -s -v --pyargs datalad/support/tests/test_repo_save.py::test_gitrepo_save_all
...
=================================================== FAILURES ====================================================
_____________________________________________ test_gitrepo_save_all _____________________________________________

path = '/tmp/datalad-fs-3eU6f/datalad_temp_test_gitrepo_save_allyi3nigi1'

    @slow  # 11sec on travis
    @known_failure_windows  # see gh-5462
    @with_tempfile
    def test_gitrepo_save_all(path=None):
>       _test_save_all(path, GitRepo)

datalad/support/tests/test_repo_save.py:70: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

path = '/tmp/datalad-fs-3eU6f/datalad_temp_test_gitrepo_save_allyi3nigi1'
repocls = <class 'datalad.support.gitrepo.GitRepo'>

    def _test_save_all(path, repocls):
        ds = get_convoluted_situation(path, GitRepo)
        orig_status = ds.repo.status(untracked='all')
        # TODO test the results when the are crafted
        res = ds.repo.save()
        # make sure we get a 'delete' result for each deleted file
        eq_(
            set(r['path'] for r in res if r['action'] == 'delete'),
            {k for k, v in orig_status.items() if k.name == 'file_deleted'}
        )
        saved_status = ds.repo.status(untracked='all')
        # we still have an entry for everything that did not get deleted
        # intentionally
        eq_(
            len([f for f, p in orig_status.items()
                 if not f.match('*_deleted')]),
            len(saved_status))
        # everything but subdataset entries that contain untracked content,
        # or modified subsubdatasets is now clean, a repo simply doesn touch
        # other repos' private parts
        for f, p in saved_status.items():
            if p.get('state', None) != 'clean':
>               assert f.match('subds_modified'), f
E               AssertionError: PosixPath('/tmp/datalad-fs-3eU6f/datalad_temp_test_gitrepo_save_allyi3nigi1/subdir/subds_added')
E               assert False
E                +  where False = <bound method PurePath.match of PosixPath('/tmp/datalad-fs-3eU6f/datalad_temp_test_gitrepo_save_allyi3nigi1/subdir/subds_added')>('subds_modified')
E                +    where <bound method PurePath.match of PosixPath('/tmp/datalad-fs-3eU6f/datalad_temp_test_gitrepo_save_allyi3nigi1/subdir/subds_added')> = PosixPath('/tmp/datalad-fs-3eU6f/datalad_temp_test_gitrepo_save_allyi3nigi1/subdir/subds_added').match

datalad/support/tests/test_repo_save.py:62: AssertionError
============================================ short test summary info ============================================
FAILED datalad/support/tests/test_repo_save.py::test_gitrepo_save_all - AssertionError: PosixPath('/tmp/datala...
============================================== 1 failed in 21.45s ===============================================

Same behavior on master as of 1c8944cfc9f360057db9c9bc693be7238edfec06

Another reasons to pay attention to #6873

mih avatar Jul 24 '22 07:07 mih