resolve pr

Author: abhaasgoyal

meorg_client/tests/conftest.py

@@ -9,37 +9,13 @@
 os.environ["MEORG_DEV_MODE"] = "1"
 
 
-class ValueStorage:
-    def __init__(self):
-        self.data = dict()
-
-    def get(self, key):
-        return self.data.get(key, None)
-
-    def set(self, key, value):
-        self.data[key] = value
-
-    def __repr__(self):
-        lines = ""
-        for k, v in self.data.items():
-            lines += f"{k} = {v}\n"
-
-        return lines
-
-
-# Add some things to the store
-store = ValueStorage()
-store.set("email", os.environ.get("MEORG_EMAIL"))
-store.set("password", os.environ.get("MEORG_PASSWORD"))
-store.set("model_output_id", os.environ.get("MEORG_MODEL_OUTPUT_ID"))
-store.set("experiment_id", os.environ.get("MEORG_EXPERIMENT_ID"))
-store.set("model_profile_id", os.environ.get("MEORG_MODEL_PROFILE_ID"))
-store.set("model_output_name", os.environ.get("MEORG_MODEL_OUTPUT_NAME"))
-
-
 # https://docs.pytest.org/en/latest/example/simple.html#making-test-result-information-available-in-fixtures
 @pytest.hookimpl(wrapper=True, tryfirst=True)
 def pytest_runtest_makereport(item, call):
+    """Have more information on the status for pytests.
+
+    The results can be used within fixtures
+    """
     # execute all other hooks to obtain the report object
     rep = yield
 

meorg_client/tests/test_cli.py

@@ -34,9 +34,33 @@ def test_filepath() -> str:
 
 @pytest.fixture
 def model_output_generator(request, runner: CliRunner, model_profile_id: str):
+    """A generator function for creating new model outputs before running a test.
+
+       After the test has run, automatically deletes all model outputs created within
+       the test
+
+    Parameters
+    -------
+    request:
+        Request object by pytest
+
+    click.testing.CliRunner
+        Runner object.
+
+    model_profile_id
+        Model profile ID.
+    """
     model_output_ids = []
 
     def _make_model_output(model_output_name):
+        """Create new model output ID.
+
+        Parameters
+        -------
+        name:
+            Model output name
+        """
+        # `model_profile_id` from `model_output_generator`
         result = runner.invoke(
             cli.create_new_model_output,
             [model_profile_id, model_output_name],
@@ -49,10 +73,13 @@ def _make_model_output(model_output_name):
 
     yield _make_model_output
 
-    # Used if already deleted
+    # If using tests like model_output_delete, where model output is already deleted,
+    # we don't want to do the teardown process
     if hasattr(request.node, "skip_teardown"):
         return
 
+    # If test failed, for debugging purposes, we want to keep the model output in
+    # me.org
     report = request.node.stash[phase_report_key]
     if report["call"].failed:
         print("Call to test failed", request.node.nodeid)
@@ -99,7 +126,7 @@ def test_create_model_output(
 
         assert result.exit_code == 0
         model_output_id = result.return_value
-        assert type(model_output_id) is str  # The new model output
+        assert isinstance(model_output_id, str)  # The new model output
 
     def test_model_output_query(self, runner: CliRunner, model_output_id: str):
         """Test Existing Model output."""
@@ -148,27 +175,24 @@ class TestBenchmark:
     def model_output_id(
         self, runner: CliRunner, model_output_generator, experiment_id: str
     ):
-        id1 = model_output_generator("meorg_test_benchmark1")
-        id2 = model_output_generator("meorg_test_benchmark2")
-        runner.invoke(
-            cli.model_output_experiments_extend,
-            [id1, experiment_id],
-        )
-        runner.invoke(
-            cli.model_output_experiments_extend,
-            [id2, experiment_id],
-        )
-        return id1
+        latest_id = None
+        for i in range(2):
+            latest_id = model_output_generator(f"meorg_test_benchmark{i}")
+            runner.invoke(
+                cli.model_output_experiments_extend,
+                [latest_id, experiment_id],
+            )
+        return latest_id
 
     def _check_available_benchmarks(self, result, expected):
         available_benchmarks = result.get("benchmarks")
-        assert type(available_benchmarks) is list
+        assert isinstance(available_benchmarks, list)
         assert len(available_benchmarks) == expected
         return available_benchmarks
 
     def _check_current_benchmarks(self, result, expected):
         current_benchmarks = result.get("current")
-        assert type(current_benchmarks) is list
+        assert isinstance(current_benchmarks, list)
         assert len(current_benchmarks) == expected
         return current_benchmarks
 

meorg_client/tests/test_client.py

@@ -82,19 +82,47 @@ def test_list_endpoints(client: Client):
 
 @pytest.fixture
 def model_output_generator(request, client: Client, model_profile_id: str):
+    """A generator function for creating new model outputs before running a test.
+
+       After the test has run, automatically deletes all model outputs created within
+       the test
+
+    Parameters
+    -------
+    request:
+        Request object by pytest
+
+    client : Client
+        Client.
+
+    model_profile_id
+        Model profile ID.
+    """
     model_output_ids = []
 
     def _make_model_output(model_output_name):
+        """Create new model output ID.
+
+        Parameters
+        -------
+        name:
+            Model output name
+        """
+        # `model_profile_id` from `model_output_generator`
         response = client.model_output_create(model_profile_id, model_output_name)
         model_output_id = response.get("data").get("modeloutput")
         model_output_ids.append(model_output_id)
         return model_output_id
 
     yield _make_model_output
 
+    # If using tests like model_output_delete, where model output is already deleted,
+    # we don't want to do the teardown process
     if hasattr(request.node, "skip_teardown"):
         return
 
+    # If test failed, for debugging purposes, we want to keep the model output in
+    # me.org
     report = request.node.stash[phase_report_key]
     if report["call"].failed:
         print("Call to test failed", request.node.nodeid)
@@ -170,21 +198,21 @@ class TestBenchmark:
     def model_output_id(
         self, client: Client, model_output_generator, experiment_id: str
     ):
-        id1 = model_output_generator("meorg_test_benchmark1")
-        id2 = model_output_generator("meorg_test_benchmark2")
-        client.model_output_experiments_extend(id1, [experiment_id])
-        client.model_output_experiments_extend(id2, [experiment_id])
-        return id1
+        latest_id = None
+        for i in range(2):
+            latest_id = model_output_generator(f"meorg_test_benchmark{i}")
+            client.model_output_experiments_extend(latest_id, [experiment_id])
+        return latest_id
 
     def _check_available_benchmarks(self, response, expected):
         available_benchmarks = response.get("data").get("benchmarks")
-        assert type(available_benchmarks) is list
+        assert isinstance(available_benchmarks, list)
         assert len(available_benchmarks) == expected
         return available_benchmarks
 
     def _check_current_benchmarks(self, response, expected):
         current_benchmarks = response.get("data").get("current")
-        assert type(current_benchmarks) is list
+        assert isinstance(current_benchmarks, list)
         assert len(current_benchmarks) == expected
         return current_benchmarks