Bring unit test coverage to 100% (scrapd#115)

In an attempt to reach the last lines of code that were not covered by
the unit tests, this patch does the following:

* adds the missing tests.
* updates the Makefile to re-run failed integration tests only.
* updates the CircleCI workflow to remove the integration tests from the
  coverage.
* adjusts the Mergify configuration.

Fixes scrapd#100

Author: rgreinho

.circleci/config.yml

@@ -58,7 +58,7 @@ jobs:
           name: run doc tests
           command: make docs
 
-  test:
+  test-units:
     <<: *defaults
     steps:
       - checkout
@@ -67,11 +67,21 @@ jobs:
       - run:
           name: run unit tests
           command: |
-            make test
+            make test-units
             venv/bin/pip install coveralls
             venv/bin/coveralls
       - store_test_results:
           path: test-results
+  test-integrations:
+    <<: *defaults
+    steps:
+      - checkout
+      - attach_workspace:
+          at: *working_directory
+      - run:
+          name: run unit tests
+          command: |
+            make test-integrations
 
   publish:
     <<: *defaults
@@ -139,7 +149,13 @@ workflows:
           filters:
             tags:
               only: /.*/
-      - test:
+      - test-units:
+          requires:
+            - prepare
+          filters:
+            tags:
+              only: /.*/
+      - test-integrations:
           requires:
             - prepare
           filters:
@@ -158,7 +174,8 @@ workflows:
           requires:
             - format
             - lint
-            - test
+            - test-units
+            - test-integrations
           filters:
             branches:
               ignore: /.*/

.mergify.yml

@@ -10,5 +10,6 @@ pull_request_rules:
   - 'status-success=ci/circleci: format'
   - 'status-success=ci/circleci: lint'
   - 'status-success=ci/circleci: prepare'
-  - 'status-success=ci/circleci: test'
+  - 'status-success=ci/circleci: test-units'
+  - 'status-success=ci/circleci: test-integrations'
   name: master

Makefile

@@ -95,7 +95,7 @@ test-units: venv ## Run the unit tests
 
 .PHONY: test-integrations
 test-integrations: venv ## Run the unit tests
-	$(RUN_CMD) tox -- -m "integrations"
+	$(RUN_CMD) tox -- -m "integrations" --reruns 3 --reruns-delay 5
 
 setup: venv ## Setup the full environment (default)
 

requirements-dev.txt

@@ -1,6 +1,7 @@
 Faker==1.0.5
 Sphinx==2.0.1
 aiohttp-theme==0.1.5
+aioresponses==0.6.0
 asynctest==0.12.4
 bpython
 flake8==3.7.7
@@ -10,6 +11,7 @@ pytest-asyncio==0.10.0
 pytest-bdd==3.1.0
 pytest-cov==2.6.1
 pytest-mock==1.10.4
+pytest-rerunfailures==7.0
 pytest-socket==0.3.3
 pytest-xdist==1.28.0
 pytest==4.4.1

scrapd/core/apd.py

@@ -592,7 +592,6 @@ async def fetch_and_parse(session, url):
     :rtype: dict
     """
     # Retrieve the page.
-    # page = await fetch_text(session, url)
     page = await fetch_detail_page(session, url)
     if not page:
         raise ValueError(f'The URL {url} returned a 0-length content.')

tests/core/test_apd.py

@@ -2,7 +2,9 @@
 from unittest import mock
 
 import aiohttp
+from aioresponses import aioresponses
 import asynctest
+from faker import Faker
 from loguru import logger
 import pytest
 from tenacity import RetryError
@@ -18,6 +20,9 @@
 # Disable logging for the tests.
 logger.remove()
 
+# Set faker object.
+fake = Faker()
+
 
 def load_test_page(page):
     """Load a test page."""
@@ -394,10 +399,17 @@ def test_parse_page_content_01(mocker):
     page_fd = TEST_DATA_DIR / 'traffic-fatality-2-3'
     page = page_fd.read_text()
     mocker.patch('scrapd.core.apd.parse_deceased_field', side_effect=ValueError)
-    apd.parse_page_content(page)
+    result = apd.parse_page_content(page)
+    assert len(result) == 6
+
 
+def test_parse_page_content_02(mocker):
+    """Ensure a log entry is created if there is no deceased field."""
+    result = apd.parse_page_content('Case: 01-2345678')
+    assert result
 
-def test_parse_page_content_02():
+
+def test_parse_page_content_03():
     """Ensure a missing case number raises an exception."""
     with pytest.raises(ValueError):
         apd.parse_page_content('The is no case number here.')
@@ -425,15 +437,8 @@ def test_parse_page_00(filename, expected):
 
 
 @asynctest.patch("scrapd.core.apd.fetch_news_page",
-                 side_effect=[load_test_page(page) for page in [
-                     '296',
-                     '296?page=1',
-                     '296?page=27',
-                 ]])
-@asynctest.patch(
-    "scrapd.core.apd.fetch_detail_page",
-    return_value=load_test_page('traffic-fatality-2-3'),
-)
+                 side_effect=[load_test_page(page) for page in ['296', '296?page=1', '296?page=27']])
+@asynctest.patch("scrapd.core.apd.fetch_detail_page", return_value=load_test_page('traffic-fatality-2-3'))
 @pytest.mark.asyncio
 async def test_date_filtering_00(fake_details, fake_news):
     """Ensure the date filtering do not fetch unnecessary data."""
@@ -444,22 +449,29 @@ async def test_date_filtering_00(fake_details, fake_news):
 
 
 @asynctest.patch("scrapd.core.apd.fetch_news_page",
-                 side_effect=[load_test_page(page) for page in [
-                     '296',
-                     '296?page=1',
-                     '296?page=27',
-                 ]])
-@asynctest.patch(
-    "scrapd.core.apd.fetch_detail_page",
-    return_value=load_test_page('traffic-fatality-2-3'),
-)
+                 side_effect=[load_test_page(page) for page in ['296', '296?page=1', '296?page=27']])
+@asynctest.patch("scrapd.core.apd.fetch_detail_page", return_value=load_test_page('traffic-fatality-2-3'))
 @pytest.mark.asyncio
 async def test_date_filtering_01(fake_details, fake_news):
     """Ensure the date filtering do not fetch unnecessary data."""
     data, _ = await apd.async_retrieve(pages=-5, from_="2019-01-02", to="2019-01-03")
     assert isinstance(data, list)
 
 
+@asynctest.patch("scrapd.core.apd.fetch_news_page",
+                 side_effect=[load_test_page(page) for page in ['296', '296?page=1', '296?page=27']])
+@asynctest.patch(
+    "scrapd.core.apd.fetch_detail_page",
+    side_effect=[load_test_page(page) for page in ['traffic-fatality-2-3'] + ['traffic-fatality-71-2'] * 14])
+@pytest.mark.asyncio
+async def test_date_filtering_02(fake_details, fake_news):
+    """Ensure the date filtering do not fetch unnecessary data."""
+    data, page_count = await apd.async_retrieve(from_="2019-01-16", to="2019-01-16")
+    assert isinstance(data, list)
+    assert len(data) == 1
+    assert page_count == 2
+
+
 @pytest.mark.asyncio
 async def test_fetch_text_00():
     """Ensure `fetch_text` retries several times."""
@@ -474,6 +486,17 @@ async def test_fetch_text_00():
     assert apd.fetch_text.retry.statistics['attempt_number'] > 1
 
 
+@pytest.mark.asyncio
+async def test_fetch_text_01():
+    """Ensure fetch_text retrieves some text."""
+    url = fake.uri()
+    with aioresponses() as m:
+        m.get(url, payload=dict(foo='bar'))
+        async with aiohttp.ClientSession() as session:
+            text = await apd.fetch_text(session, url)
+            assert '{"foo": "bar"}' == text
+
+
 @asynctest.patch("scrapd.core.apd.fetch_news_page", side_effect=ValueError)
 @pytest.mark.asyncio
 async def test_async_retrieve_00(fake_news):
@@ -524,3 +547,30 @@ async def test_fetch_and_parse_01(page, mocker):
     with pytest.raises(RetryError):
         apd.fetch_and_parse.retry.stop = stop_after_attempt(1)
         await apd.fetch_and_parse(None, 'url')
+
+
+@asynctest.patch("scrapd.core.apd.fetch_text", return_value='')
+@pytest.mark.asyncio
+async def test_fetch_news_page_00(fetch_text):
+    """Ensure the fetch function is called with the right parameters."""
+    page = 2
+    params = {'page': page - 1}
+    async with aiohttp.ClientSession() as session:
+        try:
+            await apd.fetch_news_page(session, page)
+        except Exception:
+            pass
+    fetch_text.assert_called_once_with(session, apd.APD_URL, params)
+
+
+@asynctest.patch("scrapd.core.apd.fetch_text", return_value='')
+@pytest.mark.asyncio
+async def test_fetch_detail_page_00(fetch_text):
+    """Ensure the fetch function is called with the right parameters."""
+    url = fake.uri()
+    async with aiohttp.ClientSession() as session:
+        try:
+            await apd.fetch_detail_page(session, url)
+        except Exception:
+            pass
+    fetch_text.assert_called_once_with(session, url)

tests/step_defs/test_retrieve.py

@@ -1,14 +1,11 @@
 """Define the scenarios for the feature tests."""
-import asyncio
-
 import pytest
 from pytest_bdd import given
 from pytest_bdd import scenario
 from pytest_bdd import then
 
 from scrapd.core import apd
 from tests.test_common import TEST_ROOT_DIR
-from tests.test_common import TEST_DATA_DIR
 
 
 @scenario(
@@ -45,5 +42,5 @@ def ensure_results(mocker, event_loop, output_format, time_range, entry_count):
     assert result is not None
     assert len(result) == entry_count
     assert isinstance(result, list)
-    if len(result) > 0:
+    if result:
         assert isinstance(result[0], dict)