add changes #6
add changes #6
Conversation
CaptainOfHacks
commented
Feb 10, 2022
CaptainOfHacks
commented
- move e2e tests from unit tests
- Create FakeRequest for HTTP API tests
- Use dependecy injection for NoticeFetcher and TedDocumentSearch
- move e2e tests from unit tests - Create FakeRequest for HTTP API tests - Use dependecy injection for NoticeFetcher and TedDocumentSearch
Codecov Report
@@ Coverage Diff @@
## main #6 +/- ##
==========================================
+ Coverage 87.71% 88.06% +0.35%
==========================================
Files 18 18
Lines 236 243 +7
==========================================
+ Hits 207 214 +7
Misses 29 29
Continue to review full report at Codecov.
|
| documents_content += response_body["results"] | ||
| decoded_documents_content = [] | ||
| for document_content in documents_content: | ||
| print(f"Document content: {document_content['content']}") |
There was a problem hiding this comment.
Let's start thinking of logging from the very start.
| @@ -3,6 +3,12 @@ | |||
| from typing import List | |||
|
|
|||
|
|
|||
| class RequestAPI(abc.ABC): | |||
There was a problem hiding this comment.
The value of this abstract class is to define the call methods.
I would recommend defining those here.
|
|
||
|
|
||
| class NoticeFetcherABC(abc.ABC): | ||
| """ |
There was a problem hiding this comment.
If we decide to have only functions in the service layer of the application, then the class NoticeFetcher class can be merged into DocumetnSearch class. This is to be discussed.
There was a problem hiding this comment.
We decide to keep adapter logic separate of notice fetcher service logic.
|
|
||
| def test_notice_fetcher_by_identifier(): | ||
| document_id = "067623-2022" | ||
| notice = NoticeFetcher(document_search=TedDocumentSearch(request_api=TedRequestAPI())).get_notice_by_id( |
There was a problem hiding this comment.
I would use a fixture here for TedDocumentSearch.
| @@ -0,0 +1,42 @@ | |||
| import datetime | |||
tests/fakes/fake_ted_api.py
Outdated
| from ted_sws.notice_fetcher.adapters.ted_api_abc import DocumentSearchABC, RequestAPI | ||
| from tests.unit.notice_fetcher.conftest import get_api_response | ||
|
|
||
| FAKE_RESPONSE = { |
There was a problem hiding this comment.
shouldn't this be a fixture?
|
|
||
|
|
||
| def test_notice_fetcher_by_identifier(): | ||
| document_id = "067623-2022" | ||
| notice = NoticeFetcher().get_notice_by_id(document_id=document_id) | ||
| notice = NoticeFetcher(document_search=TedDocumentSearch(request_api=FakeRequestAPI())).get_notice_by_id( |
There was a problem hiding this comment.
TedDocumentSearch shall be coming from a fixture.
| @@ -20,19 +23,21 @@ def test_notice_fetcher_by_identifier(): | |||
| def test_notice_fetcher_by_search_query(): | |||
| query = {"q": "ND=[67623-2022]"} | |||
|
|
|||
| notices = NoticeFetcher().get_notices_by_query(query=query) | |||
| notices = NoticeFetcher(document_search=TedDocumentSearch(request_api=FakeRequestAPI())).get_notices_by_query( | |||
There was a problem hiding this comment.
injected dependencies are from fixtures.
| notices = NoticeFetcher().get_notices_by_date_range(start_date=datetime.date(2022, 2, 3), | ||
| end_date=datetime.date(2022, 2, 3)) | ||
| xml_text = "<NOTICE_DATA>" | ||
| notices = NoticeFetcher(document_search=TedDocumentSearch(request_api=FakeRequestAPI())).get_notices_by_date_range( |
|
|
||
| assert isinstance(notices, list) | ||
| assert len(notices) == 95 | ||
| assert len(notices) == 1 |
