search

ReproNim / reproman

ReproMan (AKA NICEMAN, AKA ReproNim TRD3)
https://reproman.readthedocs.io
Other
24 stars 14 forks source link

Add recursion to get and put for local and ssh sessions #544

Closed chaselgrove closed 3 years ago

chaselgrove commented 4 years ago

Closes #536

yarikoptic commented 4 years ago

Please add a test

codecov[bot] commented 4 years ago

Codecov Report

Merging #544 into master will increase coverage by 3.16%. The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #544      +/-   ##
==========================================
+ Coverage   86.31%   89.47%   +3.16%     
==========================================
  Files         149      149              
  Lines       12569    12621      +52     
==========================================
+ Hits        10849    11293     +444     
+ Misses       1720     1328     -392
Impacted Files Coverage Δ
reproman/resource/ssh.py 88.70% <75.00%> (+14.54%) :arrow_up:
reproman/resource/singularity.py 80.53% <85.71%> (+1.48%) :arrow_up:
reproman/interface/tests/test_run.py 98.54% <100.00%> (+0.07%) :arrow_up:
reproman/resource/session.py 89.96% <100.00%> (+0.58%) :arrow_up:
reproman/resource/shell.py 97.33% <100.00%> (+0.07%) :arrow_up:
reproman/resource/tests/test_session.py 99.55% <100.00%> (+5.06%) :arrow_up:
reproman/support/jobs/tests/test_orchestrators.py 95.31% <0.00%> (+0.78%) :arrow_up:
reproman/interface/execute.py 94.93% <0.00%> (+8.22%) :arrow_up:
... and 10 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 b82a412...bb98770. Read the comment docs.

chaselgrove commented 4 years ago

This tests a local resource. How would you test the remote (ssh) transfer?

kyleam commented 4 years ago

How would you test the remote (ssh) transfer?

This PR is about a session method, so I think most of the extensive testing should happen in reproman/resource/tests/test_session.py. That file already has some tests for get(), and I believe one of the cases is against a docker ssh container.

https://github.com/ReproNim/reproman/blob/b82a412ecccce33c885247dbd9464972b0e87c3f/reproman/resource/tests/test_session.py#L303

kyleam commented 3 years ago

The last build had three jobs stall in the test_conda phase. I tried with and without the changes here to trigger these again on Travis but was unable to. So perhaps they were just due to connectivity issues.

yarikoptic commented 3 years ago

Thank you @chaselgrove and @kyleam . From a brief look at the code -- looks good to me too.