From 305ce6c19ece8fdf960e9f4a40084bb7162c4959 Mon Sep 17 00:00:00 2001 From: William Allen <16820599+williamjallen@users.noreply.github.com> Date: Tue, 4 Feb 2025 16:28:34 -0500 Subject: [PATCH] Reorganize CMake/CTest scripts (#2698) Following the work in https://github.com/Kitware/CDash/pull/2664, this PR removes some of the bottlenecks slowing down the test suite on developer machines by running long tests concurrently where possible. This PR also adds the ability to run Cypress tests concurrently and reorganizes some of the messy test generation helper functions. While it would be nice to run all of the Cypress tests concurrently, Cypress is resource intensive and experiments only showed a marginal improvement in running times. On my developer machine, these changes reduced the running time for the full test suite from 9 minutes to 8.5 minutes. Although the CI runners are substantially less powerful than my machine, I expect at least a small reduction in CI job times. --- CMakeLists.txt | 12 --- app/cdash/tests/CMakeLists.txt | 26 +++---- app/cdash/tests/htaccess.in | 2 - app/cdash/tests/test_autoremovebuilds.php | 55 -------------- phpstan-baseline.neon | 27 +------ tests/CMakeLists.txt | 74 +------------------ tests/Feature/SlowPageTest.php | 7 +- tests/Spec/CMakeLists.txt | 14 ++++ .../{page-header => }/header-menu.spec.js | 2 +- tests/cypress/component/CMakeLists.txt | 21 ++++++ tests/cypress/component/data-table.cy.js | 2 +- tests/cypress/e2e/CMakeLists.txt | 46 +++++++++++- 12 files changed, 100 insertions(+), 188 deletions(-) delete mode 100644 app/cdash/tests/htaccess.in delete mode 100644 app/cdash/tests/test_autoremovebuilds.php create mode 100644 tests/Spec/CMakeLists.txt rename tests/Spec/{page-header => }/header-menu.spec.js (97%) create mode 100644 tests/cypress/component/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 129d2cdcaf..77efe0d9ab 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,18 +41,6 @@ configure_file( # Any tests which need to perform file I/O write files here file(MAKE_DIRECTORY "/tmp/CDashTesting") -# For coverage builds on apache web servers, provide a default -# .htaccess file, but only if explicitly asked to: -option(CDASH_CONFIGURE_HTACCESS_FILE "Create .htaccess file for coverage testing?" OFF) -if(CDASH_CONFIGURE_HTACCESS_FILE) - # Yes, the output of this configure_file is intentionally in the - # source directory - configure_file( - ${testing_dir}/htaccess.in - ${CDash_SOURCE_DIR}/app/cdash/public/.htaccess - ) -endif() - find_program(PHP_EXE NAMES php PATHS c:/xampp/php /xampp/php REQUIRED) find_program(NPX_EXE NAMES npx PATHS c:/usr/bin/npx /usr/bin/npx REQUIRED) diff --git a/app/cdash/tests/CMakeLists.txt b/app/cdash/tests/CMakeLists.txt index 19950c2fe6..9d4152d0de 100644 --- a/app/cdash/tests/CMakeLists.txt +++ b/app/cdash/tests/CMakeLists.txt @@ -65,11 +65,8 @@ add_laravel_test(/Unit/app/Console/Command/ValidateXmlCommandTest) ################################################################################################### -cdash_install() -set_tests_properties(install_2 PROPERTIES DEPENDS cypress/e2e/user-profile) - add_laravel_test(/Feature/CDashTest) -set_tests_properties(/Feature/CDashTest PROPERTIES DEPENDS install_2) +set_tests_properties(/Feature/CDashTest PROPERTIES DEPENDS install_1) add_laravel_test(/Feature/LoginAndRegistration) set_tests_properties(/Feature/LoginAndRegistration PROPERTIES DEPENDS /Feature/CDashTest) @@ -77,11 +74,8 @@ set_tests_properties(/Feature/LoginAndRegistration PROPERTIES DEPENDS /Feature/C add_laravel_test(/Feature/ProjectPermissions) set_tests_properties(/Feature/ProjectPermissions PROPERTIES DEPENDS /Feature/LoginAndRegistration) -add_laravel_test(/Feature/SlowPageTest) -set_tests_properties(/Feature/SlowPageTest PROPERTIES DEPENDS /Feature/ProjectPermissions) - add_laravel_test(/Feature/GitHubWebhook) -set_tests_properties(/Feature/GitHubWebhook PROPERTIES DEPENDS /Feature/SlowPageTest) +set_tests_properties(/Feature/GitHubWebhook PROPERTIES DEPENDS /Feature/ProjectPermissions) add_unit_test(/CDash/BuildUseCase) set_tests_properties(/CDash/BuildUseCase PROPERTIES DEPENDS /Feature/GitHubWebhook) @@ -275,10 +269,14 @@ set_tests_properties(/Feature/Mail/AuthTokenExpiringTest PROPERTIES DEPENDS /CDa add_laravel_test(/Feature/Mail/AuthTokenExpiredTest) set_tests_properties(/Feature/Mail/AuthTokenExpiredTest PROPERTIES DEPENDS /CDash/XmlHandler/UpdateHandler) +add_laravel_test(/Feature/SlowPageTest) +set_tests_properties(/Feature/SlowPageTest PROPERTIES DEPENDS /CDash/XmlHandler/UpdateHandler) + ################################################################################################### cdash_install() -set_property(TEST install_3 PROPERTY DEPENDS +set_property(TEST install_2 PROPERTY DEPENDS + cypress/e2e/user-profile /Feature/SubmissionValidation /Feature/GraphQL/FilterTest /Feature/GraphQL/ProjectTypeTest @@ -313,11 +311,12 @@ set_property(TEST install_3 PROPERTY DEPENDS /Feature/Jobs/NotifyExpiringAuthTokensTest /Feature/Mail/AuthTokenExpiringTest /Feature/Mail/AuthTokenExpiredTest + /Feature/SlowPageTest ) add_php_test(compressedtest) -set_tests_properties(compressedtest PROPERTIES DEPENDS install_3) +set_tests_properties(compressedtest PROPERTIES DEPENDS install_2) add_php_test(createpublicdashboard) set_tests_properties(createpublicdashboard PROPERTIES DEPENDS compressedtest) @@ -364,11 +363,8 @@ set_tests_properties(image PROPERTIES DEPENDS committerinfo) add_php_test(displayimage) set_tests_properties(displayimage PROPERTIES DEPENDS image) -add_cypress_e2e_test(banner) -set_tests_properties(cypress/e2e/banner PROPERTIES DEPENDS displayimage) - add_php_test(manageprojectroles) -set_tests_properties(manageprojectroles PROPERTIES DEPENDS cypress/e2e/banner) +set_tests_properties(manageprojectroles PROPERTIES DEPENDS displayimage) add_php_test(manageusers) set_tests_properties(manageusers PROPERTIES DEPENDS manageprojectroles) @@ -458,7 +454,7 @@ add_php_test(excludesubprojects) set_tests_properties(excludesubprojects PROPERTIES DEPENDS subprojectnextprevious) add_php_test(testhistory) -set_tests_properties(testhistory PROPERTIES DEPENDS excludesubprojects) +set_tests_properties(testhistory PROPERTIES DEPENDS subprojectnextprevious) add_php_test(expectedandmissing) set_tests_properties(expectedandmissing PROPERTIES DEPENDS testhistory) diff --git a/app/cdash/tests/htaccess.in b/app/cdash/tests/htaccess.in deleted file mode 100644 index ccfe26fa54..0000000000 --- a/app/cdash/tests/htaccess.in +++ /dev/null @@ -1,2 +0,0 @@ -php_value auto_prepend_file "@CDash_BINARY_DIR@/prepend_coverage.php" -php_value opcache.enable 0 diff --git a/app/cdash/tests/test_autoremovebuilds.php b/app/cdash/tests/test_autoremovebuilds.php deleted file mode 100644 index 365832cdc0..0000000000 --- a/app/cdash/tests/test_autoremovebuilds.php +++ /dev/null @@ -1,55 +0,0 @@ -launchViaCommandLine(''); - $output = ob_get_contents(); - ob_end_clean(); - - if (!str_contains($output, 'Usage: php')) { - $this->fail("Expected output not found from autoRemoveBuilds.php.\n$output\n"); - } - - chdir($dir); - $argv[0] = 'autoRemoveBuilds.php'; - $argv[1] = 'InsightExample'; - $argc = 2; - - ob_start(); - $this->launchViaCommandLine('InsightExample'); - $output = ob_get_contents(); - ob_end_clean(); - - if (!str_contains($output, 'removing builds for InsightExample')) { - $this->fail("Expected output not found from autoRemoveBuilds.php.\n$output\n"); - $error = 1; - } elseif (!str_contains($output, 'removing old buildids')) { - $this->fail("Autoremovebuilds failed to remove old build by buildgroup setting.\n$output\n"); - $error = 1; - } else { - $this->pass('Passed'); - $error = 0; - } - - return $error; - } -} diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a8799cfd1e..3b5381ddaa 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -20458,31 +20458,6 @@ parameters: count: 1 path: app/cdash/tests/test_authtoken.php - - - message: "#^Call to an undefined method AutoRemoveBuildsTestCase\\:\\:launchViaCommandLine\\(\\)\\.$#" - count: 2 - path: app/cdash/tests/test_autoremovebuilds.php - - - - message: "#^Call to deprecated method pass\\(\\) of class SimpleTestCase\\.$#" - count: 1 - path: app/cdash/tests/test_autoremovebuilds.php - - - - message: "#^Implicit array creation is not allowed \\- variable \\$argv does not exist\\.$#" - count: 1 - path: app/cdash/tests/test_autoremovebuilds.php - - - - message: "#^Method AutoRemoveBuildsTestCase\\:\\:testAutoRemoveBuilds\\(\\) has no return type specified\\.$#" - count: 1 - path: app/cdash/tests/test_autoremovebuilds.php - - - - message: "#^Parameter \\#1 \\$haystack of function str_contains expects string, string\\|false given\\.$#" - count: 3 - path: app/cdash/tests/test_autoremovebuilds.php - - message: """ #^Call to deprecated function pdo_fetch_array\\(\\)\\: @@ -32056,7 +32031,7 @@ parameters: - message: "#^Dynamic call to static method Illuminate\\\\Http\\\\Response\\:\\:content\\(\\)\\.$#" - count: 1 + count: 2 path: tests/Feature/SlowPageTest.php - diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d2e260d870..ffa5d3444e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,63 +1,3 @@ -function(add_vue_test TestName) - add_test( - NAME ${TestName} - COMMAND "node_modules/.bin/jest" "tests/${TestName}.spec.js" - WORKING_DIRECTORY "${CDash_SOURCE_DIR}" - ) -endfunction() - -function(set_app_url) - if(DEFINED ENV{APP_URL}) - set(APP_URL "$ENV{APP_URL}" PARENT_SCOPE) - elseif(EXISTS ${CDash_SOURCE_DIR}/.env) - file(STRINGS ${CDash_SOURCE_DIR}/.env env_vars) - foreach(var IN LISTS env_vars) - if(var MATCHES "^APP_URL=(.*)$") - set(APP_URL "${CMAKE_MATCH_1}" PARENT_SCOPE) - break() - endif() - endforeach() - else() - set(APP_URL "http://localhost:8080" PARENT_SCOPE) - endif() -endfunction() - -function(add_cypress_e2e_test TestName) - set_app_url() - - add_test( - NAME cypress/e2e/${TestName} - COMMAND ${NPX_EXE} cypress run - --e2e - --project ${CDash_SOURCE_DIR} - --spec ${CDash_SOURCE_DIR}/tests/cypress/e2e/${TestName}.cy.js - --config baseUrl=${APP_URL} - ) - # Cypress tries to put stuff in our home directory, which doesn't work for /var/www. - set_tests_properties(cypress/e2e/${TestName} PROPERTIES - ENVIRONMENT "HOME=${CDash_BINARY_DIR};" - DISABLED "$" - ) -endfunction() - -function(add_cypress_component_test TestName) - set_app_url() - - add_test( - NAME cypress/component/${TestName} - COMMAND ${NPX_EXE} cypress run - --component - --project ${CDash_SOURCE_DIR} - --spec ${CDash_SOURCE_DIR}/tests/cypress/component/${TestName}.cy.js - --config baseUrl=${APP_URL} - ) - # Cypress tries to put stuff in our home directory, which doesn't work for /var/www. - set_tests_properties(cypress/component/${TestName} PROPERTIES - ENVIRONMENT "HOME=${CDash_BINARY_DIR};" - DISABLED "$" - ) -endfunction() - add_test( NAME php_style_check COMMAND ${CMAKE_SOURCE_DIR}/vendor/bin/php-cs-fixer fix --dry-run --allow-risky=yes @@ -94,18 +34,8 @@ set_tests_properties(eslint PROPERTIES FAIL_REGULAR_EXPRESSION " \+[0-9]\+:[0-9]\+ \+warning" ) -add_vue_test(Spec/build-configure) -add_vue_test(Spec/build-summary) -add_vue_test(Spec/edit-project) -add_vue_test(Spec/manage-measurements) -add_vue_test(Spec/page-header/header-menu) -add_vue_test(Spec/test-details) - -add_cypress_component_test(loading-indicator) - cdash_install() -add_cypress_e2e_test(user-profile) -set_tests_properties(cypress/e2e/user-profile PROPERTIES DEPENDS install_1) - add_subdirectory(cypress/e2e) +add_subdirectory(cypress/component) +add_subdirectory(Spec) diff --git a/tests/Feature/SlowPageTest.php b/tests/Feature/SlowPageTest.php index 98ed781eb3..0b8b8d6f15 100644 --- a/tests/Feature/SlowPageTest.php +++ b/tests/Feature/SlowPageTest.php @@ -12,15 +12,16 @@ class SlowPageTest extends TestCase public function testSlowPageLogsWarning(): void { Log::shouldReceive('warning') - ->with(Mockery::pattern('#Slow page\: /\?projectid=10 took \d*\.?\d+ seconds to load#')); + ->with(Mockery::pattern('#Slow page\: /login took \d*\.?\d+ seconds to load#')); Config::set('cdash.slow_page_time', 0); - self::assertNotEmpty($this->get('/?projectid=10')->content()); + self::assertNotEmpty($this->get('/login')->content()); } public function testFastPageDoesntLogWarning(): void { Config::set('cdash.slow_page_time', 100); Log::shouldReceive('warning')->never(); - $this->get('/?projectid=10'); + $this->get('/login'); + self::assertNotEmpty($this->get('/login')->content()); } } diff --git a/tests/Spec/CMakeLists.txt b/tests/Spec/CMakeLists.txt new file mode 100644 index 0000000000..6c0f7887c5 --- /dev/null +++ b/tests/Spec/CMakeLists.txt @@ -0,0 +1,14 @@ +function(add_vue_test TestName) + add_test( + NAME "Spec/${TestName}" + COMMAND "node_modules/.bin/jest" "tests/Spec/${TestName}.spec.js" + WORKING_DIRECTORY "${CDash_SOURCE_DIR}" + ) +endfunction() + +add_vue_test(build-configure) +add_vue_test(build-summary) +add_vue_test(edit-project) +add_vue_test(manage-measurements) +add_vue_test(header-menu) +add_vue_test(test-details) diff --git a/tests/Spec/page-header/header-menu.spec.js b/tests/Spec/header-menu.spec.js similarity index 97% rename from tests/Spec/page-header/header-menu.spec.js rename to tests/Spec/header-menu.spec.js index 74d36e40c3..d227b80a37 100644 --- a/tests/Spec/page-header/header-menu.spec.js +++ b/tests/Spec/header-menu.spec.js @@ -1,6 +1,6 @@ import {mount, config, createWrapper} from '@vue/test-utils'; config.global.mocks['$baseURL'] = 'http://localhost'; -import HeaderMenu from '../../../resources/js/vue/components/page-header/HeaderMenu.vue'; +import HeaderMenu from '../../resources/js/vue/components/page-header/HeaderMenu.vue'; import expect from 'expect'; diff --git a/tests/cypress/component/CMakeLists.txt b/tests/cypress/component/CMakeLists.txt new file mode 100644 index 0000000000..ee0e555c6f --- /dev/null +++ b/tests/cypress/component/CMakeLists.txt @@ -0,0 +1,21 @@ +function(add_cypress_component_test TestName) + set_app_url() + + add_test( + NAME cypress/component/${TestName} + COMMAND ${NPX_EXE} cypress run + --component + --project ${CDash_SOURCE_DIR} + --spec ${CDash_SOURCE_DIR}/tests/cypress/component/${TestName}.cy.js + --config baseUrl=${APP_URL} + ) + # Cypress tries to put stuff in our home directory, which doesn't work for /var/www. + set_tests_properties(cypress/component/${TestName} PROPERTIES + ENVIRONMENT "HOME=${CDash_BINARY_DIR};" + DISABLED "$" + RESOURCE_LOCK "dev-server-port" + ) +endfunction() + +add_cypress_component_test(data-table) +add_cypress_component_test(loading-indicator) diff --git a/tests/cypress/component/data-table.cy.js b/tests/cypress/component/data-table.cy.js index 7fe4d50682..916a4b65b3 100644 --- a/tests/cypress/component/data-table.cy.js +++ b/tests/cypress/component/data-table.cy.js @@ -171,7 +171,7 @@ describe('Data table component tests', () => { { test: 'Data value 1', test2: { - value: 'Link 1', + text: 'Link 1', href: 'http://localhost:8080/test', }, }, diff --git a/tests/cypress/e2e/CMakeLists.txt b/tests/cypress/e2e/CMakeLists.txt index be818423fd..dacc97e392 100644 --- a/tests/cypress/e2e/CMakeLists.txt +++ b/tests/cypress/e2e/CMakeLists.txt @@ -1,3 +1,44 @@ +function(set_app_url) + if(DEFINED ENV{APP_URL}) + set(APP_URL "$ENV{APP_URL}" PARENT_SCOPE) + elseif(EXISTS ${CDash_SOURCE_DIR}/.env) + file(STRINGS ${CDash_SOURCE_DIR}/.env env_vars) + foreach(var IN LISTS env_vars) + if(var MATCHES "^APP_URL=(.*)$") + set(APP_URL "${CMAKE_MATCH_1}" PARENT_SCOPE) + break() + endif() + endforeach() + else() + set(APP_URL "http://localhost:8080" PARENT_SCOPE) + endif() +endfunction() + +function(add_cypress_e2e_test TestName) + set_app_url() + + add_test( + NAME cypress/e2e/${TestName} + COMMAND xvfb-run -a ${NPX_EXE} cypress run + --e2e + --project ${CDash_SOURCE_DIR} + --spec ${CDash_SOURCE_DIR}/tests/cypress/e2e/${TestName}.cy.js + --config baseUrl=${APP_URL} + ) + # Cypress tries to put stuff in our home directory, which doesn't work for /var/www. + set_tests_properties(cypress/e2e/${TestName} PROPERTIES + ENVIRONMENT "HOME=${CDash_BINARY_DIR};" + DISABLED "$" + ) +endfunction() + + +# Completely independent Cypress tests +add_cypress_e2e_test(user-profile) +set_tests_properties(cypress/e2e/user-profile PROPERTIES DEPENDS install_1) + + +# Cypress tests which require data from the legacy test suite add_cypress_e2e_test(manage-overview) set_tests_properties(cypress/e2e/manage-overview PROPERTIES DEPENDS simple2_async) @@ -71,4 +112,7 @@ add_cypress_e2e_test(build-configure) set_tests_properties(cypress/e2e/build-configure PROPERTIES DEPENDS cypress/e2e/tests) add_cypress_e2e_test(all-projects) -set_tests_properties(cypress/e2e/all-projects PROPERTIES DEPENDS cypress/e2e/build-configure) +set_tests_properties(cypress/e2e/all-projects PROPERTIES DEPENDS projectindb) + +add_cypress_e2e_test(banner) +set_tests_properties(cypress/e2e/banner PROPERTIES DEPENDS simple2_async)