Fix jobs methods to use function filter correctly (#76)

* Update jobs method to use new filters, run black

* Rename test folder to conform with Qiskit standards. Split unit tests into distict files to reflect file structure in the catalog. Split tests by tested functionality.

* Add unit tests for jobs with function filter

Author: ElePT

qiskit_ibm_catalog/__init__.py

@@ -20,6 +20,7 @@
     QiskitServerless
     QiskitFunction
 """
+
 # pylint: disable=W0404
 from importlib_metadata import version as metadata_version, PackageNotFoundError
 
@@ -28,7 +29,6 @@
 from .catalog import QiskitFunctionsCatalog
 from .serverless import QiskitServerless
 
-
 try:
     __version__ = metadata_version("qiskit_ibm_catalog")
 except PackageNotFoundError:  # pragma: no cover

qiskit_ibm_catalog/catalog.py

@@ -18,6 +18,7 @@
 
     QiskitFunctionsCatalog
 """
+
 from __future__ import annotations
 
 from typing import Optional, List
@@ -103,21 +104,23 @@ def list(self, **kwargs) -> List[QiskitFunction]:
             **{**kwargs, **{"filter": self.PRE_FILTER_KEYWORD}}
         )
 
-    def jobs(self, **kwargs) -> List[Job]:
+    def jobs(self, function: Optional[QiskitFunction] = None, **kwargs) -> List[Job]:
         """Returns list of jobs.
 
         Args:
+            function (QiskitFunction): The function that created the jobs we want to retrieve.
             limit (int, optional): Maximum number of jobs to return. Defaults to 10.
             offset (int, optional): Number of jobs to skip. Defaults to 0.
             status (str, optional): Filter by job status.
             created_after (str, optional): Filter jobs created after this timestamp.
-            function_name (str, optional): Filter by function name.
             **kwargs: Additional query parameters.
 
         Returns:
             List[Job]: jobs
         """
-        return self._client.jobs(**{**kwargs, **{"filter": self.PRE_FILTER_KEYWORD}})
+        return self._client.jobs(
+            function=function, **{**kwargs, **{"filter": self.PRE_FILTER_KEYWORD}}
+        )
 
     def provider_jobs(self, function: QiskitFunction, **kwargs) -> List[Job]:
         """List of jobs created in this provider and function.

qiskit_ibm_catalog/serverless.py

@@ -18,6 +18,7 @@
 
     QiskitServerless
 """
+
 # pylint: disable=duplicate-code
 from __future__ import annotations
 
@@ -118,13 +119,15 @@ def list(self, **kwargs) -> List[QiskitFunction]:
             **{**kwargs, **{"filter": self.PRE_FILTER_KEYWORD}}
         )
 
-    def jobs(self, **kwargs) -> List[Job]:
+    def jobs(self, function: Optional[QiskitFunction] = None, **kwargs) -> List[Job]:
         """Returns list of jobs.
 
         Returns:
             List[Job]: jobs
         """
-        return self._client.jobs(**{**kwargs, **{"filter": self.PRE_FILTER_KEYWORD}})
+        return self._client.jobs(
+            function=function, **{**kwargs, **{"filter": self.PRE_FILTER_KEYWORD}}
+        )
 
     def provider_jobs(self, function: QiskitFunction, **kwargs) -> List[Job]:
         """List of jobs created in this provider and function.

tests/README.md test/README.mdtests/README.md renamed to test/README.md

@@ -0,0 +1,93 @@
+# This code is part of Qiskit.
+#
+# (C) Copyright IBM 2024.
+#
+# This code is licensed under the Apache License, Version 2.0. You may
+# obtain a copy of this license in the LICENSE.txt file in the root directory
+# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# Any modifications or derivative works of this code must retain this
+# copyright notice, and modified files need to carry a notice indicating
+# that they have been altered from the originals.
+
+"""Tests for QiskitFunctionsCatalog class."""
+
+from unittest import TestCase, mock
+
+from qiskit_serverless import IBMServerlessClient
+from qiskit_serverless.core import Job, QiskitFunction
+from qiskit_ibm_catalog import QiskitFunctionsCatalog
+
+
+class TestCatalog(TestCase):
+    """TestCatalog."""
+
+    @mock.patch(
+        "qiskit_serverless.core.clients.serverless_client.ServerlessClient._verify_credentials"
+    )
+    def test_authentication(self, _verify_mock):
+        """Tests authentication of serverless client."""
+        catalog = QiskitFunctionsCatalog(token="token", instance="instance")
+
+        # pylint: disable=protected-access
+        self.assertEqual(catalog._client.token, "token")
+        self.assertEqual(catalog._client.instance, "instance")
+        # pylint: enable=protected-access
+
+    @mock.patch.object(
+        IBMServerlessClient,
+        "functions",
+        return_value=[QiskitFunction("the-ultimate-answer")],
+    )
+    @mock.patch.object(
+        IBMServerlessClient, "jobs", return_value=[Job("42", mock.MagicMock())]
+    )
+    @mock.patch(
+        "qiskit_serverless.core.clients.serverless_client.ServerlessClient._verify_credentials"
+    )
+    def test_basic_functions(self, _verify_mock, jobs_mock, functions_list_mock):
+        """Tests basic function of catalog."""
+        catalog = QiskitFunctionsCatalog(token="token", instance="instance")
+
+        jobs = catalog.jobs(limit=10)
+        functions = catalog.list()
+
+        jobs_mock.assert_called()
+        called_kwargs = jobs_mock.call_args.kwargs
+        assert called_kwargs["filter"] == "catalog"
+        assert called_kwargs["limit"] == 10
+        functions_list_mock.assert_called_with(**{"filter": "catalog"})
+
+        self.assertEqual(len(jobs), 1)
+        self.assertEqual(len(functions), 1)
+
+    @mock.patch.object(
+        IBMServerlessClient, "jobs", return_value=[Job("42", mock.MagicMock())]
+    )
+    @mock.patch(
+        "qiskit_serverless.core.clients.serverless_client.ServerlessClient._verify_credentials"
+    )
+    def test_jobs_with_function_filter(self, _verify_mock, jobs_mock):
+        """Tests that 'function' is forwarded and 'serverless' filter is enforced."""
+        catalog = QiskitFunctionsCatalog(token="token", instance="instance")
+
+        my_function = QiskitFunction("my-func")
+
+        # Call jobs with both function and an additional kwarg
+        jobs = catalog.jobs(function=my_function, limit=7)
+
+        jobs_mock.assert_called()
+        called_args = jobs_mock.call_args.args
+        called_kwargs = jobs_mock.call_args.kwargs
+
+        # Positional args should be empty because we pass by keyword
+        assert called_args == ()
+
+        # Ensure 'function' got forwarded and 'filter' remained enforced
+        assert called_kwargs["function"] is my_function
+        assert called_kwargs["filter"] == "catalog"
+        assert called_kwargs["limit"] == 7
+
+        self.assertEqual(len(jobs), 1)
+        self.assertIsInstance(jobs[0], Job)
+        self.assertEqual(jobs[0].job_id, "42")

tests/__init__.py test/__init__.pytests/__init__.py renamed to test/__init__.py

@@ -10,44 +10,32 @@
 # copyright notice, and modified files need to carry a notice indicating
 # that they have been altered from the originals.
 
-"""Tests for wrappers."""
+"""Tests for QiskitServerless class."""
 
 from unittest import TestCase, mock
 
 from qiskit_serverless import IBMServerlessClient
 from qiskit_serverless.core import Job, QiskitFunction
-from qiskit_ibm_catalog import QiskitServerless, QiskitFunctionsCatalog
+from qiskit_ibm_catalog import QiskitServerless
 
 
-class TestCatalog(TestCase):
-    """TestCatalog."""
+class TestServerless(TestCase):
+    """TestServerless."""
 
-    @mock.patch.object(
-        IBMServerlessClient,
-        "functions",
-        return_value=[QiskitFunction("the-ultimate-answer")],
-    )
-    @mock.patch.object(
-        IBMServerlessClient, "jobs", return_value=[Job("42", mock.MagicMock())]
-    )
     @mock.patch(
         "qiskit_serverless.core.clients.serverless_client.ServerlessClient._verify_credentials"
     )
-    def test_basic_functions(self, _token_mock, jobs_mock, functions_list_mock):
-        """Tests basic function of catalog."""
-        catalog = QiskitFunctionsCatalog(token="token", instance="instance")
-        jobs = catalog.jobs(limit=10)
-        functions = catalog.list()
-
-        jobs_mock.assert_called_with(**{"filter": "catalog", "limit": 10})
-        functions_list_mock.assert_called_with(**{"filter": "catalog"})
-
-        self.assertEqual(len(jobs), 1)
-        self.assertEqual(len(functions), 1)
-
+    def test_authentication(self, _verify_mock):
+        """Tests authentication of serverless client."""
+        serverless = QiskitServerless(
+            token="token", instance="instance", host="http://host"
+        )
 
-class TestServerless(TestCase):
-    """TestServerless."""
+        # pylint: disable=protected-access
+        self.assertEqual(serverless._client.token, "token")
+        self.assertEqual(serverless._client.instance, "instance")
+        self.assertEqual(serverless._client.host, "http://host")
+        # pylint: enable=protected-access
 
     @mock.patch.object(
         IBMServerlessClient,
@@ -60,7 +48,7 @@ class TestServerless(TestCase):
     @mock.patch(
         "qiskit_serverless.core.clients.serverless_client.ServerlessClient._verify_credentials"
     )
-    def test_basic_functions(self, _token_mock, jobs_mock, functions_list_mock):
+    def test_basic_functions(self, _verify_mock, jobs_mock, functions_list_mock):
         """Tests basic function of serverless client."""
         serverless = QiskitServerless(
             token="token", instance="instance", host="http://host"
@@ -75,8 +63,44 @@ def test_basic_functions(self, _token_mock, jobs_mock, functions_list_mock):
         jobs = serverless.jobs(limit=10)
         functions = serverless.list()
 
-        jobs_mock.assert_called_with(**{"filter": "serverless", "limit": 10})
+        jobs_mock.assert_called()
+        called_kwargs = jobs_mock.call_args.kwargs
+        assert called_kwargs["filter"] == "serverless"
+        assert called_kwargs["limit"] == 10
         functions_list_mock.assert_called_with(**{"filter": "serverless"})
 
         self.assertEqual(len(jobs), 1)
         self.assertEqual(len(functions), 1)
+
+    @mock.patch.object(
+        IBMServerlessClient, "jobs", return_value=[Job("42", mock.MagicMock())]
+    )
+    @mock.patch(
+        "qiskit_serverless.core.clients.serverless_client.ServerlessClient._verify_credentials"
+    )
+    def test_jobs_with_function_filter(self, _verify_mock, jobs_mock):
+        """Tests that 'function' is forwarded and 'serverless' filter is enforced."""
+        serverless = QiskitServerless(
+            token="token", instance="instance", host="http://host"
+        )
+
+        my_function = QiskitFunction("my-func")
+
+        # Call jobs with both function and an additional kwarg
+        jobs = serverless.jobs(function=my_function, limit=7)
+
+        jobs_mock.assert_called()
+        called_args = jobs_mock.call_args.args
+        called_kwargs = jobs_mock.call_args.kwargs
+
+        # Positional args should be empty because we pass by keyword
+        assert called_args == ()
+
+        # Ensure 'function' got forwarded and 'filter' remained enforced
+        assert called_kwargs["function"] is my_function
+        assert called_kwargs["filter"] == "serverless"
+        assert called_kwargs["limit"] == 7
+
+        self.assertEqual(len(jobs), 1)
+        self.assertIsInstance(jobs[0], Job)
+        self.assertEqual(jobs[0].job_id, "42")