Removing redundancies

src/ansys/mapdl/core/launcher.py

@@ -70,7 +70,6 @@
 from ansys.mapdl.core.misc import (
     check_valid_ip,
     check_valid_port,
-    check_valid_start_instance,
     create_temp_dir,
     random_string,
     threaded,
@@ -778,20 +777,20 @@ def launch_remote_mapdl(
     )
 
 
-def get_start_instance(start_instance_default=True):
+def get_start_instance(start_instance: bool = True):
     """Check if the environment variable ``PYMAPDL_START_INSTANCE`` exists and is valid.
 
     Parameters
     ----------
-    start_instance_default : bool
+    start_instance : bool
         Value to return when ``PYMAPDL_START_INSTANCE`` is unset.
 
     Returns
     -------
     bool
         ``True`` when the ``PYMAPDL_START_INSTANCE`` environment variable is
         true, ``False`` when PYMAPDL_START_INSTANCE is false. If unset,
-        returns ``start_instance_default``.
+        returns ``start_instance``.
 
     Raises
     ------
@@ -800,25 +799,29 @@ def get_start_instance(start_instance_default=True):
         (case independent).
 
     """
-    if "PYMAPDL_START_INSTANCE" in os.environ:
-        if os.environ["PYMAPDL_START_INSTANCE"].lower() not in [
-            "true",
-            "false",
-        ]:
-            val = os.environ["PYMAPDL_START_INSTANCE"]
+    if "PYMAPDL_START_INSTANCE" in os.environ and os.environ["PYMAPDL_START_INSTANCE"]:
+        # It should not be empty
+        start_instance = os.environ["PYMAPDL_START_INSTANCE"]
+
+    if isinstance(start_instance, str):
+        start_instance = start_instance.lower().strip()
+        if start_instance not in ["true", "false"]:
             raise OSError(
-                f'Invalid value "{val}" for PYMAPDL_START_INSTANCE\n'
-                'PYMAPDL_START_INSTANCE should be either "TRUE" or "FALSE"'
+                f'Invalid value "{start_instance}" for "start_instance" (or "PYMAPDL_START_INSTANCE"\n'
+                '"start_instance" should be either "TRUE" or "FALSE"'
             )
+
+        LOG.debug(f"PYMAPDL_START_INSTANCE is set to {start_instance}")
+        return start_instance == "true"
+
+    elif isinstance(start_instance, bool):
         LOG.debug(
-            f"PYMAPDL_START_INSTANCE is set to {os.environ['PYMAPDL_START_INSTANCE']}"
+            f"PYMAPDL_START_INSTANCE is unset, using default value {start_instance}"
         )
-        return os.environ["PYMAPDL_START_INSTANCE"].lower() == "true"
+        return start_instance
 
-    LOG.debug(
-        f"PYMAPDL_START_INSTANCE is unset, using default value {start_instance_default}"
-    )
-    return start_instance_default
+    else:
+        raise ValueError("Only booleans are allowed as arguments.")
 
 
 def get_default_ansys():
@@ -1521,30 +1524,18 @@ def launch_mapdl(
 
     version = _verify_version(version)  # return a int version or none
 
-    # connect to an existing instance if enabled
-    if start_instance is None:
-        if "PYMAPDL_START_INSTANCE" in os.environ:
-            LOG.debug("Using PYMAPDL_START_INSTANCE environment variable.")
-
-        if os.environ.get("PYMAPDL_START_INSTANCE") and os.environ.get(
-            "PYMAPDL_START_INSTANCE"
-        ).lower() not in ["true", "false"]:
-            raise ValueError("PYMAPDL_START_INSTANCE must be either True or False.")
-
-        start_instance = check_valid_start_instance(
-            os.environ.get("PYMAPDL_START_INSTANCE", True)
-        )
-
-        LOG.debug("Using 'start_instance' equal to %s", start_instance)
+    # Getting "start_instance" using "True" as default.
+    start_instance = get_start_instance(True)
+    LOG.debug("Using 'start_instance' equal to %s", start_instance)
 
+    if start_instance:
         # special handling when building the gallery outside of CI. This
-        # creates an instance of mapdl the first time if PYMAPDL start instance
-        # is False.
+        # creates an instance of mapdl the first time.
         if pymapdl.BUILDING_GALLERY:  # pragma: no cover
             LOG.debug("Building gallery.")
             # launch an instance of pymapdl if it does not already exist and
             # we're allowed to start instances
-            if start_instance and GALLERY_INSTANCE[0] is None:
+            if GALLERY_INSTANCE[0] is None:
                 mapdl = launch_mapdl(
                     start_instance=True,
                     cleanup_on_exit=False,
@@ -1570,35 +1561,22 @@ def launch_mapdl(
                     mapdl.clear()
                 return mapdl
 
-                # finally, if running on CI/CD, connect to the default instance
-            else:
-                mapdl = MapdlGrpc(
-                    ip=ip,
-                    port=port,
-                    cleanup_on_exit=False,
-                    loglevel=loglevel,
-                    set_no_abort=set_no_abort,
-                    use_vtk=use_vtk,
-                    **start_parm,
-                )
-            if clear_on_connect:
-                mapdl.clear()
-            return mapdl
-
-    if not start_instance:
+    else:
         LOG.debug("Connecting to an existing instance of MAPDL at %s:%s", ip, port)
 
         if just_launch:
             print(f"There is an existing MAPDL instance at: {ip}:{port}")
             return
 
+        if pymapdl.BUILDING_GALLERY:  # pragma: no cover
+            LOG.debug("Building gallery.")
+
         mapdl = MapdlGrpc(
             ip=ip,
             port=port,
             cleanup_on_exit=False,
             loglevel=loglevel,
             set_no_abort=set_no_abort,
-            log_apdl=log_apdl,
             use_vtk=use_vtk,
             **start_parm,
         )

src/ansys/mapdl/core/misc.py

@@ -646,36 +646,6 @@ def check_valid_port(port, lower_bound=1000, high_bound=60000):
         )
 
 
-def check_valid_start_instance(start_instance):
-    """
-    Checks if the value obtained from the environmental variable is valid.
-
-    Parameters
-    ----------
-    start_instance : str
-        Value obtained from the corresponding environment variable.
-
-    Returns
-    -------
-    bool
-        Returns ``True`` if ``start_instance`` is ``True`` or ``"True"``,
-        ``False`` if otherwise.
-
-    """
-    if not isinstance(start_instance, (str, bool)):
-        raise ValueError("The value 'start_instance' should be an string or a boolean.")
-
-    if isinstance(start_instance, bool):
-        return start_instance
-
-    if start_instance.lower() not in ["true", "false"]:
-        raise ValueError(
-            f"The value 'start_instance' should be equal to 'True' or 'False' (case insensitive)."
-        )
-
-    return start_instance.lower() == "true"
-
-
 def update_information_first(update=False):
     """
     Decorator to wrap :class:`Information <ansys.mapdl.core.misc.Information>`

tests/test_launcher.py

@@ -44,12 +44,13 @@
     _parse_ip_route,
     _validate_MPI,
     _verify_version,
+    get_start_instance,
     launch_grpc,
     launch_mapdl,
     update_env_vars,
 )
 from ansys.mapdl.core.licensing import LICENSES
-from conftest import ON_LOCAL, QUICK_LAUNCH_SWITCHES, requires
+from conftest import ON_LOCAL, QUICK_LAUNCH_SWITCHES, NullContext, requires
 
 try:
     from ansys.tools.path import (
@@ -552,3 +553,45 @@ def test_deprecate_verbose():
 
     with pytest.raises(DeprecationError):
         launch_grpc(verbose=True)
+
+
+@pytest.mark.parametrize(
+    "start_instance,context",
+    [
+        pytest.param(True, NullContext(), id="Boolean true"),
+        pytest.param(False, NullContext(), id="Boolean false"),
+        pytest.param("true", NullContext(), id="String true"),
+        pytest.param("TRue", NullContext(), id="String true weird capitalization"),
+        pytest.param("2", pytest.raises(OSError), id="String number"),
+        pytest.param(2, pytest.raises(ValueError), id="Int"),
+    ],
+)
+def test_get_start_instance_argument(monkeypatch, start_instance, context):
+    if "PYMAPDL_START_INSTANCE" in os.environ:
+        monkeypatch.delenv("PYMAPDL_START_INSTANCE")
+    with context:
+        if "true" in str(start_instance).lower():
+            assert get_start_instance(start_instance)
+        else:
+            assert not get_start_instance(start_instance)
+
+
+@pytest.mark.parametrize(
+    "start_instance, context",
+    [
+        pytest.param("true", NullContext()),
+        pytest.param("TRue", NullContext()),
+        pytest.param("False", NullContext()),
+        pytest.param("FaLSE", NullContext()),
+        pytest.param("asdf", pytest.raises(OSError)),
+        pytest.param("1", pytest.raises(OSError)),
+        pytest.param("", pytest.raises(OSError)),
+    ],
+)
+def test_get_start_instance_envvar(monkeypatch, start_instance, context):
+    monkeypatch.setenv("PYMAPDL_START_INSTANCE", start_instance)
+    with context:
+        if "true" in start_instance.lower():
+            assert get_start_instance(start_instance)
+        else:
+            assert not get_start_instance(start_instance)

tests/test_misc.py

@@ -37,7 +37,6 @@
     check_valid_ip,
     check_valid_port,
     check_valid_routine,
-    check_valid_start_instance,
     last_created,
     load_file,
     no_return,
@@ -103,34 +102,6 @@ def test_check_valid_port_error(port):
         check_valid_port(port)
 
 
-@pytest.mark.parametrize(
-    "start_instance",
-    [
-        "true",
-        "TRue",
-        "False",
-        True,
-        False,
-    ],
-)
-def test_check_valid_start_instance(start_instance):
-    check_valid_start_instance(start_instance)
-
-
-@pytest.mark.parametrize(
-    "start_instance",
-    [
-        "asdf",
-        "2323",
-        1,
-        1e9,
-    ],
-)
-def test_check_valid_start_instance_error(start_instance):
-    with pytest.raises(ValueError):
-        check_valid_start_instance(start_instance)
-
-
 def test_creation_time(tmpdir):
     files_ = []
     for i in range(4):