Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grass.jupyter: Fix session tests on Windows #4368

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

echoix
Copy link
Member

@echoix echoix commented Sep 23, 2024

The interpolated path strings aren't escaped when evaluated, so using a raw string allows C:\Users to not have an invalid \U escape sequence.

@echoix echoix added this to the 8.5.0 milestone Sep 23, 2024
@github-actions github-actions bot added Python Related code is in Python libraries notebook labels Sep 23, 2024
@echoix echoix force-pushed the grass-jupyter-session-test-windows branch from 7b42da0 to 1554e3b Compare September 23, 2024 16:42
The interpolated path strings aren't escaped when evaluated, so using a
raw string allows C:\Users to not have an invalid \U escape sequence.
@echoix echoix force-pushed the grass-jupyter-session-test-windows branch from 1554e3b to 5b46b84 Compare September 23, 2024 17:55
@wenzeslaus
Copy link
Member

The diff is okay, but I find this hard to believe and would prefer the original, simpler code. Can you provide a code example or an article? I can't find or reproduce anything myself.

@echoix
Copy link
Member Author

echoix commented Sep 23, 2024

It is quite simple, as the alternative is to find a way to re-escape the string from an evaluated path.

Take a look at a run on main, the script that is saved and executed doesn't have the backslashes escaped anymore, since it was evaluated when the file was written from a string. https://github.com/OSGeo/grass/actions/runs/10992700384/job/30517619655#step:12:857

image


---------------------------- Captured stderr call -----------------------------
  File "C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\test_init_finish0\script.py", line 5

    gs.core._create_location_xy("C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\test_init_finish0", "test")

                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 2-3: truncated \UXXXXXXXX escape

From the next test (but we can see the equivalent way up), the tmp_path fixture is given as a pathlib Path, like:


tmp_path = WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/test_init_with_auto_finish0')

At the next test, we also see what an escaped path would look like:


_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
python\grass\jupyter\tests\grass_jupyter_session_test.py:19: in run_in_subprocess
    process = subprocess.run(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

input = None, capture_output = False, timeout = None, check = True
popenargs = (['C:\\OSGeo4W\\bin\\python3.exe', 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-runneradmin\\pytest-0\\test_init_with_auto_finish0\\script.py'],)
kwargs = {'stdout': -1, 'text': True}
process = <Popen: returncode: 1 args: ['C:\\OSGeo4W\\bin\\python3.exe', 'C:\\Users\\ru...>
stdout = '', stderr = None, retcode = 1

@echoix
Copy link
Member Author

echoix commented Sep 23, 2024

Note that the script here is just a string, that is then called, its not Python code yet..

@wenzeslaus
Copy link
Member

Note that the script here is just a string, that is then called, its not Python code yet..

That's the part I missed! Thanks for pointing that out.

@echoix echoix merged commit 16de4d3 into OSGeo:main Sep 24, 2024
26 checks passed
@echoix echoix deleted the grass-jupyter-session-test-windows branch September 24, 2024 01:16
@neteler neteler modified the milestone: 8.5.0 Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries notebook Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants