Skip to content

Add enable_testing() call into test\CMakeLists.txt #2590

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.

Sign up for GitHub

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

Open
SergeyKopienko wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add enable_testing() call into test\CMakeLists.txt #2590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1e3f7b
test/CMakeLists.txt - add enable_testing() call
SergeyKopienko Feb 25, 2026
29a2a93
test/CMakeLists.txt - add enable_testing() call : move before add_sub…
SergeyKopienko Feb 25, 2026
4d20cf2
CMakeLists.txt - fix review comment: Let's then remove enable_testing…
SergeyKopienko Feb 25, 2026
130ee19
Merge branch 'main' into dev/skopienko/ebable_tests
SergeyKopienko Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,6 @@ target_include_directories(oneDPL
# Setup tests
###############################################################################
if (NOT _onedpl_is_subproject)
enable_testing()
add_subdirectory(test)
endif()

Expand Down
2 changes: 2 additions & 0 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
# See https://llvm.org/LICENSE.txt for license information.
#
##===----------------------------------------------------------------------===##
enable_testing()
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

This call to enable_testing() is redundant. The root CMakeLists.txt already calls enable_testing() at line 357 before adding the test subdirectory via add_subdirectory(test) at line 358. Since enable_testing() propagates to all subdirectories, tests in test/kt/ and other subdirectories are already enabled by the root-level call. While this redundant call is harmless (enable_testing() is idempotent), it doesn't provide any additional functionality.

Suggested change
enable_testing()

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@SergeyKopienko SergeyKopienko Feb 25, 2026

Choose a reason for hiding this comment

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

Already existing call of enable_testing() at

oneDPL/CMakeLists.txt

Line 357 in 7500e3d

enable_testing()
is not enough at least for MS Visual Studio 2026 to see built tests list in Test Explorer.

Copy link
Copy Markdown
Contributor

@dmitriy-sobolev dmitriy-sobolev Feb 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Let's then remove enable_testing from the root directory and keep it here.

I think it will be closer to what enable_testing expects according to its documentation:

This command should be invoked in the top-level source directory because ctest(1) expects to find a test file in the top-level build directory.

The cmake puts executables into test subdirectory.

Copy link
Copy Markdown
Contributor Author

@SergeyKopienko SergeyKopienko Feb 25, 2026

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Contributor

@dmitriy-sobolev dmitriy-sobolev Feb 25, 2026

Choose a reason for hiding this comment

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

Sorry, I got confused by this cmake note.

Indeed, our cmake will place tests inside test directory, but I now think that the right way is to place it inside the top-level CMakeLists after investigating the sources of other different projects, e.g. see oneTBB, OpenCV or JSON. Perhaps, we should change where add_test should put the files, or there is a bug in VS2026 and we should do it only for this configuration. Let me think about it...


add_subdirectory(kt)

# rng_tests
Expand Down