diff --git a/lib/gis/tests/lib_gis_env_test.py b/lib/gis/tests/lib_gis_env_test.py index 65f5448056c..5a2a3607360 100644 --- a/lib/gis/tests/lib_gis_env_test.py +++ b/lib/gis/tests/lib_gis_env_test.py @@ -1,34 +1,67 @@ """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(function): - """Run function in a separate process +def run_in_subprocess(code, tmp_path, env): + """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, but it does not touch + runtime variables on purpose to enable dynamic library loading based on + a path 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", "GIS_LOCK"): + if variable in env: + del env[variable] + result = subprocess.run( + [sys.executable, os.fspath(source_file)], + capture_output=True, + text=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) + 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 +# 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""" @@ -37,23 +70,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(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)) - 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/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_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..9f24ab7f8b6 100644 --- a/python/grass/script/tests/grass_script_setup_test.py +++ b/python/grass/script/tests/grass_script_setup_test.py @@ -1,44 +1,66 @@ """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 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" -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, env=None): + """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. + 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. """ - 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)) + if env is None: + env = os.environ + result = subprocess.run( + [sys.executable, os.fspath(source_file)], + capture_output=True, + text=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) + try: + return json.loads(result.stdout) + except json.JSONDecodeError as error: + msg = f"Invalid JSON: {result.stdout}" + 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: @@ -48,6 +70,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" @@ -56,12 +79,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" @@ -75,88 +121,97 @@ 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) - 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() - +@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. -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 + 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). """ - - 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") + project = tmp_path / "test" + code = f""" + import json + import os + import grass.script as gs + + 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"] 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 +@pytest.mark.usefixtures("mock_no_session") +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, so we test the expected behavior + (which may change in the future). + """ + 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(r"{project}") + gs.setup.init(r"{project}") + 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() 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" + 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. -@xfail_mp_spawn -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" - 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") + code = f""" + import json + import os + import grass.script as gs + + gs.create_project(r"{project}") + gs.setup.init(r"{project}") 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 +222,70 @@ 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, + }} ) ) - - ( - 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 -@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(r"{project}") + with gs.setup.init(r"{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")