-
Notifications
You must be signed in to change notification settings - Fork 185
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
Fix python CLI CI test #2846
Fix python CLI CI test #2846
Conversation
This adds a new function argument to add_bash_tests() to specify environment variables that should be defined for running a test.
This comment was marked as outdated.
This comment was marked as outdated.
Interface of sed differs between MacOSX and Unix, such that the CI test of only the former would fail.
1974fee
to
d9587e6
Compare
clang-tidy review says "All clean, LGTM! 👍" |
d9587e6 got the new Python CLI test working on both Unix and MacOS, but the Windows CI test fails, with command |
On Linux: sys.stderr.write(repr(os.environ))
[..., 'PYTHONPATH': '/home/unimelb.edu.au/robertes/src/mrtrix3/build_cmake/lib/', ...]
sys.stder.write(repr(sys.path))
[..., '/home/unimelb.edu.au/robertes/src/mrtrix3/build_cmake/lib', ...] On MSYS2: sys.stderr.write(repr(os.environ))
[..., 'PYTHONPATH': 'C:/msys64/home/rob/src/mrtrix3_cmake/build_cmake/lib', ...]
sys.stder.write(repr(sys.path))
[..., '/home/rob/src/mrtrix3_cmake/testing/data/C', '/msys64/home/unimelb.edu.au/robertes/src/mrtrix3/build_cmake/lib', ...] Setting |
clang-tidy review says "All clean, LGTM! 👍" |
Looks like my attempted hack fix didn't work; @daljit46 this might need your expertise. |
Differences between path handling between Windows and Unix systems is always a pain. Fortunately, MSYS2 ships with a tool called For this specific case, I think it's best to set the environment conditionally based on the OS. We could add some logic to do this automatically in So I propose the following: set(PYTHON_PATH "${PROJECT_BINARY_DIR}/lib")
# On MSYS2 we need to convert Windows paths to Unix paths
if(MINGW AND WIN32)
EXECUTE_PROCESS(
COMMAND cygpath -u ${PYTHON_PATH }
OUTPUT_VARIABLE PYTHON_PATH
OUTPUT_STRIP_TRAILING_WHITESPACE
)
endif()
# Then set environment
add_bash_tests(
...
ENVIRONMENT "PYTHONPATH=${PYTHON_PATH}"
) This should do the job (seems to work on my local MSYS2 installation) |
Co-authored-by: Daljit Singh <daljit7991@gmail.com>
a3048fd
to
08bc526
Compare
…hon_cli_ci_test Conflicts: testing/unit_tests/python_cli
clang-tidy review says "All clean, LGTM! 👍" |
Testing whether the proposed change in #2845 facilitates executing the Python CLI unit tests in CI for #2678.
I do realise in retrospect that this solution might not actually live that long (even if it's a useful capability to leave in place): with #2741, the Python CLI test would need to have the same mechanism of executable binary generation.