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

Discontinue usage of testrepos #6690

Merged
merged 5 commits into from
Jun 9, 2022
Merged

Conversation

bpoldrack
Copy link
Member

@bpoldrack bpoldrack commented May 10, 2022

Closes #6144

Changelog

🛡 Tests

  • Discontinue use of with_testrepos decorator other than for the deprecation cycle for nose

@codecov
Copy link

codecov bot commented May 10, 2022

Codecov Report

Merging #6690 (ada1778) into master (efc7d21) will increase coverage by 1.60%.
The diff coverage is 99.33%.

@@            Coverage Diff             @@
##           master    #6690      +/-   ##
==========================================
+ Coverage   88.82%   90.43%   +1.60%     
==========================================
  Files         353      353              
  Lines       45733    45802      +69     
==========================================
+ Hits        40622    41419     +797     
+ Misses       5111     4383     -728     
Impacted Files Coverage Δ
datalad/tests/test_tests_utils.py 92.34% <ø> (+92.34%) ⬆️
datalad/distribution/tests/test_install.py 99.81% <99.00%> (-0.19%) ⬇️
datalad/distribution/tests/test_create_sibling.py 78.37% <100.00%> (+0.37%) ⬆️
datalad/distribution/tests/test_get.py 100.00% <100.00%> (ø)
datalad/support/tests/test_annexrepo.py 97.75% <100.00%> (-0.13%) ⬇️
datalad/tests/utils_testdatasets.py 100.00% <100.00%> (ø)
datalad/tests/utils_testrepos.py 94.11% <100.00%> (ø)
datalad/customremotes/datalad.py 61.64% <0.00%> (-2.74%) ⬇️
datalad/tests/utils_pytest.py 89.72% <0.00%> (-0.50%) ⬇️
... and 22 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 84f979a...ada1778. Read the comment docs.

@bpoldrack bpoldrack force-pushed the rm-testrepos branch 2 times, most recently from 891956e to 78e96dc Compare May 18, 2022 22:36
@bpoldrack bpoldrack added the semver-tests Changes only affect tests, no impact on version label May 18, 2022
@bpoldrack bpoldrack marked this pull request as ready for review May 19, 2022 01:34
@bpoldrack
Copy link
Member Author

FTR: AppVeyor Failure is on win "no module named coverage". I consider this unrelated and it's after the actual test runs which are fine.

@yarikoptic yarikoptic added this to the 0.17.0 milestone May 24, 2022
Copy link
Member

@adswa adswa left a comment

Choose a reason for hiding this comment

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

I noticed a few inconsistent relative imports - is that intentional?

datalad/distribution/tests/test_create_sibling.py Outdated Show resolved Hide resolved
datalad/distribution/tests/test_install.py Outdated Show resolved Hide resolved
@with_testrepos(flavors=['network'])
def magical():
raise AssertionError("Must not be ran")
assert_raises(SkipTest, magical)
Copy link
Member

Choose a reason for hiding this comment

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

@yarikoptic would have appreciated to have this tested as long as we have debian packages built for extensions which still use @with_testrepos

Copy link
Member

Choose a reason for hiding this comment

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

From my POV the only extension using it is deprecated, which would also be the place to migrate this functionality to. @yarikoptic are you aware of other extensions using it?

Copy link
Member

Choose a reason for hiding this comment

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

I found none other. To be kosher should have gone through deprecation cycle for removal but since only for testing, indeed might be worth replacing with a stub right away requiring datalad-deprecated to be installed to use it. This PR doesn't need to be contingent on that

@codeclimate
Copy link

codeclimate bot commented Jun 8, 2022

Code Climate has analyzed commit ada1778 and detected 0 issues on this pull request.

View more on Code Climate.

@mih
Copy link
Member

mih commented Jun 8, 2022

This has been hanging here for too long, hence I made an attempt to unstuck it be addressing the previous comments: absolute imports, keeping the with_testrepo tests.

If the tests pass, this closes #6144 from my POV.

@yarikoptic
Copy link
Member

I believe my comments were addressed, I have no objections, thank you @bpoldrack - let's proceed and the rest (full deprecation) could be in a follow up PR if so desired

@yarikoptic yarikoptic merged commit 4d53077 into datalad:master Jun 9, 2022
@mih
Copy link
Member

mih commented Jun 9, 2022

Thank you!

@mih mih mentioned this pull request Jul 1, 2022
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-tests Changes only affect tests, no impact on version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fully discontinue the use of testrepos
5 participants