Skip to content

fix test set properties (with FDB_BUILD_TOOLS disabled)#232

Merged
Ozaq merged 3 commits intodevelopfrom
fix/test-properties
Feb 25, 2026
Merged

fix test set properties (with FDB_BUILD_TOOLS disabled)#232
Ozaq merged 3 commits intodevelopfrom
fix/test-properties

Conversation

@danovaro
Copy link
Member

@danovaro danovaro commented Feb 24, 2026

Description

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

🌈🌦️📖🚧 Documentation Z3FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/z3fdb/pull-requests/PR-232

🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/fdb/pull-requests/PR-232

Comment on lines 23 to 25
if (TARGET test_fdb_tools_wipe_${_test})
set_tests_properties(test_fdb_tools_wipe_${_test} PROPERTIES RESOURCE_LOCK fdb_tools_wipe_lock)
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dumb question, but ecbuild_add_test supports PROPERTIES and TEST_PROPERTIES, why not just add the LOCK in the call with condition above?

Copy link
Member Author

@danovaro danovaro Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I tried setting PROPERTIES inside ecbuild_add_test:

ecbuild_add_test(
        TARGET test_fdb_tools_wipe_${_test}
        ...
        PROPERTIES
        RESOURCE_LOCK fdb_tools_wipe_lock
        )

getting the following error:

CMake Error at /usr/local/share/ecbuild/cmake/ecbuild_add_test.cmake:475 (set_target_properties):
  set_target_properties Can not find target to add properties to:
  test_fdb_tools_wipe_shared_root
Call Stack (most recent call first):
  fdb5/tests/fdb/tools/wipe/CMakeLists.txt:13 (ecbuild_add_test)

But you are right @Ozaq, TEST_PROPERTIES seems to works well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just saw it in the doc, thanks for trying it out :)

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.03%. Comparing base (10c24f3) to head (d189ac8).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #232   +/-   ##
========================================
  Coverage    74.02%   74.03%           
========================================
  Files          376      376           
  Lines        23738    23738           
  Branches      2469     2471    +2     
========================================
+ Hits         17572    17574    +2     
+ Misses        6166     6164    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ozaq Ozaq merged commit fb08876 into develop Feb 25, 2026
366 of 383 checks passed
@Ozaq Ozaq deleted the fix/test-properties branch February 25, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants