Skip to content

Commit

Permalink
Reorganize CMake/CTest scripts (#2698)
Browse files Browse the repository at this point in the history
Following the work in #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.
  • Loading branch information
williamjallen authored Feb 4, 2025
1 parent f78e107 commit 305ce6c
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 188 deletions.
12 changes: 0 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
26 changes: 11 additions & 15 deletions app/cdash/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,23 +65,17 @@ 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)

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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions app/cdash/tests/htaccess.in

This file was deleted.

55 changes: 0 additions & 55 deletions app/cdash/tests/test_autoremovebuilds.php

This file was deleted.

27 changes: 1 addition & 26 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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\\(\\)\\:
Expand Down Expand Up @@ -32056,7 +32031,7 @@ parameters:

-
message: "#^Dynamic call to static method Illuminate\\\\Http\\\\Response\\:\\:content\\(\\)\\.$#"
count: 1
count: 2
path: tests/Feature/SlowPageTest.php

-
Expand Down
74 changes: 2 additions & 72 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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 "$<STREQUAL:${CDASH_IMAGE},ubi>"
)
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 "$<STREQUAL:${CDASH_IMAGE},ubi>"
)
endfunction()

add_test(
NAME php_style_check
COMMAND ${CMAKE_SOURCE_DIR}/vendor/bin/php-cs-fixer fix --dry-run --allow-risky=yes
Expand Down Expand Up @@ -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)
7 changes: 4 additions & 3 deletions tests/Feature/SlowPageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
14 changes: 14 additions & 0 deletions tests/Spec/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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';

Expand Down
21 changes: 21 additions & 0 deletions tests/cypress/component/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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 "$<STREQUAL:${CDASH_IMAGE},ubi>"
RESOURCE_LOCK "dev-server-port"
)
endfunction()

add_cypress_component_test(data-table)
add_cypress_component_test(loading-indicator)
2 changes: 1 addition & 1 deletion tests/cypress/component/data-table.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
Expand Down
Loading

0 comments on commit 305ce6c

Please sign in to comment.