Skip to content

Commit c3a9498

Browse files
authored
Removed the use of bash subshells to execute the test commands per project in build-script-impl. The original reasoning seems to have been to disable Ninja's output buffering which was causing the lit progress bar to behave oddly, however local (parallel) testing does not show any peculiarities now. (#20025)
1 parent ca53d0b commit c3a9498

File tree

4 files changed

+30
-45
lines changed

4 files changed

+30
-45
lines changed

test/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ foreach(SDK ${SWIFT_SDKS})
330330
${LIT_ARGS}
331331
"--param" "swift_test_subset=${test_subset}"
332332
"--param" "swift_test_mode=${test_mode}"
333+
${SWIFT_LIT_TEST_PATHS}
333334
DEPENDS ${dependencies}
334335
COMMENT "Running ${test_subset} Swift tests for ${VARIANT_TRIPLE} from custom test locations"
335336
USES_TERMINAL)

utils/build-script

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -290,16 +290,6 @@ class BuildScriptInvocation(object):
290290
args.build_subdir = \
291291
workspace.compute_build_subdir(args)
292292

293-
# --test-paths implies --test and/or --validation-test
294-
# depending on what directories/files have been specified.
295-
if args.test_paths:
296-
for path in args.test_paths:
297-
if path.startswith('test'):
298-
args.test = True
299-
elif path.startswith('validation-test'):
300-
args.test = True
301-
args.validation_test = True
302-
303293
# Add optional stdlib-deployment-targets
304294
if args.android:
305295
args.stdlib_deployment_targets.append(

utils/build-script-impl

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1954,6 +1954,21 @@ for host in "${ALL_HOSTS[@]}"; do
19541954
-DSWIFT_RUNTIME_ENABLE_LEAK_CHECKER:BOOL=$(true_false "${SWIFT_RUNTIME_ENABLE_LEAK_CHECKER}")
19551955
)
19561956

1957+
if [[ "${TEST_PATHS}" ]]; then
1958+
build_dir=$(build_directory ${host} "swift")
1959+
1960+
test_paths=()
1961+
for path in ${TEST_PATHS}; do
1962+
test_paths+=(
1963+
"${build_dir}/$(echo ${path} | sed -E "s/^(validation-test|test)/\1-${host}/")"
1964+
)
1965+
done
1966+
1967+
swift_cmake_options=(
1968+
"${swift_cmake_options[@]}"
1969+
-DSWIFT_LIT_TEST_PATHS="${test_paths}")
1970+
fi
1971+
19571972
if [[ "${SKIP_TEST_SOURCEKIT}" ]] ; then
19581973
swift_cmake_options=(
19591974
"${swift_cmake_options[@]}"
@@ -3174,43 +3189,12 @@ for host in "${ALL_HOSTS[@]}"; do
31743189
trap "tests_busted ${product} '(${target})'" ERR
31753190

31763191
test_target="$target"
3177-
test_paths=()
3178-
3179-
if [[ ${test_target} == check-swift* ]]; then
3180-
if [[ "${TEST_PATHS}" ]]; then
3181-
test_target="${test_target}-custom"
3182-
for path in ${TEST_PATHS}; do
3183-
test_paths+=(
3184-
"${build_dir}/$(echo ${path} | sed -E "s/^(validation-test|test)/\1-${host}/")"
3185-
)
3186-
done
3187-
fi
3192+
if [[ ${test_target} == check-swift* ]] && [[ "${TEST_PATHS}" ]]; then
3193+
test_target="${test_target}-custom"
31883194
fi
31893195

3190-
# NOTE: In dry-run mode, build_dir might not exist yet. In that
3191-
# case, -n query will fail. So, in dry-run mode,
3192-
# we don't expand test script.
3193-
if [[ ! "${DRY_RUN}" && "${CMAKE_GENERATOR}" == Ninja ]] && !( "${build_cmd[@]}" --version 2>&1 | grep -i -q llbuild ) && [[ -z "${DISTCC_PUMP}" ]]; then
3194-
# Ninja buffers command output to avoid scrambling the output
3195-
# of parallel jobs, which is awesome... except that it
3196-
# interferes with the progress meter when testing. Instead of
3197-
# executing ninja directly, have it dump the commands it would
3198-
# run, strip Ninja's progress prefix with sed, and tell the
3199-
# shell to execute that.
3200-
# However, if we do this in a subshell in the ``sh -e -x -c`` line,
3201-
# errors in the command will not stop the script as they should.
3202-
echo "Generating dry run test command from: ${build_cmd[@]} -n -v ${test_target}"
3203-
dry_run_command_output="$(${build_cmd[@]} -n -v ${test_target} | sed -e 's/[^]]*] //')"
3204-
echo "Test command: ${dry_run_command_output}"
3205-
3206-
if [[ ! "${test_paths}" ]]; then
3207-
env bash -ex <(echo -e "${dry_run_command_output}")
3208-
else
3209-
env bash -ex <(echo -e "${dry_run_command_output}" "${test_paths[@]}")
3210-
fi
3211-
else
3212-
call "${build_cmd[@]}" ${BUILD_TARGET_FLAG} ${test_target}
3213-
fi
3196+
call "${build_cmd[@]}" ${BUILD_TARGET_FLAG} ${test_target}
3197+
32143198
echo "-- ${target} finished --"
32153199
fi
32163200
done

utils/build_swift/driver_arguments.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,16 @@ def _apply_default_arguments(args):
166166
if not args.android or not args.build_android:
167167
args.build_android = False
168168

169+
# --test-paths implies --test and/or --validation-test
170+
# depending on what directories/files have been specified.
171+
if args.test_paths:
172+
for path in args.test_paths:
173+
if path.startswith('test'):
174+
args.test = True
175+
elif path.startswith('validation-test'):
176+
args.test = True
177+
args.validation_test = True
178+
169179
# --validation-test implies --test.
170180
if args.validation_test:
171181
args.test = True

0 commit comments

Comments
 (0)