Skip to content

[Build System] Remove the use of sub-shells when running test targets #20025

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

Merged
merged 1 commit into from
Oct 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ foreach(SDK ${SWIFT_SDKS})
${LIT_ARGS}
"--param" "swift_test_subset=${test_subset}"
"--param" "swift_test_mode=${test_mode}"
${SWIFT_LIT_TEST_PATHS}
DEPENDS ${dependencies}
COMMENT "Running ${test_subset} Swift tests for ${VARIANT_TRIPLE} from custom test locations"
USES_TERMINAL)
Expand Down
10 changes: 0 additions & 10 deletions utils/build-script
Original file line number Diff line number Diff line change
Expand Up @@ -290,16 +290,6 @@ class BuildScriptInvocation(object):
args.build_subdir = \
workspace.compute_build_subdir(args)

# --test-paths implies --test and/or --validation-test
# depending on what directories/files have been specified.
if args.test_paths:
for path in args.test_paths:
if path.startswith('test'):
args.test = True
elif path.startswith('validation-test'):
args.test = True
args.validation_test = True

# Add optional stdlib-deployment-targets
if args.android:
args.stdlib_deployment_targets.append(
Expand Down
54 changes: 19 additions & 35 deletions utils/build-script-impl
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,21 @@ for host in "${ALL_HOSTS[@]}"; do
-DSWIFT_RUNTIME_ENABLE_LEAK_CHECKER:BOOL=$(true_false "${SWIFT_RUNTIME_ENABLE_LEAK_CHECKER}")
)

if [[ "${TEST_PATHS}" ]]; then
build_dir=$(build_directory ${host} "swift")

test_paths=()
for path in ${TEST_PATHS}; do
test_paths+=(
"${build_dir}/$(echo ${path} | sed -E "s/^(validation-test|test)/\1-${host}/")"
)
done

swift_cmake_options=(
"${swift_cmake_options[@]}"
-DSWIFT_LIT_TEST_PATHS="${test_paths}")
fi

if [[ "${SKIP_TEST_SOURCEKIT}" ]] ; then
swift_cmake_options=(
"${swift_cmake_options[@]}"
Expand Down Expand Up @@ -3174,43 +3189,12 @@ for host in "${ALL_HOSTS[@]}"; do
trap "tests_busted ${product} '(${target})'" ERR

test_target="$target"
test_paths=()

if [[ ${test_target} == check-swift* ]]; then
if [[ "${TEST_PATHS}" ]]; then
test_target="${test_target}-custom"
for path in ${TEST_PATHS}; do
test_paths+=(
"${build_dir}/$(echo ${path} | sed -E "s/^(validation-test|test)/\1-${host}/")"
)
done
fi
if [[ ${test_target} == check-swift* ]] && [[ "${TEST_PATHS}" ]]; then
test_target="${test_target}-custom"
fi

# NOTE: In dry-run mode, build_dir might not exist yet. In that
# case, -n query will fail. So, in dry-run mode,
# we don't expand test script.
if [[ ! "${DRY_RUN}" && "${CMAKE_GENERATOR}" == Ninja ]] && !( "${build_cmd[@]}" --version 2>&1 | grep -i -q llbuild ) && [[ -z "${DISTCC_PUMP}" ]]; then
# Ninja buffers command output to avoid scrambling the output
# of parallel jobs, which is awesome... except that it
# interferes with the progress meter when testing. Instead of
# executing ninja directly, have it dump the commands it would
# run, strip Ninja's progress prefix with sed, and tell the
# shell to execute that.
# However, if we do this in a subshell in the ``sh -e -x -c`` line,
# errors in the command will not stop the script as they should.
echo "Generating dry run test command from: ${build_cmd[@]} -n -v ${test_target}"
dry_run_command_output="$(${build_cmd[@]} -n -v ${test_target} | sed -e 's/[^]]*] //')"
echo "Test command: ${dry_run_command_output}"

if [[ ! "${test_paths}" ]]; then
env bash -ex <(echo -e "${dry_run_command_output}")
else
env bash -ex <(echo -e "${dry_run_command_output}" "${test_paths[@]}")
fi
else
call "${build_cmd[@]}" ${BUILD_TARGET_FLAG} ${test_target}
fi
call "${build_cmd[@]}" ${BUILD_TARGET_FLAG} ${test_target}

echo "-- ${target} finished --"
fi
done
Expand Down
10 changes: 10 additions & 0 deletions utils/build_swift/driver_arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,16 @@ def _apply_default_arguments(args):
if not args.android or not args.build_android:
args.build_android = False

# --test-paths implies --test and/or --validation-test
# depending on what directories/files have been specified.
if args.test_paths:
for path in args.test_paths:
if path.startswith('test'):
args.test = True
elif path.startswith('validation-test'):
args.test = True
args.validation_test = True

# --validation-test implies --test.
if args.validation_test:
args.test = True
Expand Down