Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Disallow starting build-time check method names with 'test_' #44730

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

tldahlgren
Copy link
Contributor

This PR flags attempts to run build-time check methods (https://spack.readthedocs.io/en/latest/packaging_guide.html#adding-installation-phase-tests) as stand-alone methods (https://spack.readthedocs.io/en/latest/packaging_guide.html#adding-stand-alone-tests) because the check method's name incorrectly starts with test_, which is now reserved for stand-alone test methods. Left alone, the test appears to pass when it was never actually run.

Motivation

Updates to stand-alone tests revealed a flaw in processing build-time check method names. If someone gives a build-time (e.g., @run_after) method a name starting test_, spack test run will claim it runs it successfully when it doesn't actually execute the method. This PR

For example, given a build-time check of test_execute for libstdcompat -- that calls make("test") -- basically runs as expected for spack install --test=root libstdcompat:

==> Testing package libstdcompat-0.0.20-h7drrbe
==> [2024-06-14-12:10:02.159556] Running build-time tests
==> [2024-06-14-12:10:02.159716] RUN-TESTS: build-time tests [check]
==> [2024-06-14-12:10:02.162396] 'make' '-j16' '-n' 'test'
==> [2024-06-14-12:10:02.183327] 'make' '-j16' 'test'
Running tests...
$spack/opt/spack/linux-rhel8-x86_64_v3/gcc-12.1.1/cmake-3.27.9-qg3djkkjmhzmgufb6de3qvpmyczz3fzf/bin/ctest --force-new-ctest-process 
Test project /tmp/dahlgren/spack-stage/spack-stage-libstdcompat-0.0.20-h7drrbezv6x7z5t354leunyyom4cafmc/spack-build-h7drrbe
    Start 1: stdcompat.integation1
1/1 Test #1: stdcompat.integation1 ............   Passed    0.00 sec

100% tests passed, 0 tests failed out of 1

Total Test time (real) =   0.00 sec
==> [2024-06-14-12:10:02.264505] 'make' '-j16' '-n' 'check'
==> [2024-06-14-12:10:02.284177] Target 'check' not found in Makefile

BUT it also appears to run successfully as a stand-alone test:

==> Testing package libstdcompat-0.0.20-h7drrbe
==> [2024-06-14-13:12:07.361367] test: test_execute: Test if libstdcompat executes correctly
PASSED: Libstdcompat::test_execute
==> [2024-06-14-13:12:07.364175] Completed testing
==> [2024-06-14-13:12:07.364308] 
===================== SUMMARY: libstdcompat-0.0.20-h7drrbe =====================
Libstdcompat::test_execute .. PASSED
============================= 1 passed of 1 parts ==============================

even though there is no way for the test to actually run/do anything.

TODO

  • Add unit test(s)

@spackbot-app spackbot-app bot added the core PR affects Spack core functionality label Jun 14, 2024
@tldahlgren tldahlgren added the bugfix Something wasn't working, here's a fix label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Something wasn't working, here's a fix core PR affects Spack core functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant