View Issue Details [ Jump to Notes ]

[ Print ]

ID

Project

Category

View Status

Date Submitted

Last Update

0010612

CMake

Modules

public

2010-04-26 04:07

2016-06-10 14:31

Reporter

Jens Mueller

Assigned To

Philip Lowman

Priority

normal

Severity

feature

Reproducibility

always

Status

closed

Resolution

moved

Platform

OS

OS Version

Product Version

CMake-2-8

Target Version

Fixed in Version

Summary

0010612: GTEST_ADD_TESTS in FindGTest.cmake ignores disabled tests

Description

The GTEST_ADD_TESTS function in FindGTest.cmake does not handle disabled tests (see http://code.google.com/p/googletest/wiki/GoogleTestAdvancedGuide#Temporarily_Disabling_Tests [^])
I added this feature. The simple patch is attached.
I'd like to enhance it such that this functionality depends on a variable, e.g. IGNORE_DISABLED_GTEST with some good default. I still have to figure out how to do this with CMake. Any hints are welcome.
Further the function should to my mind make sure that CMake is automatically re-run if a test is added to a file. Because this avoids manual re-running of CMake which is error-prone. Unfortunately I don't know how to implement this.

Tags

No tags attached.

Attached Files

gtest.patch [^] (1,067 bytes) 2010-04-26 04:07 [Show Content] [Hide Content]

diff --git a/Modules/FindGTest.cmake b/Modules/FindGTest.cmake
index 1bcd469..74ec560 100644
--- a/Modules/FindGTest.cmake
+++ b/Modules/FindGTest.cmake
@@ -79,8 +79,13 @@ function(GTEST_ADD_TESTS executable extra_args)
         file(READ "${source}" contents)
         string(REGEX MATCHALL "TEST_?F?\\(([A-Za-z_0-9 ,]+)\\)" found_tests ${contents})
         foreach(hit ${found_tests})
-            string(REGEX REPLACE ".*\\( *([A-Za-z_0-9]+), *([A-Za-z_0-9]+) *\\).*" "\\1.\\2" test_name ${hit})
-            add_test(${test_name} ${executable} --gtest_filter=${test_name} ${extra_args})
+            string(REGEX REPLACE ".*\\(([A-Za-z_0-9]+)[, ]*([A-Za-z_0-9]+)\\).*" "\\1.\\2" test_name ${hit})
+            # checks if test is disabled
+            if (NOT ${test_name} MATCHES "^DISABLED_|\\.DISABLED_")
+                add_test(${test_name} ${executable} --gtest_filter=${test_name} ${extra_args})
+            else()
+                message("Test ${test_name} is _not_ added.")
+            endif()
         endforeach()
     endforeach()
 endfunction()

Relationships

Notes
(0021861) Philip Lowman (developer) 2010-08-19 23:25	Sorry this languished in the tracker for so long. To answer your question, when we want to enable an optional change in behavior in a module distributed with CMake, generally we add a boolean variable to the interface for the module (e.g. GTEST_WHATEVER) and document what it does and when it was introduced. After having looked over the DISABLED feature of GTest, I don't think it calls for the user setting a special variable so that tests called DISABLED are actually not run by the CMake macro. In other words your patch seems fine to me. The only thing I would add is a way to force the tests to be run even if they are DISABLED. This could be a variable check. Also GTest has an environment variable that we could obey that you could check for with $ENV{GTEST_ALSO_RUN_DISABLED_TESTS}. Regarding your second question, I can't think of a way to have CMake automatically re-run if a (C++) source file is modified. There might be a way to do this. It's probably not the behavior you're going to want, however, because what will inevitably happen is that 97% of your file writes to the unit test source file are not going to be adding new test fixtures. You'll slow down the common case re-running CMake everytime you make after a modification. Aside from drilling it into people to rerun CMake after adding a new unit test fixture I can't think of a good solution to the problem. How would you like to proceed?

(0021865) Jens Mueller (reporter) 2010-08-20 03:00	I meant it as you say above: Running tests even if they are disabled. That will be useful. And that should default to false. I just reread the googletest documentation (http://code.google.com/p/googletest/wiki/AdvancedGuide#Temporarily_Disabling_Tests [^]) and I think my patch doesn't handle disabling the complete test case. "If you need to disable all tests in a test case, you can either add DISABLED_ to the front of the name of each test, or alternatively _add it to the front of the test case name_." That might be difficult to add. Your right about the second point. But I still don't like the idea that tests are not run automatically by ctest if I add them. One could write a ctest script that runs cmake each and every time. So when you are testing then use this script. That sounds reasonable to me. In summary I agree with you and like to have the functionality added into FindGTest.cmake. Can you add the changes? I can offer reviewing and testing on a Linux machine. If you see an easy way to add disabling all tests by disabling the test case that would be fine too. Many thanks.

(0041687) Kitware Robot (administrator) 2016-06-10 14:27	Resolving issue as `moved`. This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.

Issue History
Date Modified	Username	Field	Change
2010-04-26 04:07	Jens Mueller	New Issue
2010-04-26 04:07	Jens Mueller	File Added: gtest.patch
2010-05-08 03:38	Alex Neundorf	Category	CMake => Modules
2010-08-19 08:16	Philip Lowman	Status	new => assigned
2010-08-19 08:16	Philip Lowman	Assigned To	=> Philip Lowman
2010-08-19 23:25	Philip Lowman	Note Added: 0021861
2010-08-20 03:00	Jens Mueller	Note Added: 0021865
2016-06-10 14:27	Kitware Robot	Note Added: 0041687
2016-06-10 14:27	Kitware Robot	Status	assigned => resolved
2016-06-10 14:27	Kitware Robot	Resolution	open => moved
2016-06-10 14:31	Kitware Robot	Status	resolved => closed

Copyright © 2000 - 2018 MantisBT Team