Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions tests/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,26 @@ def raise_for_status(self):
return DummyResponse()


def make_dummy_get_success(fixture_path):
"""Factory that creates a mock GET function returning fixture data."""
with open(fixture_path, "r") as f:
fixture_data = load(f)

def dummy_get_success(url, headers=None):
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter signature dummy_get_success(url, headers=None) is inconsistent with dummy_get_403(url, headers) which requires the headers parameter. While this works because headers=None makes it optional, for consistency with the existing mock function pattern (dummy_get_403 at line 29), consider making the signature match: dummy_get_success(url, headers).

This ensures both mock functions have the same signature, making them more interchangeable and reducing potential confusion.

Suggested change
def dummy_get_success(url, headers=None):
def dummy_get_success(url, headers):

Copilot uses AI. Check for mistakes.
class DummyResponse:
status_code = 200

def json(self):
return fixture_data

def raise_for_status(self):
pass

return DummyResponse()

return dummy_get_success


class TestMetadataFetcher403:
def test_api_403_response(self, session, monkeypatch):
monkeypatch.setattr(session, "get", dummy_get_403)
Expand All @@ -49,12 +69,14 @@ def test_api_403_response(self, session, monkeypatch):
expected = "OpenAire refresh token is invalid or expired. Please update token and try again."
assert str(e.value) == expected

def test_openaire_v2(self, session):
def test_openaire_v2(self, session, monkeypatch):
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test class is named TestMetadataFetcher403, which suggests it's specifically for testing 403 responses. However, test_openaire_v2 is testing a successful API response (200 status code), not a 403 error. Consider either:

  1. Renaming the class to something more general like TestMetadataFetcher or TestOpenAIREMetadataFetcher, or
  2. Moving test_openaire_v2 to a separate class with an appropriate name like TestMetadataFetcherSuccess

This would make the test organization clearer and more consistent.

Copilot uses AI. Check for mistakes.
fixture_path = os.path.join("tests", "fixtures", "openaire_v2.json")
monkeypatch.setattr(session, "get", make_dummy_get_success(fixture_path))

fetcher = MetadataFetcher(session=session)
actual = fetcher.get_metadata_from_openaire("10.5281/zenodo.4650794")
with open(
os.path.join("tests", "fixtures", "openaire_v2.json"), "r"
) as f:

with open(fixture_path, "r") as f:
expected = load(f)
Comment on lines +73 to 80
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fixture file is being read twice: once when creating the mock function with make_dummy_get_success(fixture_path) (line 74), and again when loading the expected data (lines 79-80). This is inefficient and could be simplified.

Consider refactoring the test to reuse fixture_data from the factory or restructure the test to read the file only once. For example, you could read the fixture once, use it to create the mock, and also use it as the expected value without re-reading the file.

Copilot uses AI. Check for mistakes.
assert actual["results"] == expected["results"]

Expand Down
Loading