Stop using ctest by CaseyCarter · Pull Request #5169 · microsoft/STL (original) (raw)

It's pointless to use ctest to run one of two different LIT commands when we can simply run LIT directly. This will make it easier to add future tests: we can simply add more targets, we won't need to jump through hoops to tell ctest not to run the new test all the time. This change will ease adding a new EDG-only test target for the IntelliSense team.

Detailed changes:

• In tests/CMakeLists.txt:
  • Replace add_test targets with add_custom_targets. The stl and stlasan tests are now test and extra-asan-tests targets, respectively.
  • Replace list(APPEND unset_variable_name a b c d) with the equivalent set(unset_variable_name a b c d) which makes it clear that we expect unset_variable_name to be undefined.
  • Merge ${Python_EXECUTABLE} into STL_LIT_COMMAND, and extract ${STL_LIT_TEST_DIRS}. The makes it easy to reuse ${STL_LIT_COMMAND} and ${STL_LIT_TEST_DIRS} with target-specific LIT options.
  • Note that USES_TERMINAL in the add_custom_targets makes ninja run those targets serially, maintaining the observable effects of the removed set_tests_properties(stl stlasan PROPERTIES RUN_SERIAL ON).
• Remove enable_testing from the top-level CMakeLists.txt; it does nothing now that we have no add_tests. We still use BUILD_TESTING to control including the test targets in the generated build system, but I'm not certain there's a good reason to do so. Maybe to enable folks without python to build but not test the STL?
• In run-tests.yml, replace ctestOptions parameter with a testTargets parameter, a list of targets to build via cmake --build meow --target woof quack. This needs to be set in asan-pipeline.yml and plumbed through build-and-test.yml with a default of test to run the vanilla STL tests.
• Remove/Replace the ctest discussion in README.md.
• Drive-by: Report that the psutil python module is unavailable at most once per stl-lit run. Seeing three repetitions in the CI logs bothered me.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

It's pointless to use ctest to run one of two different LIT commands when we can simply run LIT directly. This will make it easier to add future tests: we can simply add more targets, we won't need to jump through hoops to tell ctest not to run the new test all the time. This change will ease adding a new EDG-only test target for the IntelliSense team.

Detailed changes:

• In tests/CMakeLists.txt:
  • Replace add_test targets with add_custom_targets. The stl and stlasan tests are now test and extra-asan-tests targets, respectively.
  • Replace list(APPEND unset_variable_name a b c d) with the equivalent set(unset_variable_name a b c d) which makes it clear that we expect unset_variable_name to be undefined.
  • Merge ${Python_EXECUTABLE} into STL_LIT_COMMAND, and extract ${STL_LIT_TEST_DIRS}. The makes it easy to reuse ${STL_LIT_COMMAND} and ${STL_LIT_TEST_DIRS} with target-specific LIT options.
  • Note that USES_TERMINAL in the add_custom_targets makes ninja run those targets serially, maintaining the observable effects of the removed set_tests_properties(stl stlasan PROPERTIES RUN_SERIAL ON).
• Remove enable_testing from the top-level CMakeLists.txt; it does nothing now that we have no add_tests. We still use BUILD_TESTING to control including the test targets in the generated build system, but I'm not certain there's a good reason to do so. Maybe to enable folks without python to build but not test the STL?
• In run-tests.yml, replace ctestOptions parameter with a testTargets parameter, a list of targets to build via cmake --build meow --target woof quack. This needs to be set in asan-pipeline.yml and plumbed through build-and-test.yml with a default of test to run the vanilla STL tests.
• Remove/Replace the ctest discussion in README.md.
• Drive-by: Report that the psutil python module is unavailable at most once per stl-lit run. Seeing three repetitions in the CI logs bothered me.

This comment was marked as outdated.

This comment was marked as resolved.

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

• /azp run STL-ASan-CI (There's a later push in the PR, but it's a non-functional change to documentation so I didn't revalidate.)

This comment was marked as resolved.

README.md Outdated Show resolved Hide resolved

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

🛑 😻 💚