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: Add test to ensure functionality with subdatasets starting with a hyphen (-) #6042

Merged
merged 1 commit into from Oct 6, 2021

Conversation

DisasterMo
Copy link
Contributor

Description

Just a test to make sure datalad remains compatible with subdatasets that start with a hyphen.

Checks:

  • create
  • clone
  • save
  • uninstall
  • get

Related Issues

Inspired by #5590

@mih mih added the tests Add or improve existing tests label Oct 6, 2021
@codecov
Copy link

codecov bot commented Oct 6, 2021

Codecov Report

Merging #6042 (0607ccd) into maint (64d6cbd) will decrease coverage by 55.01%.
The diff coverage is 0.00%.

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

@@             Coverage Diff             @@
##            maint    #6042       +/-   ##
===========================================
- Coverage   90.22%   35.20%   -55.02%     
===========================================
  Files         312      312               
  Lines       42157    42158        +1     
===========================================
- Hits        38036    14843    -23193     
- Misses       4121    27315    +23194     
Impacted Files Coverage Δ
datalad/local/tests/test_subdataset.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/wtf.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/addurls.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/tests/test_api.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/tests/test_cmd.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/no_annex.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/support/digests.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/tests/test_base.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/add_readme.py 0.00% <0.00%> (-100.00%) ⬇️
... and 264 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 64d6cbd...92d7853. Read the comment docs.

@mih
Copy link
Member

mih commented Oct 6, 2021

Looks good! Wanna take it out of draft mode?

Comment on lines +353 to +360

# get
ds_clone.get('-clone')
assert_true(dash_clone.is_installed())
assert_result_count(
ds_clone.subdatasets(), 1, path=dash_clone.path, state='present')

assert_repo_status(ds.path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't seem to serve any purpose, does it? I think this is a remnant of a previous version.
I'll just remove this, alright?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean the very last line.

Copy link
Member

@mih mih Oct 6, 2021

Choose a reason for hiding this comment

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

It checks whether any of the above operations have left the original dataset with unsaved modifications. It is certainly not criticial, but also not too expensive, and would make sure that nothing got unexpectedly left behind.

@DisasterMo DisasterMo marked this pull request as ready for review October 6, 2021 14:20
@yarikoptic
Copy link
Member

FWIW: although I don't mind a dedicated test, I wonder if in the long run we should use the "obscure filename" functionality/constant to ensure that everything (not just a sample of dedicated operations) works on such datasets.
ATM we have (on my laptop)

$> python -c 'from datalad.tests.utils import OBSCURE_FILENAME; print(repr(OBSCURE_FILENAME))' 
' |;&%b5{}\'"ΔЙקم๗あ .datc '

where it starts with the space -- we had issues with not proper quoting of invocations etc. But we could also make a CI run where it would start not with a space but with - -- if file system supports that. I have not reviewed filesystems' support for filenames to start with - and I expect them all to be sane, but experience says that the expectation of sanity is often violated (great to see that this PR is all green, so at least on those which we test seems to be ok) ;)

@mih
Copy link
Member

mih commented Oct 6, 2021

Good idea, but I would prefer to not multiplex too much into a single test. Here it is specifically about Git explicitly not being able to work with such a case. It is useful to know that we can deal with it, and that it stays this way. This is related to other obscure names, but not identical or incremental.

@mih mih added the release Create a release when this pr is merged label Oct 6, 2021
@mih mih merged commit 89fa0c8 into datalad:maint Oct 6, 2021
@yarikoptic
Copy link
Member

FWIW 0.15.2 release did happen!

@DisasterMo DisasterMo deleted the tst-subdataset-name branch October 7, 2021 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Create a release when this pr is merged tests Add or improve existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants