ReproNim / reproman

ReproMan (AKA NICEMAN, AKA ReproNim TRD3)

https://reproman.readthedocs.io

Other

24 stars 14 forks source link

Enh/launcher #577

Closed jbwexler closed 3 years ago

jbwexler commented 3 years ago

I ended up creating a new branch to keep it a bit cleaner. Continuation of https://github.com/ReproNim/reproman/pull/574

codecov[bot] commented 3 years ago

Codecov Report

Merging #577 (f29a3e0) into master (6c49cfe) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #577      +/-   ##
==========================================
+ Coverage   89.22%   89.23%   +0.01%     
==========================================
  Files         149      149              
  Lines       13054    13070      +16     
==========================================
+ Hits        11647    11663      +16     
  Misses       1407     1407

Impacted Files	Coverage Δ
reproman/interface/run.py	`100.00% <ø> (ø)`
reproman/support/jobs/orchestrators.py	`94.23% <100.00%> (+0.03%)`	:arrow_up:
reproman/support/jobs/tests/test_orchestrators.py	`94.72% <100.00%> (+0.11%)`	:arrow_up:

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 6c49cfe...535fda2. Read the comment docs.

jbwexler commented 3 years ago

It looked like pbs was the only other type of submitter that is supported by Launcher at the moment, so I only added launcher compatibility to slurm and pbs. https://github.com/TACC/launcher/tree/master/plugins

kyleam commented 3 years ago

Thanks for the update, @jbwexler. Looks good. I've pushed a fixup for the test failures and will look into adding a test that uses launcher.

kyleam commented 3 years ago

I said:

and will look into adding a test that uses launcher.

The new tests pass, but codecov flagged some of the new bits in test_orchestrators.py as unexecuted. Either the tests aren't working as I intended or coverage is confused. I'll first make sure that the new assertion is being executed locally and then I will push a temporary commit to this PR's branch to check that it's being triggered by the github workflow.

kyleam commented 3 years ago

then I will push a temporary commit to this PR's branch to check that it's being triggered by the github workflow.

The expected lines were triggered. I'll drop the tip commit and conclude that codecov was/is confused.

https://github.com/ReproNim/reproman/pull/577/checks?check_run_id=2040423024

jbwexler commented 3 years ago

No problem, thanks for taking the time to incorporate the PR!