python-cmake / pytest-cmake

Pytest module for CMake
https://python-cmake.github.io/pytest-cmake/
MIT License
27 stars 2 forks source link

Environment variables are mangled by pytest-cmake #22

Closed ZbigniewRA closed 6 months ago

ZbigniewRA commented 7 months ago

Environment variables are mangled by pytest-cmake==0.5.1 on Windows. pytest-cmake==0.4.1 works fine.

For example PATH variable with content like C:\Hello;;C:\Good\bye becomes C:/Hello\\;\\;C:/Good/bye\\. So:

I think this is caused by invalid code somewhere here: https://github.com/buddly27/pytest-cmake/commit/f498cabe5840c71ce6632fdb853c6dbc231bd5ca#diff-b053514ed18dd3e50bf5307d5f659bb1e8d8105dc4afe373937aa330029d5d86L10 (I don't fully understand that code, so can't be certain.)

buddly27 commented 6 months ago

The semi-colon usage as a path separator for environment variables in Windows conflicted with its use as a list delimiter in the CMake script language. So, it is escaped before writing the test files (\\;). Otherwise, only the first path was considered (ref).

But this update was done for 0.4.0 so it might not be related to your issue if you say it works with 0.4.1.

Could you give me a copy of the pytest_discover_tests function usage to help investigate the issue?

buddly27 commented 6 months ago

Without further details, I am guessing that you are trying to debug a collection error, as exposing them has been the main update since v0.4.1, and you said that this version was working fine.

The "mangling" of environment variables you describe is a conversion from Windows paths into "Unix-like" paths. It is done via the cmake_path function in the module finder script: https://github.com/buddly27/pytest-cmake/blob/2adb9e7690e6834f49046e888a13066d57ff580e/cmake/FindPytest.cmake#L75-L77

Using forward slashes for paths is easier when passing them as arguments to the CMake script responsible for creating tests and serializing them to the CTest script responsible for running the tests (Only the semicolons need to be escaped on Windows paths, as mentioned in my previous comment).

As far as I can tell, there is no need to convert them back to the backslash syntax, as all Windows-compatible shells seem to support this format. https://learn.microsoft.com/en-us/cpp/c-runtime-library/unix?view=msvc-170

So I don't think this conversion caused your hypothetical collection issue, but I understand that it could cause confusion when debugging, so I'll update the CMake script to convert it back to "native format" before serialization.

ZbigniewRA commented 6 months ago

Hi. Thanks for looking into this.

By mangling I mean what I've provided in example: PATH variable with content like C:\Hello;;C:\Good\bye becomes C:/Hello\\;\\;C:/Good/bye\\. Please note that it was not about escaping ; inside CMake generated files. Those were the values actually set as the PATH variable visible by the tests executable. At that point any escaping should no longer exist. This was problematic, because \\ is not a valid path, which caused issues.

I have reverted to version 0.3.0 of pytest-cmake, because I actually had problems with 0.4.1 as well (I didn't get to the bottom of it, but tests were not discovered properly).

buddly27 commented 6 months ago

I'm having a hard time trying to reproduce this error. I am building and running tests on the example as follows:

$ [Environment]::SetEnvironmentVariable("PATH", "$($env:Path);C:\Hello;;C:\Good\bye")
$ cmake -D "CMAKE_MODULE_PATH:FILEPATH=${BOOST_CMAKE_CONFIG}" -S . -B .\build
$ cmake --build .\build\ --config Release
$ ctest --test-dir .\build\ -VV --config Release

But I see the following value within PATH (ignoring escaped semi-colons):

PATH=...;C:/Hello;C:/Good/bye

The cmake_path function should have removed the empty path, so I don't get where these double backslash and extra semi-colons come from.

cmake_path(CONVERT "$ENV{PATH}" TO_CMAKE_PATH_LIST value) # C:/Hello;C:/Good/bye

I have reverted to version 0.3.0 of pytest-cmake, because I actually had problems with 0.4.1 as well (I didn't get to the bottom of it, but tests were not discovered properly).

The issue with 0.3.0 was that the environment variables were not correctly serialized, which means that only the first path included in the PATH environment variable was used when running the tests on Windows. So, I'm unsure if you encountered this mangling issue because of a workaround you used to get around this limitation or if this is very specific to your use case.

Could you please provide me with a detailed example that can be reproduced?

Also, please provide the following details:

ZbigniewRA commented 6 months ago

Hi, I am on Windows 10 and using cmd. I have tried to reproduce this problem on the same code I used before, and now I don't have a problem. I also tried on a synthetic example - also works. So I am not sure what was wrong before... I did upgrade Visual Studio and CMake in the mean time, so that might have been a factor. Thatnk you for checking it, and I am sorry for wasting your time.

buddly27 commented 6 months ago

No problem, thanks for the update!