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

BF: gitrepo.get_content_info: Do not list empty directories #3238

Merged
merged 2 commits into from Mar 20, 2019

Conversation

@kyleam
Copy link
Contributor

@kyleam kyleam commented Mar 18, 2019

When we pass --directory to ls-files, pass --no-empty-directory as
well so that we do not include directories that contain only excluded
files.

Fixes #3235.

@kyleam
Copy link
Contributor Author

@kyleam kyleam commented Mar 18, 2019

This breaks expectations elsewhere:

======================================================================
FAIL: datalad.core.local.tests.test_create.test_nested_create
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/tests/utils.py", line 607, in newfunc
    return t(*(arg + (filename,)), **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/core/local/tests/test_create.py", line 249, in test_nested_create
    assert_repo_status(ds.path, untracked=['lvl1/empty'])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/tests/utils.py", line 1539, in assert_repo_status
    % (state, state_files, oktobefound))
AssertionError: unexpected content of state "untracked": [] != [PosixPath('/tmp/datalad_temp_test_nested_create1ucrkmv7/lvl1/empty')]
======================================================================
FAIL: datalad.core.local.tests.test_status.test_status
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/tests/utils.py", line 607, in newfunc
    return t(*(arg + (filename,)), **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/tests/utils.py", line 607, in newfunc
    return t(*(arg + (filename,)), **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/core/local/tests/test_status.py", line 194, in test_status
    path=apath,
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/tests/utils.py", line 1250, in assert_result_count
    dumps(results, indent=1, default=lambda x: str(x))))
AssertionError: Got 0 instead of 1 expected results matching {'state': 'untracked', 'refds': '/tmp/datalad_temp_test_statusmic7_9az', 'path': '/tmp/datalad_temp_test_statusmic7_9az/subds_modified/subds_lvl1_modified/directory_untracked', 'type': 'directory'}. Inspected 0 record(s):
[]

@kyleam kyleam added the WIP label Mar 18, 2019
@kyleam kyleam force-pushed the status-no-empty-dir branch from f19aaca to bff33d5 Mar 18, 2019
@codecov
Copy link

@codecov codecov bot commented Mar 18, 2019

Codecov Report

Merging #3238 into master will decrease coverage by 9.8%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3238      +/-   ##
==========================================
- Coverage   91.03%   81.23%   -9.81%     
==========================================
  Files         260      260              
  Lines       34010    33996      -14     
==========================================
- Hits        30960    27615    -3345     
- Misses       3050     6381    +3331
Impacted Files Coverage Δ
datalad/support/gitrepo.py 70.07% <0%> (-19.43%) ⬇️
datalad/tests/utils.py 89.91% <100%> (-0.61%) ⬇️
datalad/core/local/tests/test_create.py 100% <100%> (ø) ⬆️
datalad/cmdline/tests/test_formatters.py 14.28% <0%> (-85.72%) ⬇️
datalad/plugin/addurls.py 18.32% <0%> (-81.37%) ⬇️
datalad/interface/rerun.py 18.48% <0%> (-77.73%) ⬇️
datalad/plugin/export_archive.py 24.65% <0%> (-75.35%) ⬇️
datalad/core/local/save.py 30.98% <0%> (-67.61%) ⬇️
datalad/core/local/status.py 33.68% <0%> (-64.22%) ⬇️
datalad/plugin/add_readme.py 29.31% <0%> (-63.8%) ⬇️
... and 85 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 af7265d...83e889e. Read the comment docs.

@kyleam kyleam removed the WIP label Mar 18, 2019
@kyleam
Copy link
Contributor Author

@kyleam kyleam commented Mar 18, 2019

This breaks expectations elsewhere:

I've updated those spots.

range-diff
-:  --------- > 1:  71c09fb65 TST: get_deeply_nested_structure: Add file to untracked directory
1:  f19aaca18 ! 2:  bff33d50f BF: gitrepo.get_content_info: Do not list empty directories
    @@ -8,6 +8,25 @@
     
         Fixes #3235.
     
    + diff --git a/datalad/core/local/tests/test_create.py b/datalad/core/local/tests/test_create.py
    + --- a/datalad/core/local/tests/test_create.py
    + +++ b/datalad/core/local/tests/test_create.py
    +@@
    +     with open(op.join(lvl2path, 'file'), 'w') as f:
    +         f.write('some')
    +     ok_(ds.rev_save())
    +-    assert_repo_status(ds.path, untracked=['lvl1/empty'])
    ++    # Empty directories are filtered out.
    ++    assert_repo_status(ds.path, untracked=[])
    +     # later create subdataset in a fresh dir
    +     # WINDOWS FAILURE IS NEXT LINE
    +     subds1 = ds.rev_create(op.join('lvl1', 'subds'))
    +-    assert_repo_status(ds.path, untracked=['lvl1/empty'])
    ++    assert_repo_status(ds.path, untracked=[])
    +     eq_(ds.subdatasets(result_xfm='relpaths'), [op.join('lvl1', 'subds')])
    +     # later create subdataset in an existing empty dir
    +     subds2 = ds.rev_create(op.join('lvl1', 'empty'))
    +
      diff --git a/datalad/support/gitrepo.py b/datalad/support/gitrepo.py
      --- a/datalad/support/gitrepo.py
      +++ b/datalad/support/gitrepo.py

@mih
Copy link
Member

@mih mih commented Mar 19, 2019

Rerunning the PY35 tests, they still show this issue.

@kyleam
Copy link
Contributor Author

@kyleam kyleam commented Mar 19, 2019

Rerunning the PY35 tests, they still show this issue.

Can you point me to those errors?

If I run the two failing tests locally with py3.5 and bff33d5, they pass for me.

For the most recent build, I see only the unrelated test__version__ failures and one related failing test in the revolution run that will need to be updated on -revolution's side.

@kyleam
Copy link
Contributor Author

@kyleam kyleam commented Mar 19, 2019

Travis is currently quiet, so I'll rebase this to get rid of the test__version__ errors.

kyleam added 2 commits Mar 19, 2019
The next commit will add --no-empty-directory to the ls-files call.
Add an untracked file to the directory so that `status` will care
about it.
When we pass --directory to ls-files, pass --no-empty-directory as
well so that we do not include directories that contain only excluded
files.

Fixes datalad#3235.
@kyleam kyleam force-pushed the status-no-empty-dir branch from bff33d5 to 83e889e Mar 19, 2019
@kyleam
Copy link
Contributor Author

@kyleam kyleam commented Mar 19, 2019

The only remaining failure on the latest Travis build was the -revolution run due to a spot in -revolution's test_run that needs to be updated for this change.

https://travis-ci.org/datalad/datalad/jobs/508408950#L1804

@mih
Copy link
Member

@mih mih commented Mar 20, 2019

I guess with #3251 we can just drop the failing test (and all related ones) on the -revolution end. I'll hit merge here and now.

@mih mih merged commit 0f0fafc into datalad:master Mar 20, 2019
2 of 3 checks passed
@kyleam kyleam deleted the status-no-empty-dir branch Mar 20, 2019
kyleam added a commit to kyleam/datalad that referenced this issue Mar 20, 2019
As of 83e889e (BF: gitrepo.get_content_info: Do not list empty
directories, 2019-03-18), .status() does not consider empty
directories to be untracked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants