PROTON-2135 Set TEST_ENV more consistently #207
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
astitcher
reviewed
Nov 14, 2019
There was a problem hiding this comment.
This seems evidently correct. However it slides the cmake test code even further from easy comprehension by making it even more wordy than it was before!
Given that there is so much repeated common stuff n all these tests now I'd really like to see some abstraction of this commonality - say using a cmake macro to make each individual test easier to understand.
|
Making large changes to CMake scares me, but well, I can try to make it into a macro. The difficulty is the usual one, the test invocations are all the same, but often a little bit different. |
|
Well,l to some extent this already is a large change! |
|
And I just wanted to run leak check with proton-python! :( OK, so the test registration instances I need to unify look like this |
jirkadanek
force-pushed
the
jd_2019-11_c_cpp_test_env
branch
from
9619271
to
115f690
Compare
|
I can quickly test this by building all bindings, and then do |
|
What I have currently in mind is something like this. Hopefully it will work well for other tests besides ruby. +include(CMakeParseArguments)
+
+function(pn_add_test)
+ set(options EXECUTABLE INTERPRETED UNWRAPPED IGNORE_ENVIRONMENT)
+ set(oneValueArgs NAME COMMAND)
+ set(multiValueArgs PREPEND_ENVIRONMENT APPEND_ENVIRONMENT)
+ cmake_parse_arguments(pn_add_test "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
+
+ if (pn_add_test_IGNORE_ENVIRONMENT)
+ set (ignore_environment "--ignore_environment")
+ else(pn_add_test_IGNORE_ENVIRONMENT)
+ set (ignore_environment "")
+ endif(pn_add_test_IGNORE_ENVIRONMENT)
+
+ if (pn_add_test_UNWRAPPED)
+ set (wrapper "")
+ elseif(pn_add_test_INTERPRETED)
+ set (wrapper "${TEST_WRAP_PREFIX_CMD}")
+ elseif(pn_add_test_EXECUTABLE)
+ set (wrapper "${TEST_EXE_PREFIX_CMD}")
+ else()
+ message(FATAL_ERROR "pn_add_test requires one of EXECUTABLE INTERPRETED UNWRAPPED")
+ endif()
+
+ add_test (
+ NAME ${pn_add_test_NAME}
+ COMMAND ${PN_ENV_SCRIPT} ${ignore_environment} -- ${pn_add_test_PREPEND_ENVIRONMENT} ${TEST_ENV} ${pn_add_test_APPEND_ENVIRONMENT} ${wrapper} ${pn_add_test_COMMAND}
+ ${pn_add_test_UNPARSED_ARGUMENTS}
+ )
+endfunction(pn_add_test)
+
execute_process(COMMAND ${RUBY_EXECUTABLE} -r minitest -e ""
RESULT_VARIABLE result OUTPUT_QUIET ERROR_QUIET)
if (result EQUAL 0) # Have minitest
- set(test_env
+ set(ruby_test_env
"PATH=${PATH}"
"RUBYLIB=${RUBYLIB}"
- "SASLPASSWD=${CyrusSASL_Saslpasswd_EXECUTABLE}"
- ${TEST_ENV})
+ "SASLPASSWD=${CyrusSASL_Saslpasswd_EXECUTABLE}")
macro(add_ruby_test script)
get_filename_component(name ${script} NAME_WE)
string(REPLACE "_" "-" name "ruby-${name}")
- add_test(
+ pn_add_test(
+ INTERPRETED
NAME ${name}
- COMMAND ${PN_ENV_SCRIPT} -- ${test_env} ${TEST_WRAP_PREFIX_CMD} ${RUBY_EXECUTABLE} ${script} -v
+ PREPEND_ENVIRONMENT ${ruby_test_env}
+ COMMAND ${RUBY_EXECUTABLE} ${script} -v
${ARGN})
endmacro()
- add_test(
+ pn_add_test(
+ UNWRAPPED
NAME ruby-example-test
- COMMAND ${PN_ENV_SCRIPT} -- ${test_env} ${RUBY_EXECUTABLE} testme -v
+ PREPEND_ENVIRONMENT ${ruby_test_env}
+ COMMAND ${RUBY_EXECUTABLE} testme -v
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/examples) |
|
First version of the PR
Current version with the helper function
I'll do the log diffing later and edit the message with the result. edit: New current version that does not introduce a diff in go test commands and better handles Windows PATH |
jirkadanek
force-pushed
the
jd_2019-11_c_cpp_test_env
branch
from
e35322e
to
c6a9a40
Compare
|
Test command has changed in some Python tests, but I think there is nothing wrong with the new state of things. Unix26: Windows6: 26: 27: |
astitcher
approved these changes
Jan 22, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.