print_info test and function
#71
Conversation
|
@sbillinge Can you review the test to see if you agree with the logic? I've outlined the logic above |
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
the test looks good. I think it could be lighter weight if we just reuse and extend the example_cases fixture.
news/printing-info.rst
Outdated
| @@ -0,0 +1,23 @@ | |||
| **Added:** | |||
|
|
|||
| * Added test for ``print_info`` function. | |||
There was a problem hiding this comment.
This can just say "added print_info function. " it is understood that it will have a test.
tests/conftest.py
Outdated
|
|
||
| @pytest.fixture(scope="function") | ||
| def temp_dir_print_info(tmp_path_factory): | ||
| root_temp_dir = tmp_path_factory.mktemp("temp") |
There was a problem hiding this comment.
is it not possible to reuse the example_cases fixture below? That is building a full temp dir. Don't we just need to add a few new things?
There was a problem hiding this comment.
Yeah we can, I'll do that 👍
|
@sbillinge reused |
|
much cleaner, thanks. |
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
one small cosmentic thing. Also it was failing tests, but I think you are working on it, right? Make sure you run tests locally before uploading
src/diffpy/cmi/packsmanager.py
Outdated
|
|
||
| def print_info(self) -> None: | ||
| """Print information about available packs and examples.""" | ||
| uninstall_packs = [] |
There was a problem hiding this comment.
I think this should be uninstalled_packs, right? noun vs verb.
There was a problem hiding this comment.
nice catch! thanks
|
@sbillinge Tests pass locally but fail on CI since we're pulling packages from envs (ie my local env must be different than CI's env). I'm trying to figure this out right now. It looks like its failing to find the actual requirments file... |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
+ Coverage 91.46% 92.39% +0.92%
==========================================
Files 3 3
Lines 164 184 +20
==========================================
+ Hits 150 170 +20
Misses 14 14
🚀 New features to boost your workflow:
|
|
@sbillinge Got tests passing 😅 ready for review |
|
This code looks really nice and clean, congrats! Extremely readable. I will merge. We should check that it passes on windows. The subprocess.run can sometimes be problematic on windows but this will be tested on the merge to main CI |
2 participants
The test works as follows:
packaginginto the temp envprint_inforuns and the outputs are compared against expected