diff --git a/src/base_classes/data_sources.py b/src/base_classes/data_sources.py index 8c73df6c..80c6177a 100644 --- a/src/base_classes/data_sources.py +++ b/src/base_classes/data_sources.py @@ -132,18 +132,19 @@ class ESPNDataSource(DataSource): endpoints = ["rankings", "standings"] if wants_poll else ["standings", "rankings"] for endpoint in endpoints: + url = f"{self.base_url}/{sport}/{league}/{endpoint}" + # Only the request is guarded. Inspecting the payload happens + # below, outside the handler, so that a bug in this method cannot + # be mistaken for an endpoint that failed -- that mistake would + # silently drop rankings for a league that has them, which is the + # exact failure this function was written to fix. try: - url = f"{self.base_url}/{sport}/{league}/{endpoint}" response = self.session.get( url, headers=self.get_headers(), timeout=15 ) response.raise_for_status() data = response.json() - if endpoint == "rankings" and not data.get("rankings"): - continue - self.logger.debug(f"Fetched {endpoint} for {sport}/{league}") - return data - except Exception as e: + except (requests.RequestException, ValueError) as e: status = getattr(getattr(e, "response", None), "status_code", None) # Only a 404 is routine -- it is how a league says "no poll # here". Everything else is worth an error, and `status is @@ -156,6 +157,21 @@ class ESPNDataSource(DataSource): f"Error fetching {endpoint} from ESPN for " f"{sport}/{league}: {e}" ) + continue + + if not isinstance(data, dict): + # A list or a bare string is not something the callers can + # read. Treat it as a miss so the other endpoint still gets a + # turn, but say so -- this means ESPN changed shape. + self.logger.error( + f"Unexpected {endpoint} payload for {sport}/{league}: " + f"got {type(data).__name__}, expected an object" + ) + continue + if endpoint == "rankings" and not data.get("rankings"): + continue + self.logger.debug(f"Fetched {endpoint} for {sport}/{league}") + return data self.logger.debug( f"Standings/rankings not available for {sport}/{league} from ESPN API" ) diff --git a/test/test_data_sources.py b/test/test_data_sources.py index 9aad0588..d7366923 100644 --- a/test/test_data_sources.py +++ b/test/test_data_sources.py @@ -92,10 +92,108 @@ class TestESPNDataSource: assert result == payload def test_fetch_standings_returns_empty_on_error(self): - with patch.object(self.source.session, "get", side_effect=Exception("error")): + # A transport failure is a RequestException, not a bare Exception. + # The old stand-in passed only because the handler caught everything, + # including bugs in the method under test. + with patch.object(self.source.session, "get", + side_effect=requests.ConnectionError("error")): result = self.source.fetch_standings("football", "nfl") assert result == {} + # ------------------------------------------------------------------ + # fetch_standings endpoint selection + # + # College leagues publish a poll at /rankings and a records table at + # /standings; professional leagues have only /standings. Probing them in + # the wrong order still returns 200 -- just without a poll in it -- so + # nothing failed and the rank badge simply never appeared. Order is the + # behaviour here, so these tests assert it directly. + # ------------------------------------------------------------------ + + @staticmethod + def _requested_endpoints(mock_get): + """The endpoint names probed, in the order they were requested.""" + return [call.args[0].rsplit("/", 1)[-1] for call in mock_get.call_args_list] + + def test_professional_league_asks_standings_first(self): + payload = {"standings": []} + with patch.object(self.source.session, "get", + return_value=_mock_response(payload)) as mock_get: + result = self.source.fetch_standings("football", "nfl") + assert result == payload + assert self._requested_endpoints(mock_get) == ["standings"] + + def test_college_league_asks_rankings_first(self): + poll = {"rankings": [{"name": "AP Top 25"}]} + with patch.object(self.source.session, "get", + return_value=_mock_response(poll)) as mock_get: + result = self.source.fetch_standings("football", "college-football") + assert result == poll + assert self._requested_endpoints(mock_get) == ["rankings"] + + def test_rankings_200_without_a_poll_falls_through_to_standings(self): + """A 200 is not the same as an answer. + + This is the case the old code could not see: the endpoint responded, + so nothing raised, but the body carried no poll. + """ + empty_poll = _mock_response({"rankings": []}) + table = _mock_response({"standings": [{"entries": []}]}) + with patch.object(self.source.session, "get", + side_effect=[empty_poll, table]) as mock_get: + result = self.source.fetch_standings( + "basketball", "mens-college-basketball") + assert result == {"standings": [{"entries": []}]} + assert self._requested_endpoints(mock_get) == ["rankings", "standings"] + + def test_404_on_the_first_endpoint_falls_through_quietly(self): + missing = _mock_response({}, status_code=404) + table = _mock_response({"standings": []}) + with patch.object(self.source.session, "get", + side_effect=[missing, table]) as mock_get: + result = self.source.fetch_standings("baseball", "college-baseball") + assert result == {"standings": []} + assert self._requested_endpoints(mock_get) == ["rankings", "standings"] + + def test_recovers_from_a_non_404_failure_on_the_first_endpoint(self): + table = _mock_response({"standings": [{"entries": []}]}) + with patch.object(self.source.session, "get", + side_effect=[requests.ConnectionError("reset"), table]) as mock_get: + result = self.source.fetch_standings("football", "college-football") + assert result == {"standings": [{"entries": []}]} + assert self._requested_endpoints(mock_get) == ["rankings", "standings"] + + def test_both_endpoints_failing_returns_empty(self): + with patch.object(self.source.session, "get", + side_effect=requests.ConnectionError("down")) as mock_get: + result = self.source.fetch_standings("football", "nfl") + assert result == {} + assert self._requested_endpoints(mock_get) == ["standings", "rankings"] + + def test_a_non_object_payload_is_treated_as_a_miss(self): + odd = _mock_response(["not", "an", "object"]) + table = _mock_response({"standings": []}) + with patch.object(self.source.session, "get", + side_effect=[odd, table]) as mock_get: + result = self.source.fetch_standings("football", "college-football") + assert result == {"standings": []} + assert self._requested_endpoints(mock_get) == ["rankings", "standings"] + + def test_a_bug_in_this_method_is_not_swallowed_as_a_failed_endpoint(self): + """The guard for the narrowed handler. + + An error raised while reading the payload used to be caught by the + endpoint handler and reported as 'no poll here', which would silently + drop rankings for a league that has them. It must surface instead. + """ + boom = Mock(spec=requests.Response) + boom.status_code = 200 + boom.raise_for_status = Mock() + boom.json.side_effect = TypeError("a bug, not a network failure") + with patch.object(self.source.session, "get", return_value=boom): + with pytest.raises(TypeError): + self.source.fetch_standings("football", "nfl") + def test_base_url_set_correctly(self): assert "espn.com" in self.source.base_url