-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Ability to specify a test or a test group when building with docker release #4030
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
Ability to specify a test or a test group when building with docker release #4030
Conversation
… or test dir (e.g. //test/common/...) when building with docker release Signed-off-by: trabetti <talis@il.ibm.com>
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.
Make sense to me. Thank you! Small nit.
ci/do_ci.sh
Outdated
# Run only specified tests. | ||
bazel --batch test ${BAZEL_TEST_OPTIONS} -c opt $2 | ||
else | ||
echo "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.
nit: indent got a bit messed up here.
Signed-off-by: trabetti <talis@il.ibm.com>
Signed-off-by: trabetti <talis@il.ibm.com>
ci/do_ci.sh
Outdated
bazel --batch test ${BAZEL_TEST_OPTIONS} -c opt //test/... | ||
|
||
if [[ $# > 2 ]]; then | ||
echo "Multiple tests are not supported, only $2 will run" |
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.
nit: inconsistent indent. Also, couldn't we run more than 2 if we do some shifting of args and then wildcard arguments?
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.
Right, it is quite simple, did not think of it. Do you think this option should be added to other targets beside release?
Signed-off-by: trabetti <talis@il.ibm.com>
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.
Thanks, this is great. I would say let's just merge this and we can add to other targets later if people want them? (I can see dbg being useful also).
added ability to specify a test (e.g. //test/common/common:assert_test) or a test group (e.g. //test/common/...) when building with docker release
Signed-off-by: trabetti talis@il.ibm.com
Description: When compiling with Docker for development, I found it useful to compile and run only one test which I am currently working on, so I updated locally the do_ci.sh script to accommodate this option.
I thought I can suggest it as a PR, maybe it can be useful for someone else. If seems unnecessary, feel free to close...
example usage:
ENVOY_DOCKER_BUILD_DIR=~/envoy_fork/build-debug ./ci/run_envoy_docker.sh './ci/do_ci.sh bazel.release //test/common/stats:thread_local_store_test'
Risk Level: Low
Testing:
Docs Changes: ci/README.md
Release Notes: