👌 Improve and test github needservice directive
#1113
Conversation
- remove duplication of request mocking solutions (responses is more popular than requests-mock) - remove superfluous mocking in basic docs tests - add test for github `needservice` directives - replace raising`NeedGithubServiceException` with emitting `needs.github` warnings - pass the directive to the service (in a non-breaking manner) so that it can be used to add the directive source location to the warnings - Replace remaining tests that used `Sphinx` with `SphinxTestApp`, which can be "cleaned" after running (otherwise later test were emitting warnings due to these earlier tests)
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1113 +/- ##
==========================================
+ Coverage 83.93% 85.81% +1.88%
==========================================
Files 56 56
Lines 6462 6480 +18
==========================================
+ Hits 5424 5561 +137
+ Misses 1038 919 -119
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @pytest.mark.parametrize( | ||
| "test_app", [{"buildername": "html", "srcdir": "doc_test/doc_basic"}], indirect=True | ||
| ) | ||
| def test_build_html(test_app): | ||
| responses.add_callback( |
There was a problem hiding this comment.
Does removing this mean, our tests are firing now real requests to the GitHub API.
Or have I missed the locations, where this is still mocked?
There was a problem hiding this comment.
doc_test/doc_basic has no needservice directives in, i.e. there are no requests to make 😄
There was a problem hiding this comment.
argh sure, thought these removals had some functional reason.
But now it makes all sense...
By codecov, this directive was completely untested in the current test suite, so I added a test and also improved some other things I noticed:
needservicedirectivesNeedGithubServiceExceptionwith emittingneeds.githubwarningsservice.requestmethod (in a non-breaking manner) so that it can be used to add the directive source location to the warningsSphinxwithSphinxTestApp, which can be "cleaned" after running (otherwise later test were emitting warnings caused by global variables previously mutated by these earlier tests)