abc-classroom icon indicating copy to clipboard operation
abc-classroom copied to clipboard

Tests for git / github module - adventures in patching and mocking

Open lwasser opened this issue 3 years ago • 2 comments

@nkorinek i'm just playing with some of the tests. i've added some notes and todos. we can talk about this more next week but some of the tests weren't running correctly for me locally such as git branc for _call_git. so i'm working through them. but generally i think what you setup is great and will work for us. we just need to think through exactly what we are testing.

NOTES:

  • [ ] updates code cov build to ensure pytest-subprocess is there
  • [ ] in many places there is a specific github3. exception thrown and we are not capturing the specific error and missing an opportuity to add a user friendly error there.
  • [ ] we are using both subprocess and github3.py i looked and don't see that pygithub or github3 have a clone wrapper which is surprising. maybe we can talk about this more or maybe i'm missing something.
    • [ ] FROM KAREN these tools only wrap GITHUB API calls NOT command line git commands - the git API is different / separate
  • [ ] we have to be careful with subprocess checks because the stout may vary with machine. for me i have a lot of branches locally so test 5 was failing. i changed to git status which i thikn throws a consistent first message of on branch ... but all sub process testing will be a bit tricky because of this.
  • [ ] i worked on one test at a time so some are commented out as i haven't gotten to them yet. lots to discuss but i think i understand how this works now.

I also noticed in many places we aren't capturing specific exceptions so we may want to talk about that more next week.


UPDATE I am not convinced that pytest.subprocess is doing what we want. it seems to work well when the subprocess is directly called but it was actually cloning on my computer. i found a great SO post that i linked to https://stackoverflow.com/questions/25692440/mocking-a-subprocess-call-in-python that uses unittest.mock. i was able to mock things successfully (i think) but need to look at it again with fresh eyes. there is one part i'm not sure about. i then captured the expected return standard out feedback from clone and tested against that. Do we also want to create a fixture (i started this) with a fake git repo and test that the repo was placed in the correct spot? it seems a bit contrived but i did start to create that fixture. We'd then want to use tmp_path fixture to avoid saving any files to a users directory.


Adding this resource https://github.com/pytest-dev/pytest/issues/4576 For a discussion on mock vs monkeypatch in unittest vs pytest.

lwasser avatar Oct 22 '21 00:10 lwasser

Codecov Report

Merging #445 (d994013) into main (4073701) will increase coverage by 10.34%. The diff coverage is 97.35%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #445       +/-   ##
===========================================
+ Coverage   50.44%   60.79%   +10.34%     
===========================================
  Files          22       22               
  Lines        1447     1584      +137     
===========================================
+ Hits          730      963      +233     
+ Misses        717      621       -96     
Flag Coverage Δ
unittests 60.79% <97.35%> (+10.34%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
abcclassroom/tests/test_config.py 100.00% <ø> (ø)
abcclassroom/tests/test_distribute.py 100.00% <ø> (ø)
abcclassroom/tests/test_github.py 97.53% <97.31%> (-2.47%) :arrow_down:
abcclassroom/config.py 78.57% <100.00%> (+10.00%) :arrow_up:
abcclassroom/github.py 72.15% <100.00%> (+42.58%) :arrow_up:
abcclassroom/distribute.py 0.00% <0.00%> (-35.00%) :arrow_down:

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 4073701...d994013. Read the comment docs.

codecov[bot] avatar Oct 22 '21 00:10 codecov[bot]

ALSO: why is code cov looking at coverage for a test file?

lwasser avatar Oct 22 '21 00:10 lwasser