Add mock unit tests for Materials Project integration by jan-janssen · Pull Request #477 · pyiron/structuretoolkit

jan-janssen · 2026-04-06T07:32:25Z

I've added mock unit tests for the Materials Project integration in structuretoolkit. These tests cover the search and by_id functions, ensuring they correctly interact with the Materials Project API (via mocks) and process the returned data. By using mocks, we avoid the need for an actual API key and internet connectivity during testing. I've also included a mechanism to mock the mp_api dependency if it's missing in the test environment.

PR created automatically by Jules for task 1184161835048489774 started by @jan-janssen

This commit adds `tests/test_materialsproject.py`, which provides unit tests for the `search` and `by_id` functions in `src/structuretoolkit/build/materialsproject.py`. The tests use `unittest.mock` to simulate the `mp_api.client.MPRester` and `structuretoolkit.build.materialsproject.pymatgen_to_ase` functions, ensuring that the integration logic is correctly tested without requiring a real API key or an internet connection. Test cases include: - `test_search`: Verifies that `materialsproject_search` yields processed results and correctly handles the `structure` field. - `test_by_id`: Verifies that `materialsproject_by_id` correctly returns a single structure for `final=True` and a list of structures for `final=False`. The `mp_api` module is mocked in `sys.modules` to avoid import errors in environments where it is not installed. Co-authored-by: jan-janssen <3854739+jan-janssen@users.noreply.github.com>

google-labs-jules · 2026-04-06T07:32:26Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

coderabbitai · 2026-04-06T07:32:37Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e7d35454-f18e-44f5-9dcd-d2f14180353d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch add-materialsproject-mock-tests-1184161835048489774

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-04-06T07:34:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.82%. Comparing base (08c2a0f) to head (b0ae3a8).

Additional details and impacted files

@@            Coverage Diff             @@
##           mp-api     #477      +/-   ##
==========================================
+ Coverage   81.96%   82.82%   +0.86%     
==========================================
  Files          26       26              
  Lines        1857     1857              
==========================================
+ Hits         1522     1538      +16     
+ Misses        335      319      -16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Adds unit tests to validate the structuretoolkit.build Materials Project integration without requiring a real Materials Project API key or network access by using mocks.

Changes:

Introduces tests/test_materialsproject.py with mocked tests for materialsproject_search and materialsproject_by_id.
Adds a sys.modules-based stub for mp_api to allow tests to run when the optional dependency is not installed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T07:36:14Z

tests/test_materialsproject.py

+
+# Mock mp_api before importing structuretoolkit
+mock_mp_api = MagicMock()
+sys.modules["mp_api"] = mock_mp_api
+sys.modules["mp_api.client"] = mock_mp_api.client
+
+import structuretoolkit as stk
+
+class TestMaterialsProject(unittest.TestCase):


The test module permanently overwrites sys.modules["mp_api"] / sys.modules["mp_api.client"] at import time and never restores them. This can leak into other tests in the same process and also masks behavior when the real mp_api package is installed. Consider scoping this to the tests (e.g., patch.dict(sys.modules, ...) in setUp/tearDown or a context manager) and restoring the original sys.modules entries afterward.

Suggested change

# Mock mp_api before importing structuretoolkit

mock_mp_api = MagicMock()

sys.modules["mp_api"] = mock_mp_api

sys.modules["mp_api.client"] = mock_mp_api.client

import structuretoolkit as stk

class TestMaterialsProject(unittest.TestCase):

import importlib

import types

stk = None

class TestMaterialsProject(unittest.TestCase):

@classmethod

def setUpClass(cls):

global stk

mock_mp_api = types.ModuleType("mp_api")

mock_mp_api.client = types.ModuleType("mp_api.client")

mock_mp_api.client.MPRester = MagicMock()

cls._sys_modules_patcher = patch.dict(

sys.modules,

{"mp_api": mock_mp_api, "mp_api.client": mock_mp_api.client},

)

cls._sys_modules_patcher.start()

if "structuretoolkit" in sys.modules:

stk = importlib.reload(sys.modules["structuretoolkit"])

else:

stk = importlib.import_module("structuretoolkit")

@classmethod

def tearDownClass(cls):

cls._sys_modules_patcher.stop()

Copilot · 2026-04-06T07:36:14Z

tests/test_materialsproject.py

+    @patch("mp_api.client.MPRester")
+    @patch("structuretoolkit.build.materialsproject.pymatgen_to_ase")
+    def test_search(self, mock_pymatgen_to_ase, mock_mp_rester):
+        # Setup mock for MPRester as a context manager
+        mock_mpr = MagicMock()
+        mock_mp_rester.return_value.__enter__.return_value = mock_mpr
+
+        # Setup mock for summary.search
+        mock_mpr.summary.search.return_value = [
+            {"material_id": "mp-1", "structure": "mock_pmg_struct"}
+        ]
+
+        # Setup mock for pymatgen_to_ase
+        mock_pymatgen_to_ase.return_value = "mock_ase_struct"
+
+        # Call search
+        results = list(stk.build.materialsproject_search("Fe"))
+
+        # Assertions
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0]["material_id"], "mp-1")
+        self.assertEqual(results[0]["structure"], "mock_ase_struct")
+        mock_mpr.summary.search.assert_called_once()
+        mock_pymatgen_to_ase.assert_called_once_with("mock_pmg_struct")


These tests only assert that summary.search() / get_structure_by_material_id() were called, but don’t verify the arguments passed (e.g., chemsys, fields including structure/material_id, or that MPRester is constructed with use_document_model=False / include_user_agent=True). Without asserting call parameters, the tests can pass even if the integration sends an incorrect query or omits required options. Add assertions on the relevant call arguments to make the tests actually guard the intended API interaction contract.

Copilot · 2026-04-06T07:36:14Z

tests/test_materialsproject.py

+    @patch("mp_api.client.MPRester")
+    @patch("structuretoolkit.build.materialsproject.pymatgen_to_ase")
+    def test_by_id(self, mock_pymatgen_to_ase, mock_mp_rester):
+        # Setup mock for MPRester as a context manager
+        mock_mpr = MagicMock()
+        mock_mp_rester.return_value.__enter__.return_value = mock_mpr
+
+        # Setup mock for pymatgen_to_ase
+        mock_pymatgen_to_ase.side_effect = lambda x: f"ase_{x}"
+
+        # Test final=True
+        mock_mpr.get_structure_by_material_id.return_value = "pmg_struct"
+        res = stk.build.materialsproject_by_id("mp-1", final=True)
+        self.assertEqual(res, "ase_pmg_struct")
+        mock_mpr.get_structure_by_material_id.assert_called_with(
+            material_id="mp-1", final=True, conventional_unit_cell=False
+        )
+
+        # Test final=False
+        mock_mpr.get_structure_by_material_id.return_value = ["pmg_1", "pmg_2"]
+        res = stk.build.materialsproject_by_id("mp-1", final=False)
+        self.assertEqual(res, ["ase_pmg_1", "ase_pmg_2"])
+        mock_mpr.get_structure_by_material_id.assert_called_with(
+            material_id="mp-1", final=False, conventional_unit_cell=False
+        )
+


test_by_id similarly doesn’t assert that MPRester is instantiated with the expected kwargs (notably include_user_agent=True, and optionally api_key when provided). Adding an assertion on the MPRester(...) constructor call would better protect against regressions in how the client is configured.

pmrv · 2026-04-06T16:58:00Z

@jan-janssen I didn't see you had already opened this PR in the morning, so I pushed some tests already to #91 I think claude came up with more comprehensive test cases anyway.

Copilot AI review requested due to automatic review settings April 6, 2026 07:32

Copilot started reviewing on behalf of jan-janssen April 6, 2026 07:33 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mock unit tests for Materials Project integration#477

Add mock unit tests for Materials Project integration#477
jan-janssen wants to merge 1 commit intomp-apifrom
add-materialsproject-mock-tests-1184161835048489774

jan-janssen commented Apr 6, 2026

Uh oh!

google-labs-jules bot commented Apr 6, 2026

Uh oh!

coderabbitai bot commented Apr 6, 2026

Review skipped

Uh oh!

codecov bot commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

pmrv commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-# Mock mp_api before importing structuretoolkit
-mock_mp_api = MagicMock()
-sys.modules["mp_api"] = mock_mp_api
-sys.modules["mp_api.client"] = mock_mp_api.client
-import structuretoolkit as stk
-class TestMaterialsProject(unittest.TestCase):
+import importlib
+import types
+stk = None
+class TestMaterialsProject(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        global stk
+        mock_mp_api = types.ModuleType("mp_api")
+        mock_mp_api.client = types.ModuleType("mp_api.client")
+        mock_mp_api.client.MPRester = MagicMock()
+        cls._sys_modules_patcher = patch.dict(
+            sys.modules,
+            {"mp_api": mock_mp_api, "mp_api.client": mock_mp_api.client},
+        )
+        cls._sys_modules_patcher.start()
+        if "structuretoolkit" in sys.modules:
+            stk = importlib.reload(sys.modules["structuretoolkit"])
+        else:
+            stk = importlib.import_module("structuretoolkit")
+    @classmethod
+    def tearDownClass(cls):
+        cls._sys_modules_patcher.stop()

Conversation

jan-janssen commented Apr 6, 2026

Uh oh!

google-labs-jules bot commented Apr 6, 2026

Uh oh!

coderabbitai bot commented Apr 6, 2026

Review skipped

Uh oh!

codecov bot commented Apr 6, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

pmrv commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants