From b00f56909027667fe99fc6e713175f91f401c128 Mon Sep 17 00:00:00 2001 From: jonawals Date: Sun, 6 Jun 2021 19:09:24 +0100 Subject: [PATCH] Address PR comments --- .../Code/Source/TestImpactConsoleMain.cpp | 2 +- cmake/LYTestWrappers.cmake | 32 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp index fab6509244..4853c6da15 100644 --- a/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp +++ b/Code/Tools/TestImpactFramework/Frontend/Console/Static/Code/Source/TestImpactConsoleMain.cpp @@ -121,7 +121,7 @@ namespace TestImpact } else { - // One sequence timed out whilst the other sequence succeeded or both sequences timed out + // One or both sequences timed out or failed result = TestSequenceResult::Timeout; } } diff --git a/cmake/LYTestWrappers.cmake b/cmake/LYTestWrappers.cmake index 3308c4411a..eebc281d06 100644 --- a/cmake/LYTestWrappers.cmake +++ b/cmake/LYTestWrappers.cmake @@ -71,6 +71,7 @@ endfunction() #! ly_add_test: Adds a new RUN_TEST using for the specified target using the supplied command # # \arg:NAME - Name to for the test run target +# \arg:PARENT_NAME(optional) - Name of the parent test run target (if this is a subsequent call to specify a suite) # \arg:TEST_REQUIRES(optional) - List of system resources that are required to run this test. # Only available option is "gpu" # \arg:TEST_SUITE(optional) - "smoke" or "periodic" or "sandbox" - prevents the test from running normally @@ -94,7 +95,7 @@ endfunction() # sets LY_ADDED_TEST_NAME to the fully qualified name of the test, in parent scope function(ly_add_test) set(options EXCLUDE_TEST_RUN_TARGET_FROM_IDE) - set(one_value_args NAME TEST_LIBRARY TEST_SUITE TIMEOUT) + set(one_value_args NAME PARENT_NAME TEST_LIBRARY TEST_SUITE TIMEOUT) set(multi_value_args TEST_REQUIRES TEST_COMMAND NON_IDE_PARAMS RUNTIME_DEPENDENCIES COMPONENT LABELS) # note that we dont use TEST_LIBRARY here, but PAL files might so do not remove! @@ -241,18 +242,24 @@ function(ly_add_test) endif() + if(NOT ly_add_test_PARENT_NAME) + set(test_target ${ly_add_test_NAME}) + else() + set(test_target ${ly_add_test_PARENT_NAME}) + endif() + # Check to see whether or not this test target has been stored in the global list for walking by the test impact analysis framework get_property(all_tests GLOBAL PROPERTY LY_ALL_TESTS) - if(NOT "${LY_ALL_TESTS_TARGET_NAME}" IN_LIST all_tests) + if(NOT "${test_target}" IN_LIST all_tests) # This is the first reference to this test target so add it to the global list - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS ${LY_ALL_TESTS_TARGET_NAME}) - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${LY_ALL_TESTS_TARGET_NAME}_TEST_LIBRARY ${ly_add_test_TEST_LIBRARY}) + set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS ${test_target}) + set_property(GLOBAL PROPERTY LY_ALL_TESTS_${test_target}_TEST_LIBRARY ${ly_add_test_TEST_LIBRARY}) endif() # Add the test suite and timeout value to the test target params set(LY_TEST_PARAMS "${LY_TEST_PARAMS}#${ly_add_test_TEST_SUITE}") set(LY_TEST_PARAMS "${LY_TEST_PARAMS}#${ly_add_test_TIMEOUT}") # Store the params for this test target - set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${LY_ALL_TESTS_TARGET_NAME}_PARAMS ${LY_TEST_PARAMS}) + set_property(GLOBAL APPEND PROPERTY LY_ALL_TESTS_${test_target}_PARAMS ${LY_TEST_PARAMS}) endfunction() #! ly_add_pytest: registers target PyTest-based test with CTest @@ -294,13 +301,12 @@ function(ly_add_pytest) string(REPLACE "::" "_" pytest_report_directory "${PYTEST_XML_OUTPUT_DIR}/${ly_add_pytest_NAME}.xml") - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${ly_add_pytest_NAME}) # Add the script path to the test target params set(LY_TEST_PARAMS "${ly_add_pytest_PATH}") ly_add_test( NAME ${ly_add_pytest_NAME} + PARENT_NAME ${ly_add_pytest_NAME} TEST_SUITE ${ly_add_pytest_TEST_SUITE} LABELS FRAMEWORK_pytest TEST_COMMAND ${LY_PYTEST_EXECUTABLE} ${ly_add_pytest_PATH} ${ly_add_pytest_EXTRA_ARGS} --junitxml=${pytest_report_directory} ${custom_marks_args} @@ -351,14 +357,13 @@ function(ly_add_editor_python_test) file(REAL_PATH ${ly_add_editor_python_test_TEST_PROJECT} project_real_path BASE_DIRECTORY ${LY_ROOT_FOLDER}) - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${ly_add_editor_python_test_NAME}) # Add the script path to the test target params set(LY_TEST_PARAMS "${ly_add_editor_python_test_PATH}") # Run test via the run_epbtest.cmake script. # Parameters used are explained in run_epbtest.cmake. ly_add_test( NAME ${ly_add_editor_python_test_NAME} + PARENT_NAME ${ly_add_editor_python_test_NAME} TEST_REQUIRES ${ly_add_editor_python_test_TEST_REQUIRES} TEST_COMMAND ${CMAKE_COMMAND} -DCMD_ARG_TEST_PROJECT=${project_real_path} @@ -457,12 +462,10 @@ function(ly_add_googletest) string(REPLACE "::" "_" report_directory "${GTEST_XML_OUTPUT_DIR}/${ly_add_googletest_NAME}.xml") - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${target_name}) - # Invoke the lower level ly_add_test command to add the actual ctest and setup the test labels to add_dependencies on the target ly_add_test( NAME ${ly_add_googletest_NAME} + PARENT_NAME ${target_name} TEST_SUITE ${ly_add_googletest_TEST_SUITE} LABELS FRAMEWORK_googletest TEST_COMMAND ${full_test_command} --gtest_output=xml:${report_directory} ${LY_GOOGLETEST_EXTRA_PARAMS} @@ -548,12 +551,11 @@ function(ly_add_googlebenchmark) # Start the test target params and dd the command runner command set(LY_TEST_PARAMS "${stripped_test_command}") endif() - - # Set the name of the current test target for storage in the global list - set(LY_ALL_TESTS_TARGET_NAME ${ly_add_googlebenchmark_NAME}) + # Set the name of the current test target for storage in the global list ly_add_test( NAME ${ly_add_googlebenchmark_NAME} + PARENT_NAME ${ly_add_googlebenchmark_NAME} TEST_REQUIRES ${ly_add_googlebenchmark_TEST_REQUIRES} TEST_COMMAND ${full_test_command} ${LY_GOOGLETEST_EXTRA_PARAMS} TEST_SUITE "benchmark"