From 522b81cb398abe1cb59fd2d2281d34f6e0d8ea5a Mon Sep 17 00:00:00 2001 From: Vaclav Petras Date: Wed, 15 Oct 2025 23:46:50 -0400 Subject: [PATCH 1/6] tests: Use string and subprocess for isolation Instead of using multiprocessing, use code in a string and subprocess with sys.executable to run code in isolation. While this is requires the tested code to be in a string as opposed to a function, this avoids many issues of multiprocessing (stuck queue on failure, complexities of spawning). It replaces queue communication with JSON in stdout which requires additional care in what is actually printed. As a result, the global environment is clearly not changed (it is a subprocess) as opposed to relying on multiprocessing behavior. For C library test, the separation of processes is even more clear, while at the same time, the test itself can now set up the runtime environment for dynamic libraries instead of relying on the external environmet. --- lib/gis/tests/lib_gis_env_test.py | 72 +++-- .../tests/grass_script_core_location_test.py | 49 +--- .../script/tests/grass_script_setup_test.py | 251 ++++++++++-------- 3 files changed, 186 insertions(+), 186 deletions(-) diff --git a/lib/gis/tests/lib_gis_env_test.py b/lib/gis/tests/lib_gis_env_test.py index 65f5448056c..487fe0fcba8 100644 --- a/lib/gis/tests/lib_gis_env_test.py +++ b/lib/gis/tests/lib_gis_env_test.py @@ -1,34 +1,46 @@ """Test environment and GIS environment functions""" -import multiprocessing +import os +import subprocess +import sys +import json +from textwrap import dedent -import pytest import grass.script as gs -xfail_mp_spawn = pytest.mark.xfail( - multiprocessing.get_start_method() == "spawn", - reason="Multiprocessing using 'spawn' start method requires pickable functions", - raises=AttributeError, - strict=True, -) +def run_in_subprocess(code, tmp_path, env): + """Code as a script in a separate process and return parsed JSON result -def run_in_subprocess(function): - """Run function in a separate process - - The function must take a Queue and put its result there. - The result is then returned from this function. + This is useful when we want to ensure that function like init does + not change the global environment for other tests. + Some effort is made to remove a current if any, but it does not touch + paths on purpose to enable dynamic library loading based on a variable. """ - queue = multiprocessing.Queue() - process = multiprocessing.Process(target=function, args=(queue,)) - process.start() - result = queue.get() - process.join() - return result + source_file = tmp_path / "test.py" + source_file.write_text(dedent(code)) + env = env.copy() + for variable in ("GISRC", "GISBASE", "GRASS_PREFIX"): + if variable in env: + del env[variable] + result = subprocess.run( + [sys.executable, os.fspath(source_file)], + stdout=subprocess.PIPE, + text=True, + check=True, + env=env, + ) + if not result.stdout: + msg = "Empty result from subprocess running code" + raise ValueError(msg) + try: + return json.loads(result.stdout) + except json.JSONDecodeError as error: + msg = f"Invalid JSON: {result.stdout}" + raise ValueError(msg) from error -@xfail_mp_spawn def test_reading_respects_change_of_session(tmp_path): """Check new session file path is retrieved and the file is read""" @@ -37,23 +49,27 @@ def test_reading_respects_change_of_session(tmp_path): # process-separated otherwise other tests will be influenced by the loaded # libraries and initialized data structures. - def switch_through_locations(queue): - """Switches through a list of locations""" - # Just to be sure we don't influence other tests. - # pylint: disable=import-outside-toplevel + code = f""" + # Switches through a list of projects + import json + from pathlib import Path + import grass.script as gs import grass.pygrass.utils as pygrass_utils import grass.lib.gis as libgis names = [] for project_name in ["test1", "test2", "abc"]: - gs.create_project(tmp_path / project_name) - with gs.setup.init(tmp_path / project_name): + gs.create_project(Path("{tmp_path}") / project_name) + with gs.setup.init(Path("{tmp_path}") / project_name): libgis.G__read_gisrc_path() libgis.G__read_gisrc_env() names.append((pygrass_utils.getenv("LOCATION_NAME"), project_name)) - queue.put(names) + print(json.dumps(names)) + """ - names = run_in_subprocess(switch_through_locations) + gs.create_project(tmp_path / "base") + with gs.setup.init(tmp_path / "base", env=os.environ.copy()) as session: + names = run_in_subprocess(code, tmp_path=tmp_path, env=session.env) for getenv_name, expected_name in names: assert getenv_name == expected_name, f"All recorded names: {names}" diff --git a/python/grass/script/tests/grass_script_core_location_test.py b/python/grass/script/tests/grass_script_core_location_test.py index 607877f24ce..b3003fd427b 100644 --- a/python/grass/script/tests/grass_script_core_location_test.py +++ b/python/grass/script/tests/grass_script_core_location_test.py @@ -1,6 +1,5 @@ """Test functions in grass.script.setup""" -import multiprocessing import os import pytest @@ -9,32 +8,15 @@ from grass.exceptions import ScriptError from grass.tools import Tools -xfail_mp_spawn = pytest.mark.xfail( - multiprocessing.get_start_method() == "spawn", - reason="Multiprocessing using 'spawn' start method requires pickable functions", - raises=AttributeError, - strict=True, -) +def test_with_same_path(tmp_path): + """Check correct EPSG is created with same path as the current one. -# This is useful when we want to ensure that function like init does -# not change the global environment. -def run_in_subprocess(function): - """Run function in a separate process - - The function must take a Queue and put its result there. - The result is then returned from this function. + Creates two projects, a XY bootstrap one and a desired project using + the same path (build for the case when only XY project can be created + which is no longer the case, but still it makes sense to test that as + a possible situation). """ - queue = multiprocessing.Queue() - process = multiprocessing.Process(target=function, args=(queue,)) - process.start() - result = queue.get() - process.join() - return result - - -def create_and_get_srid(tmp_path): - """Create location on the same path as the current one""" bootstrap = "bootstrap" desired = "desired" gs.create_project(tmp_path / bootstrap) @@ -46,29 +28,12 @@ def create_and_get_srid(tmp_path): gs.run_command("g.gisenv", set=f"GISDBASE={tmp_path}", env=session.env) gs.run_command("g.gisenv", set=f"LOCATION_NAME={desired}", env=session.env) gs.run_command("g.gisenv", set="MAPSET=PERMANENT", env=session.env) - return gs.parse_command("g.proj", flags="p", format="shell", env=session.env)[ + srid = gs.parse_command("g.proj", flags="p", format="shell", env=session.env)[ "srid" ] - - -def test_with_same_path(tmp_path): - """Check correct EPSG is created with same path as the current one""" - srid = create_and_get_srid(tmp_path) assert srid == "EPSG:3358" -@xfail_mp_spawn -def test_with_init_in_subprocess(tmp_path): - """Check creation when running in a subprocess""" - - def workload(queue): - """Transfer the return value using queue""" - queue.put(create_and_get_srid(tmp_path)) - - epsg = run_in_subprocess(workload) - assert epsg == "EPSG:3358" - - @pytest.mark.usefixtures("mock_no_session") def test_without_session(tmp_path): """Check that creation works outside of session. diff --git a/python/grass/script/tests/grass_script_setup_test.py b/python/grass/script/tests/grass_script_setup_test.py index e0c66ae1fd3..bbf3b426753 100644 --- a/python/grass/script/tests/grass_script_setup_test.py +++ b/python/grass/script/tests/grass_script_setup_test.py @@ -1,9 +1,10 @@ """Test functions in grass.script.setup""" -import multiprocessing import os import sys -from functools import partial +import subprocess +import json +from textwrap import dedent import pytest @@ -13,28 +14,35 @@ RUNTIME_GISBASE_SHOULD_BE_PRESENT = "Runtime (GISBASE) should be present" SESSION_FILE_NOT_DELETED = "Session file not deleted" -xfail_mp_spawn = pytest.mark.xfail( - multiprocessing.get_start_method() == "spawn", - reason="Multiprocessing using 'spawn' start method requires pickable functions", - raises=AttributeError, - strict=True, -) - -# This is useful when we want to ensure that function like init does -# not change the global environment. -def run_in_subprocess(function): - """Run function in a separate process +def run_in_subprocess(code, tmp_path): + """Code as a script in a separate process and return parsed JSON result - The function must take a Queue and put its result there. - The result is then returned from this function. + This is useful when we want to ensure that function like init does + not change the global environment for other tests. + Some effort is made to remove a current if any. """ - queue = multiprocessing.Queue() - process = multiprocessing.Process(target=function, args=(queue,)) - process.start() - result = queue.get() - process.join() - return result + source_file = tmp_path / "test.py" + source_file.write_text(dedent(code)) + env = os.environ.copy() + for variable in ("GISRC", "GISBASE", "GRASS_PREFIX"): + if variable in env: + del env[variable] + result = subprocess.run( + [sys.executable, os.fspath(source_file)], + stdout=subprocess.PIPE, + text=True, + check=True, + env=env, + ) + if not result.stdout: + msg = "Empty result from subprocess running code" + raise ValueError(msg) + try: + return json.loads(result.stdout) + except json.JSONDecodeError as error: + msg = f"Invalid JSON: {result.stdout}" + raise ValueError(msg) from error def test_init_as_context_manager(tmp_path): @@ -75,88 +83,84 @@ def test_init_finish_global_functions_with_env(tmp_path): assert not os.path.exists(session_file) -def init_finish_global_functions_capture_strerr0_partial(tmp_path, queue): - gs.set_capture_stderr(True) +def test_init_finish_global_functions_capture_strerr_enabled(tmp_path): + """Check that init and finish with global env with set_capture_stderr enabled""" project = tmp_path / "test" - gs.create_project(project) - gs.setup.init(project) - gs.run_command("g.region", flags="p") - runtime_present = bool(os.environ.get("GISBASE")) - queue.put((os.environ["GISRC"], runtime_present)) - gs.setup.finish() - - -def test_init_finish_global_functions_capture_strerr0_partial(tmp_path): - """Check that init and finish global functions work with global env using a partial - function - """ + code = f""" + import json + import os + import grass.script as gs - init_finish = partial( - init_finish_global_functions_capture_strerr0_partial, tmp_path - ) - session_file, runtime_present = run_in_subprocess(init_finish) - assert session_file, "Expected file name from the subprocess" - assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT - assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED - - -@xfail_mp_spawn -def test_init_finish_global_functions_capture_strerr0(tmp_path): - """Check that init and finish global functions work with global env""" - - def init_finish(queue): gs.set_capture_stderr(True) - project = tmp_path / "test" - gs.create_project(project) - gs.setup.init(project) - gs.run_command("g.region", flags="p") + gs.create_project("{project}") + gs.setup.init("{project}") + crs_type = gs.parse_command("g.region", flags="p", format="json")["crs"]["type"] runtime_present = bool(os.environ.get("GISBASE")) - queue.put((os.environ["GISRC"], runtime_present)) + result = {{ + "session_file": os.environ["GISRC"], + "runtime_present": runtime_present, + "crs_type": crs_type + }} gs.setup.finish() + print(json.dumps(result)) + """ + result = run_in_subprocess(code, tmp_path=tmp_path) + assert result["session_file"], "Expected file name from the subprocess" + assert result["runtime_present"], RUNTIME_GISBASE_SHOULD_BE_PRESENT + assert not os.path.exists(result["session_file"]), SESSION_FILE_NOT_DELETED + assert result["crs_type"] == "xy" - session_file, runtime_present = run_in_subprocess(init_finish) - assert session_file, "Expected file name from the subprocess" - assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT - assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED +def test_init_finish_global_functions_runtime_persists(tmp_path): + """Check that init and finish leave runtime behind -@xfail_mp_spawn -def test_init_finish_global_functions_capture_strerrX(tmp_path): - """Check that init and finish global functions work with global env""" + This is not necessarily desired behavior and it is not documented that way, + but it is the current implementation. + """ + project = tmp_path / "test" + code = f""" + import json + import os + import grass.script as gs - def init_finish(queue): gs.set_capture_stderr(True) - project = tmp_path / "test" - gs.create_project(project) - gs.setup.init(project) - gs.run_command("g.region", flags="p") + gs.create_project("{project}") + gs.setup.init("{project}") + region_data = gs.read_command("g.region", flags="p") runtime_present = bool(os.environ.get("GISBASE")) session_file = os.environ["GISRC"] gs.setup.finish() runtime_present_after = bool(os.environ.get("GISBASE")) - queue.put((session_file, runtime_present, runtime_present_after)) + print(json.dumps({{ + "session_file": session_file, + "runtime_present": runtime_present, + "runtime_present_after": runtime_present_after, + "region_data": region_data + }})) + """ - session_file, runtime_present, runtime_present_after = run_in_subprocess( - init_finish - ) - assert session_file, "Expected file name from the subprocess" - assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT - assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED + result = run_in_subprocess(code, tmp_path=tmp_path) + assert result["session_file"], "Expected file name from the subprocess" + assert result["runtime_present"], RUNTIME_GISBASE_SHOULD_BE_PRESENT + assert not os.path.exists(result["session_file"]), SESSION_FILE_NOT_DELETED # This is testing the current implementation behavior, but it is not required # to be this way in terms of design. - assert runtime_present_after, "Runtime should continue to be present" + assert result["runtime_present_after"], "Runtime should continue to be present" -@xfail_mp_spawn def test_init_finish_global_functions_isolated(tmp_path): """Check that init and finish global functions work with global env""" + project = tmp_path / "test" + + code = f""" + import json + import os + import grass.script as gs - def init_finish(queue): gs.set_capture_stderr(True) - project = tmp_path / "test" - gs.create_project(project) - gs.setup.init(project) - gs.run_command("g.region", flags="p") + gs.create_project("{project}") + gs.setup.init("{project}") + region_data = gs.parse_command("g.region", flags="p", format="json") runtime_present_during = bool(os.environ.get("GISBASE")) session_file_variable_present_during = bool(os.environ.get("GISRC")) session_file = os.environ.get("GISRC") @@ -167,60 +171,75 @@ def init_finish(queue): gs.setup.finish() session_file_variable_present_after = bool(os.environ.get("GISRC")) runtime_present_after = bool(os.environ.get("GISBASE")) - queue.put( - ( - session_file, - session_file_variable_present_during, - session_file_present_during, - session_file_variable_present_after, - runtime_present_during, - runtime_present_after, + print(json.dumps({{ + "session_file": session_file, + "session_file_variable_present_during": session_file_variable_present_during, + "session_file_present_during": session_file_present_during, + "session_file_variable_present_after": session_file_variable_present_after, + "runtime_present_during": runtime_present_during, + "runtime_present_after": runtime_present_after, + "region_data": region_data + }} ) ) + """ - ( - session_file, - session_file_variable_present_during, - session_file_present_during, - session_file_variable_present_after, - runtime_present_during, - runtime_present_after, - ) = run_in_subprocess(init_finish) + result = run_in_subprocess(code, tmp_path=tmp_path) # Runtime - assert runtime_present_during, RUNTIME_GISBASE_SHOULD_BE_PRESENT + assert result["runtime_present_during"], RUNTIME_GISBASE_SHOULD_BE_PRESENT # This is testing the current implementation behavior, but it is not required # to be this way in terms of design. - assert runtime_present_after, "Expected GISBASE to be present when finished" + assert result["runtime_present_after"], ( + "Expected GISBASE to be present when finished" + ) # Session - assert session_file_present_during, "Expected session file to be present" - assert session_file_variable_present_during, "Variable GISRC should be present" - assert not session_file_variable_present_after, "Not expecting GISRC when finished" - assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED + assert result["session_file_present_during"], "Expected session file to be present" + assert result["session_file_variable_present_during"], ( + "Variable GISRC should be present" + ) + assert not result["session_file_variable_present_after"], ( + "Not expecting GISRC when finished" + ) + assert not os.path.exists(result["session_file"]), SESSION_FILE_NOT_DELETED + + # Region + assert result["region_data"]["crs"]["type"] == "xy" -@xfail_mp_spawn @pytest.mark.usefixtures("mock_no_session") def test_init_as_context_manager_env_attribute(tmp_path): """Check that session has global environment as attribute""" - - def workload(queue): - project = tmp_path / "test" - gs.create_project(project) - with gs.setup.init(project) as session: - gs.run_command("g.region", flags="p", env=session.env) + project = tmp_path / "test" + code = f""" + import json + import os + import grass.script as gs + + gs.create_project("{project}") + with gs.setup.init("{project}") as session: + region_data = gs.parse_command( + "g.region", flags="p", format="json", env=session.env + ) session_file = os.environ["GISRC"] runtime_present = bool(os.environ.get("GISBASE")) - queue.put((session_file, os.path.exists(session_file), runtime_present)) - - session_file, file_existed, runtime_present = run_in_subprocess(workload) - assert session_file, "Expected file name from the subprocess" - assert file_existed, "File should have been present" - assert runtime_present, RUNTIME_GISBASE_SHOULD_BE_PRESENT - assert not os.path.exists(session_file), SESSION_FILE_NOT_DELETED + print(json.dumps({{ + "session_file": session_file, + "file_existed": os.path.exists(session_file), + "runtime_present": runtime_present, + "region_data": region_data + }})) + """ + + result = run_in_subprocess(code, tmp_path=tmp_path) + assert result["session_file"], "Expected file name from the subprocess" + assert result["file_existed"], "File should have been present" + assert result["runtime_present"], RUNTIME_GISBASE_SHOULD_BE_PRESENT + assert not os.path.exists(result["session_file"]), SESSION_FILE_NOT_DELETED assert not os.environ.get("GISRC") assert not os.environ.get("GISBASE") + assert result["region_data"]["crs"]["type"] == "xy" @pytest.mark.usefixtures("mock_no_session") From fbe4467ca7e4386cd5907f8679ec085d4a977f72 Mon Sep 17 00:00:00 2001 From: Vaclav Petras Date: Thu, 16 Oct 2025 09:21:54 -0400 Subject: [PATCH 2/6] The constructed script needs to use raw strings for the path to accomodate backslashes (as a way of dealing with Windows paths). --- lib/gis/tests/lib_gis_env_test.py | 4 ++-- .../script/tests/grass_script_setup_test.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/gis/tests/lib_gis_env_test.py b/lib/gis/tests/lib_gis_env_test.py index 487fe0fcba8..4a3520ed9d1 100644 --- a/lib/gis/tests/lib_gis_env_test.py +++ b/lib/gis/tests/lib_gis_env_test.py @@ -59,8 +59,8 @@ def test_reading_respects_change_of_session(tmp_path): names = [] for project_name in ["test1", "test2", "abc"]: - gs.create_project(Path("{tmp_path}") / project_name) - with gs.setup.init(Path("{tmp_path}") / project_name): + gs.create_project(Path(r"{tmp_path}") / project_name) + with gs.setup.init(Path(r"{tmp_path}") / project_name): libgis.G__read_gisrc_path() libgis.G__read_gisrc_env() names.append((pygrass_utils.getenv("LOCATION_NAME"), project_name)) diff --git a/python/grass/script/tests/grass_script_setup_test.py b/python/grass/script/tests/grass_script_setup_test.py index bbf3b426753..5881bac7f95 100644 --- a/python/grass/script/tests/grass_script_setup_test.py +++ b/python/grass/script/tests/grass_script_setup_test.py @@ -92,8 +92,8 @@ def test_init_finish_global_functions_capture_strerr_enabled(tmp_path): import grass.script as gs gs.set_capture_stderr(True) - gs.create_project("{project}") - gs.setup.init("{project}") + gs.create_project(r"{project}") + gs.setup.init(r"{project}") crs_type = gs.parse_command("g.region", flags="p", format="json")["crs"]["type"] runtime_present = bool(os.environ.get("GISBASE")) result = {{ @@ -124,8 +124,8 @@ def test_init_finish_global_functions_runtime_persists(tmp_path): import grass.script as gs gs.set_capture_stderr(True) - gs.create_project("{project}") - gs.setup.init("{project}") + gs.create_project(r"{project}") + gs.setup.init(r"{project}") region_data = gs.read_command("g.region", flags="p") runtime_present = bool(os.environ.get("GISBASE")) session_file = os.environ["GISRC"] @@ -158,8 +158,8 @@ def test_init_finish_global_functions_isolated(tmp_path): import grass.script as gs gs.set_capture_stderr(True) - gs.create_project("{project}") - gs.setup.init("{project}") + gs.create_project(r"{project}") + gs.setup.init(r"{project}") region_data = gs.parse_command("g.region", flags="p", format="json") runtime_present_during = bool(os.environ.get("GISBASE")) session_file_variable_present_during = bool(os.environ.get("GISRC")) @@ -217,8 +217,8 @@ def test_init_as_context_manager_env_attribute(tmp_path): import os import grass.script as gs - gs.create_project("{project}") - with gs.setup.init("{project}") as session: + gs.create_project(r"{project}") + with gs.setup.init(r"{project}") as session: region_data = gs.parse_command( "g.region", flags="p", format="json", env=session.env ) From 39ecfeba6ecfc21983ee91d7128ee14e71131de6 Mon Sep 17 00:00:00 2001 From: Vaclav Petras Date: Thu, 16 Oct 2025 11:21:30 -0400 Subject: [PATCH 3/6] Use exception to report stderr to ensure it is reported instead of relying on stderr being printed through pytest (does not show up on Windows in CI) --- lib/gis/tests/lib_gis_env_test.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/gis/tests/lib_gis_env_test.py b/lib/gis/tests/lib_gis_env_test.py index 4a3520ed9d1..7ba33d02c97 100644 --- a/lib/gis/tests/lib_gis_env_test.py +++ b/lib/gis/tests/lib_gis_env_test.py @@ -16,21 +16,34 @@ def run_in_subprocess(code, tmp_path, env): This is useful when we want to ensure that function like init does not change the global environment for other tests. Some effort is made to remove a current if any, but it does not touch - paths on purpose to enable dynamic library loading based on a variable. + runtime variables on purpose to enable dynamic library loading based on + a path variable. """ source_file = tmp_path / "test.py" source_file.write_text(dedent(code)) env = env.copy() - for variable in ("GISRC", "GISBASE", "GRASS_PREFIX"): + for variable in ("GISRC", "GIS_LOCK"): if variable in env: del env[variable] result = subprocess.run( [sys.executable, os.fspath(source_file)], - stdout=subprocess.PIPE, + capture_output=True, text=True, - check=True, + check=False, env=env, ) + if result.returncode != 0: + if result.stderr: + msg = ( + "Execution of code in subprocess failed, " + f"captured stderr from subprocess:\n{result.stderr}\n" + ) + else: + msg = ( + f"Execution of code in subprocess gave return code {result.returncode}" + "but there was no stderr" + ) + raise RuntimeError(msg) if not result.stdout: msg = "Empty result from subprocess running code" raise ValueError(msg) From 8eedb8b2a5f42a51a04534883daed661b3a894c7 Mon Sep 17 00:00:00 2001 From: Vaclav Petras Date: Thu, 16 Oct 2025 11:25:48 -0400 Subject: [PATCH 4/6] Document tests with their new/current scope. Add more capture_stderr smoke testing. Be more consitent in mocking no global session. --- python/grass/script/tests/conftest.py | 4 + .../script/tests/grass_script_setup_test.py | 80 +++++++++++++------ 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/python/grass/script/tests/conftest.py b/python/grass/script/tests/conftest.py index d8e85428344..e07eb568124 100644 --- a/python/grass/script/tests/conftest.py +++ b/python/grass/script/tests/conftest.py @@ -20,8 +20,12 @@ def test_session_handling(): There may or may not be a session in the background (we don't check either way). """ + # Session monkeypatch.delenv("GISRC", raising=False) + monkeypatch.delenv("GIS_LOCK", raising=False) + # Runtime monkeypatch.delenv("GISBASE", raising=False) + monkeypatch.delenv("GRASS_PREFIX", raising=False) @pytest.fixture diff --git a/python/grass/script/tests/grass_script_setup_test.py b/python/grass/script/tests/grass_script_setup_test.py index 5881bac7f95..fb8be8e825e 100644 --- a/python/grass/script/tests/grass_script_setup_test.py +++ b/python/grass/script/tests/grass_script_setup_test.py @@ -9,25 +9,26 @@ import pytest import grass.script as gs +from grass.exceptions import CalledModuleError from grass.app.data import MapsetLockingException RUNTIME_GISBASE_SHOULD_BE_PRESENT = "Runtime (GISBASE) should be present" SESSION_FILE_NOT_DELETED = "Session file not deleted" -def run_in_subprocess(code, tmp_path): +def run_in_subprocess(code, tmp_path, env=None): """Code as a script in a separate process and return parsed JSON result This is useful when we want to ensure that function like init does not change the global environment for other tests. - Some effort is made to remove a current if any. + The current environment is used for the subprocess as is, so any + existing global session is passed to the subprocess or it needs to + be removed beforehand, e.g., through the mock_no_session fixture. """ source_file = tmp_path / "test.py" source_file.write_text(dedent(code)) - env = os.environ.copy() - for variable in ("GISRC", "GISBASE", "GRASS_PREFIX"): - if variable in env: - del env[variable] + if env is None: + env = os.environ result = subprocess.run( [sys.executable, os.fspath(source_file)], stdout=subprocess.PIPE, @@ -45,8 +46,9 @@ def run_in_subprocess(code, tmp_path): raise ValueError(msg) from error +@pytest.mark.usefixtures("mock_no_session") def test_init_as_context_manager(tmp_path): - """Check that init function return value works as a context manager""" + """Check that init function's return value works as a context manager""" project = tmp_path / "test" gs.create_project(project) with gs.setup.init(project, env=os.environ.copy()) as session: @@ -56,6 +58,7 @@ def test_init_as_context_manager(tmp_path): assert not os.path.exists(session_file) +@pytest.mark.usefixtures("mock_no_session") def test_init_session_finish(tmp_path): """Check that init works with finish on the returned session object""" project = tmp_path / "test" @@ -64,12 +67,35 @@ def test_init_session_finish(tmp_path): gs.run_command("g.region", flags="p", env=session.env) session_file = session.env["GISRC"] session.finish() - with pytest.raises(ValueError): # noqa: PT011 + # When operating on the object, the error message should be clear + # about the session being finished (as opposed to not existing session). + with pytest.raises(ValueError, match="finished session"): session.finish() assert not session.active assert not os.path.exists(session_file) +@pytest.mark.usefixtures("mock_no_session") +def test_global_finish_session_only_in_env(tmp_path): + """Check that init works with finish on the returned session object""" + project = tmp_path / "test" + gs.create_project(project) + session = gs.setup.init(project, env=os.environ.copy()) + gs.run_command("g.region", flags="p", env=session.env) + session_file = session.env["GISRC"] + session.finish() + # Test preconditions + assert not session.active + assert not os.path.exists(session_file) + # The global function can only know that a session does not exist, + # but not that it existed but has been finished. + # On top of that, without capured stderr, the error message has + # only the tool name, but actual error is in stderr. + with pytest.raises(CalledModuleError, match=r"g\.gisenv"): + gs.setup.finish(env=session.env) + + +@pytest.mark.usefixtures("mock_no_session") def test_init_finish_global_functions_with_env(tmp_path): """Check that init and finish global functions work""" project = tmp_path / "test" @@ -83,15 +109,23 @@ def test_init_finish_global_functions_with_env(tmp_path): assert not os.path.exists(session_file) -def test_init_finish_global_functions_capture_strerr_enabled(tmp_path): - """Check that init and finish with global env with set_capture_stderr enabled""" +@pytest.mark.parametrize("capture_stderr", [True, False, None]) +@pytest.mark.usefixtures("mock_no_session") +def test_init_finish_global_functions_capture_strerr(tmp_path, capture_stderr): + """Check init and finish with global env and with set_capture_stderr. + + This actually runs a tool in the created environment and checks its output. + Capturing or not capturing stderr should not influence the result + (in is global for grass.script so it may interfere tool calls inside + grass.script.setup). + """ project = tmp_path / "test" code = f""" import json import os import grass.script as gs - gs.set_capture_stderr(True) + gs.set_capture_stderr({capture_stderr}) gs.create_project(r"{project}") gs.setup.init(r"{project}") crs_type = gs.parse_command("g.region", flags="p", format="json")["crs"]["type"] @@ -111,11 +145,13 @@ def test_init_finish_global_functions_capture_strerr_enabled(tmp_path): assert result["crs_type"] == "xy" +@pytest.mark.usefixtures("mock_no_session") def test_init_finish_global_functions_runtime_persists(tmp_path): - """Check that init and finish leave runtime behind + """Check that init and finish leave runtime behind. This is not necessarily desired behavior and it is not documented that way, - but it is the current implementation. + but it is the current implementation, so we test the expected behavior + (which may change in the future). """ project = tmp_path / "test" code = f""" @@ -126,7 +162,7 @@ def test_init_finish_global_functions_runtime_persists(tmp_path): gs.set_capture_stderr(True) gs.create_project(r"{project}") gs.setup.init(r"{project}") - region_data = gs.read_command("g.region", flags="p") + region_data = gs.parse_command("g.region", flags="p", format="json") runtime_present = bool(os.environ.get("GISBASE")) session_file = os.environ["GISRC"] gs.setup.finish() @@ -146,10 +182,15 @@ def test_init_finish_global_functions_runtime_persists(tmp_path): # This is testing the current implementation behavior, but it is not required # to be this way in terms of design. assert result["runtime_present_after"], "Runtime should continue to be present" + assert result["region_data"]["crs"]["type"] == "xy" + +@pytest.mark.usefixtures("mock_no_session") +def test_init_finish_global_functions_set_environment(tmp_path): + """Check that init and finish global functions work with global env. -def test_init_finish_global_functions_isolated(tmp_path): - """Check that init and finish global functions work with global env""" + This checks the session without running a tool. + """ project = tmp_path / "test" code = f""" @@ -157,10 +198,8 @@ def test_init_finish_global_functions_isolated(tmp_path): import os import grass.script as gs - gs.set_capture_stderr(True) gs.create_project(r"{project}") gs.setup.init(r"{project}") - region_data = gs.parse_command("g.region", flags="p", format="json") runtime_present_during = bool(os.environ.get("GISBASE")) session_file_variable_present_during = bool(os.environ.get("GISRC")) session_file = os.environ.get("GISRC") @@ -178,12 +217,10 @@ def test_init_finish_global_functions_isolated(tmp_path): "session_file_variable_present_after": session_file_variable_present_after, "runtime_present_during": runtime_present_during, "runtime_present_after": runtime_present_after, - "region_data": region_data }} ) ) """ - result = run_in_subprocess(code, tmp_path=tmp_path) # Runtime @@ -204,9 +241,6 @@ def test_init_finish_global_functions_isolated(tmp_path): ) assert not os.path.exists(result["session_file"]), SESSION_FILE_NOT_DELETED - # Region - assert result["region_data"]["crs"]["type"] == "xy" - @pytest.mark.usefixtures("mock_no_session") def test_init_as_context_manager_env_attribute(tmp_path): From 0722e40aab6544880d74042c3f82aaa633bfedb6 Mon Sep 17 00:00:00 2001 From: Vaclav Petras Date: Thu, 16 Oct 2025 11:28:05 -0400 Subject: [PATCH 5/6] Use exception to report stderr also in the grass.script.setup test --- .../script/tests/grass_script_setup_test.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/python/grass/script/tests/grass_script_setup_test.py b/python/grass/script/tests/grass_script_setup_test.py index fb8be8e825e..9f24ab7f8b6 100644 --- a/python/grass/script/tests/grass_script_setup_test.py +++ b/python/grass/script/tests/grass_script_setup_test.py @@ -31,11 +31,23 @@ def run_in_subprocess(code, tmp_path, env=None): env = os.environ result = subprocess.run( [sys.executable, os.fspath(source_file)], - stdout=subprocess.PIPE, + capture_output=True, text=True, - check=True, + check=False, env=env, ) + if result.returncode != 0: + if result.stderr: + msg = ( + "Execution of code in subprocess failed, " + f"captured stderr from subprocess:\n{result.stderr}\n" + ) + else: + msg = ( + f"Execution of code in subprocess gave return code {result.returncode}" + "but there was no stderr" + ) + raise RuntimeError(msg) if not result.stdout: msg = "Empty result from subprocess running code" raise ValueError(msg) From 704b73ed2e31b8d00c3919c118acfb87ccc22c52 Mon Sep 17 00:00:00 2001 From: Vaclav Petras Date: Thu, 16 Oct 2025 13:10:59 -0400 Subject: [PATCH 6/6] Skip the library test on Windows. The environment is not updated from Python (it is updated, but the getter function keeps using a cached version), so the C functions don't see GISRC. --- lib/gis/tests/lib_gis_env_test.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/gis/tests/lib_gis_env_test.py b/lib/gis/tests/lib_gis_env_test.py index 7ba33d02c97..5a2a3607360 100644 --- a/lib/gis/tests/lib_gis_env_test.py +++ b/lib/gis/tests/lib_gis_env_test.py @@ -6,6 +6,7 @@ import json from textwrap import dedent +import pytest import grass.script as gs @@ -54,6 +55,13 @@ def run_in_subprocess(code, tmp_path, env): raise ValueError(msg) from error +# To read the new variables on Windows, another subprocess would be needed, +# but that would not allow for the dynamic changes of the session variables within +# a single process which is what this test is testing. +@pytest.mark.skipif( + sys.platform.startswith("win"), + reason="On Windows, C has a cached environment, so the libraries don't know about the session.", +) def test_reading_respects_change_of_session(tmp_path): """Check new session file path is retrieved and the file is read"""