Harden parsing, fix bugs, and add offline + authenticated tests

.github/workflows/ci.yml

@@ -25,12 +25,22 @@ jobs:
           python -m pip install --upgrade pip
           pip install -e .
           pip install pytest pytest-cov pytest-html flaky
-      # These tests call flightradar24 directly; Cloudflare blocks GitHub-hosted
-      # runner IPs regardless of TLS impersonation, so failures here are expected
-      # and don't reflect code health. Kept informational rather than a hard gate.
-      - name: Run tests
+      # Offline unit tests (test_unit.py) exercise pure parsing/filtering/state
+      # logic with no network access, so they are a real gate: if these break,
+      # fail the build.
+      - name: Run offline unit tests
+        run: pytest pyflightdata/test_unit.py
+      # The remaining tests call flightradar24 directly; Cloudflare blocks
+      # GitHub-hosted runner IPs regardless of TLS impersonation, so failures
+      # here are expected and don't reflect code health. Kept informational.
+      # Authenticated tests read FR24_EMAIL/FR24_PASSWORD if those repo secrets
+      # exist, and skip themselves otherwise.
+      - name: Run integration tests (informational)
         continue-on-error: true
-        run: pytest pyflightdata
+        env:
+          FR24_EMAIL: ${{ secrets.FR24_EMAIL }}
+          FR24_PASSWORD: ${{ secrets.FR24_PASSWORD }}
+        run: pytest pyflightdata --ignore=pyflightdata/test_unit.py
 
   publish:
     if: github.event_name == 'release' && github.event.action == 'published'

.gitignore

@@ -39,6 +39,10 @@ htmlcov/
 .cache
 nosetests.xml
 coverage.xml
+# pytest-html report generated by the forced --html addopts in pytest.ini
+/test_report.html
+/assets/
+.pytest_cache/
 
 # Translations
 *.mo

pyflightdata/common.py

@@ -21,7 +21,6 @@
 # SOFTWARE.
 
 import json
-import sys
 import time
 
 from bs4 import BeautifulSoup
@@ -54,8 +53,6 @@ def _byteify(self, data, ignore_dicts=False):
         # if this is a unicode string, return its string representation
         if isinstance(data, str):
             return self.encode_and_get(data)
-        if sys.version_info[0] < 3 and isinstance(data, unicode):
-            return self.encode_and_get(data)
         # if this is a list of values, return list of byteified values
         if isinstance(data, list):
             return [self._byteify(item, ignore_dicts=True) for item in data]
@@ -159,7 +156,4 @@ def get_raw_data_json(self, url, path):
                 pass
 
     def encode_and_get(self, string):
-        if sys.version_info[0] < 3:
-            return string.encode('unicode-escape').replace('\\xa0', ' ')
-        else:
-            return string.encode('unicode-escape').replace(b'\\xa0', b' ').decode('utf-8')
+        return string.encode('unicode-escape').replace(b'\\xa0', b' ').decode('utf-8')

pyflightdata/common_fr24.py

@@ -255,13 +255,21 @@ def get_raw_airline_fleet_data(self, url):
     def process_raw_airline_fleet_data(self, data, authenticated):
         result = []
         for parent in data:
-            record = {}
             cells = parent.find_all('td')
-            record['reg'] = cells[0].find('a').text.strip()
+            # Rows without at least the reg + type columns are layout/header
+            # artifacts from flightradar24's markup, not fleet entries; skip them
+            # instead of raising IndexError when the site tweaks its HTML.
+            if len(cells) < 2:
+                continue
+            reg_link = cells[0].find('a')
+            if reg_link is None:
+                continue
+            record = {}
+            record['reg'] = reg_link.text.strip()
             record['type'] = cells[1].text.strip()
             record['msn'] = 'requires user login'
             record['age'] = 'requires user login'
-            if authenticated:
+            if authenticated and len(cells) >= 4:
                 record['msn'] = cells[2].text.strip()
                 record['age'] = cells[3].text.strip()
             result.append(record)

pyflightdata/conftest.py

@@ -0,0 +1,79 @@
+# MIT License
+#
+# Copyright (c) 2020 Hari Allamraju
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in all
+# copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+"""Shared pytest fixtures.
+
+The authenticated online tests need real flightradar24 credentials. They are
+read, in order of precedence, from:
+
+1. the ``FR24_EMAIL`` / ``FR24_PASSWORD`` environment variables (use these in
+   CI secrets), or
+2. a local ``.env`` file at the repository root with ``email=`` / ``password=``
+   lines (gitignored, handy for local runs).
+
+When no credentials are available the ``credentials`` fixture skips the test, so
+the suite stays green on CI runners that have no secret configured.
+"""
+
+import os
+
+import pytest
+
+
+def _read_env_file(path):
+    values = {}
+    try:
+        with open(path) as handle:
+            for line in handle:
+                line = line.strip()
+                if not line or line.startswith('#') or '=' not in line:
+                    continue
+                key, _, value = line.partition('=')
+                values[key.strip()] = value.strip().strip('"').strip("'")
+    except OSError:
+        pass
+    return values
+
+
+def load_credentials():
+    """Return ``(email, password)`` or ``(None, None)`` if not configured."""
+    email = os.environ.get('FR24_EMAIL')
+    password = os.environ.get('FR24_PASSWORD')
+    if email and password:
+        return email, password
+    env_path = os.path.join(os.path.dirname(os.path.dirname(__file__)), '.env')
+    values = _read_env_file(env_path)
+    email = email or values.get('FR24_EMAIL') or values.get('email')
+    password = password or values.get('FR24_PASSWORD') or values.get('password')
+    if email and password:
+        return email, password
+    return None, None
+
+
+@pytest.fixture(scope='session')
+def credentials():
+    email, password = load_credentials()
+    if not (email and password):
+        pytest.skip(
+            'no flightradar24 credentials; set FR24_EMAIL/FR24_PASSWORD or a '
+            'local .env with email=/password=')
+    return email, password

pyflightdata/flightdata.py

@@ -661,25 +661,41 @@ def login(self, email, password):
             f=FlightData()
             f.login(myemail,mypassword)
 
+        Returns:
+            bool: True if the login succeeded and an auth token was obtained,
+            False otherwise (bad credentials, network error, or an unexpected
+            response shape from flightradar24).
+
         """
-        response = FlightData.session.post(
-            url=LOGIN_URL,
-            data={
-                'email': email,
-                'password': password,
-                'remember': 'true',
-                'type': 'web'
-            },
-            headers={
-                'Origin': 'https://www.flightradar24.com',
-                'Referer': 'https://www.flightradar24.com'
-            }
-        )
-        response = self._fr24.json_loads_byteified(
-            response.content) if response.status_code == 200 else None
-        if response:
-            token = response['userData']['subscriptionKey']
-            self.AUTH_TOKEN = token
+        try:
+            response = FlightData.session.post(
+                url=LOGIN_URL,
+                data={
+                    'email': email,
+                    'password': password,
+                    'remember': 'true',
+                    'type': 'web'
+                },
+                headers={
+                    'Origin': 'https://www.flightradar24.com',
+                    'Referer': 'https://www.flightradar24.com'
+                }
+            )
+        except Exception:
+            return False
+        if response.status_code != 200:
+            return False
+        try:
+            payload = self._fr24.json_loads_byteified(response.content)
+            token = payload['userData']['subscriptionKey']
+        except (ValueError, KeyError, TypeError):
+            # ValueError: body was not JSON; KeyError/TypeError: JSON shape
+            # changed and the token is no longer where we expect it.
+            return False
+        if not token:
+            return False
+        self.AUTH_TOKEN = token
+        return True
 
     def logout(self):
         """Logout from the flightradar24 session.

pyflightdata/pytest.ini

@@ -1,3 +1,6 @@
 [pytest]
 addopts = --html=test_report.html
-python_files = *.py
+python_files = test_*.py
+markers =
+    online: test hits the live flightradar24 site (may fail behind Cloudflare)
+    auth: test needs flightradar24 credentials (skipped when none configured)

pyflightdata/test_online_auth.py

@@ -0,0 +1,102 @@
+# MIT License
+#
+# Copyright (c) 2020 Hari Allamraju
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in all
+# copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+"""Authenticated online tests against the live flightradar24 site.
+
+These require real credentials (see ``conftest.load_credentials``) and are
+skipped automatically when none are configured, so they never break CI. The
+premise, per the flightradar24 plans, is that anonymous and paid access differ
+mainly in how far back the history goes: a logged-in client should therefore see
+*at least* as much history as an anonymous one within the same page budget.
+"""
+
+import time
+
+import pytest
+from flaky import flaky
+
+from .flightdata import FlightData
+
+
+def delay_rerun(*args):
+    time.sleep(5)
+    return True
+
+
+def _history_depth(client, tail, max_pages=3):
+    """Count history rows for ``tail`` across up to ``max_pages`` pages.
+
+    ``FlightData._fr24`` holds pagination state and is shared across every
+    ``FlightData`` instance, so callers must never interleave two clients'
+    paged reads. We reset that shared state before and after each pass.
+    """
+    client.clear_last_request()
+    total = 0
+    for page in range(1, max_pages + 1):
+        rows = client.get_history_by_tail_number(tail, page=page)
+        total += len(rows)
+        if not rows:
+            break
+        time.sleep(2)
+    client.clear_last_request()
+    return total
+
+
+@pytest.mark.online
+@pytest.mark.auth
+@flaky(max_runs=3, rerun_filter=delay_rerun)
+class TestAuthenticatedAccess(object):
+
+    # A busy long-haul aircraft, so there is plenty of history to page through.
+    TAIL = '9V-SMC'
+
+    def teardown(self):
+        time.sleep(3)
+
+    def test_login_then_logout(self, credentials):
+        email, password = credentials
+        f = FlightData()
+        assert f.is_authenticated() is False
+        assert f.login(email, password) is True
+        assert f.is_authenticated() is True
+        f.logout()
+        assert f.is_authenticated() is False
+
+    def test_login_via_constructor(self, credentials):
+        email, password = credentials
+        f = FlightData(email, password)
+        assert f.is_authenticated() is True
+
+    def test_authenticated_returns_history(self, credentials):
+        email, password = credentials
+        f = FlightData(email, password)
+        assert _history_depth(f, self.TAIL) > 0
+
+    def test_authenticated_history_at_least_anonymous(self, credentials):
+        email, password = credentials
+        # Sequential, never interleaved, because _fr24 pagination state is shared.
+        anon_depth = _history_depth(FlightData(), self.TAIL)
+        auth_depth = _history_depth(FlightData(email, password), self.TAIL)
+        assert auth_depth > 0
+        # Paid history reaches at least as far back as anonymous within the same
+        # page budget; the only advertised difference between the tiers is depth.
+        assert auth_depth >= anon_depth

pyflightdata/test_pyflightdata.py

@@ -52,11 +52,11 @@ def test_check_there_is_history_data(self):
         result = f.get_history_by_flight_number('AI101')
         assert result.__len__() > 0
 
-    def test_check_there_is_history_data(self):
+    def test_flight_for_today_has_data(self):
         result = f.get_flight_for_date('AI101',self.t)
         assert result.__len__() > 0
 
-    def test_check_there_is_history_data(self):
+    def test_flight_for_far_future_has_no_data(self):
         result = f.get_flight_for_date('AI101',self.tpls90)
         assert result.__len__() == 0
 
@@ -136,16 +136,16 @@ def test_get_airport_arrivals(self):
     def test_get_airport_departures(self):
         assert f.get_airport_departures('SIN').__len__() >= 0
 
-    def test_get_airport_departures_earlier(self):
+    def test_get_airport_departures_earlier_pmf(self):
         assert f.get_airport_departures('PMF', earlier_data=True).__len__() >= 0
 
-    def test_get_airport_departures_no_earlier(self):
+    def test_get_airport_departures_no_earlier_sin(self):
         assert f.get_airport_departures('SIN', earlier_data=False).__len__() >= 0
 
-    def test_get_airport_departures_earlier(self):
+    def test_get_airport_departures_earlier_gau(self):
         assert f.get_airport_departures('GAU', earlier_data=True).__len__() >= 0
 
-    def test_get_airport_departures_no_earlier(self):
+    def test_get_airport_departures_no_earlier_gau(self):
         assert f.get_airport_departures('GAU', earlier_data=False).__len__() >= 0
 
     def test_get_airport_onground(self):