-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Build: add dependencies for test_paddle_inference_api_impl. #11064
Conversation
@luotao1 Can you give some advice. I'm not very good at it. There is one TODO for the cmake that I haven't fixed: |
@wanglei828 CI is failing for this PR |
else() | ||
list(APPEND arg_list "_") | ||
set_tests_properties(test_paddle_inference_${TARGET_NAME} | ||
PROPERTIES DEPENDS "${inference_test_ARGS}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paddle/paddle/fluid/inference/tests/book/CMakeLists.txt
Lines 22 to 23 in e00c1ee
set_tests_properties(test_inference_${TARGET_NAME}${arg} | |
PROPERTIES DEPENDS test_${TARGET_NAME}) |
python单测的名字是test_xxx,而inference_test_ARGS的名字是api_impl,没对上导致依赖没生效。
所以仿照paddle/fluid/inference/tests/book/CMakeLists.txt
再来改一下就可以了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@luotao1 I think you may not understand this correctly. In the file, I call the function like this:
inference_api_test(api_impl
ARGS test_word2vec test_image_classification)
after parsing,
the variable "inference_test_ARGS" should be "test_word2vec; test_image_classification" which is correct.
when I use:
set_tests_properties(test_paddle_inference_${TARGET_NAME}
PROPERTIES DEPENDS "${inference_test_ARGS}")
it will be interpreted as this:
set_tests_properties(test_paddle_inference_api_impl PROPERTIES DEPENDS "test_word2vec; test_image_classification" )
You can read this reference: https://cmake.org/cmake/help/v3.0/module/CMakeParseArguments.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
… test_paddle_inference_api_impl.
@panyx0718 I fixed the CI fail. It is mainly because the test cases are not put in the "WITH_TESTING" control flow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
cc_test(test_paddle_inference_api | ||
SRCS test_paddle_inference_api.cc | ||
DEPS paddle_inference_api) | ||
if(WITH_TESTING) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc_test里面会判断if(WITH_TESTING)
,这里不需要加了。
else() | ||
list(APPEND arg_list "_") | ||
set_tests_properties(test_paddle_inference_${TARGET_NAME} | ||
PROPERTIES DEPENDS "${inference_test_ARGS}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
Thanks! |
No description provided.