Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Address comments for PR#451:
  * Move test folder in every module;
  * Remove mock objects;
  * Clarify comment;
  * Add a simple test.
  • Loading branch information
vgvassilev committed Apr 6, 2017
commit 2befa51c0308b8d0bd1ad01f3d1eacf4e2c0a7db
15 changes: 9 additions & 6 deletions cmake/modules/RootNewMacros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1079,16 +1079,19 @@ function(ROOT_ADD_TEST test)
endfunction()

#----------------------------------------------------------------------------
# function ROOT_ADD_GTEST(<testsuite> <name> LIBRARIES)
# function ROOT_ADD_GTEST(<testsuite> source1 source2... LIBRARIES)
Copy link
Member

Choose a reason for hiding this comment

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

Is that LIBRARIES <lib1> <lib2> or how does that work? Could you clarify in the comment/doc?

#
function(ROOT_ADD_GTEST test_suite test_name)
function(ROOT_ADD_GTEST test_suite)
CMAKE_PARSE_ARGUMENTS(ARG "" "" "LIBRARIES" ${ARGN})

include_directories(${GTEST_INCLUDE_DIR} ${GMOCK_INCLUDE_DIR})
Copy link
Member

Choose a reason for hiding this comment

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

Can we add the source directory to the include dirs, or does that happen by default? (I want to be able to #include a header in the tests.)

Copy link
Member

Choose a reason for hiding this comment

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

Ping?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.


# Note we cannot use ROOT_EXECUTABLE because it requires to pass LIBRARIES to link with.
# The test suites should chose this in their specific CMakeLists.txt file.
ROOT_EXECUTABLE(${test_suite} ${test_name} LIBRARIES ${ARG_LIBRARIES})
set(source_files ${ARG_UNPARSED_ARGUMENTS})
# Note we cannot use ROOT_EXECUTABLE without user-specified set of LIBRARIES to link with.
# The test suites should choose this in their specific CMakeLists.txt file.
# FIXME: For better coherence we could restrict the libraries the test suite could link
# against. For example, tests in Core should link only against libCore. This could be tricky
# to implement because some ROOT components create more than one library.
ROOT_EXECUTABLE(${test_suite} ${source_files} LIBRARIES ${ARG_LIBRARIES})
target_link_libraries(${test_suite} gtest gtest_main gmock gmock_main)
ROOT_ADD_TEST(gtest-${test_suite} COMMAND ${test_suite})
Copy link
Member

Choose a reason for hiding this comment

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

I believe this is missing some environment:

  set(environment ENVIRONMENT
                  ${ROOTTEST_ENV_EXTRA}
                  ${ARG_ENVIRONMENT}
                  ROOTSYS=${ROOTSYS}
                  PATH=${_path}:$ENV{PATH}
                  PYTHONPATH=${_pythonpath}:$ENV{PYTHONPATH}
                  ${ld_library_path}=${_librarypath}:$ENV{${ld_library_path}})

is what I pass to ROOT_ADD_TEST in roottest/cmake/modules/RootCTestMacros.cmake's ROOTTEST_ADD_UNITTEST_DIR

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I am missing context but it seems this is out of the scope of this PR.

Copy link
Member

Choose a reason for hiding this comment

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

This is needed for runing the tests without setting thisroot.sh before. I.e. no, AFAIK this is a prerequisite.

Copy link
Member

Choose a reason for hiding this comment

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

Ping?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC, this was resolved internally. There was something to check, outside of this PR.

endfunction()
Expand Down
4 changes: 0 additions & 4 deletions core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ add_subdirectory(thread)
add_subdirectory(zip)
add_subdirectory(lzma)

if(testing)
ROOT_ADD_TEST_SUBDIRECTORY(test)
endif()

if(NOT WIN32)
add_subdirectory(newdelete)
endif()
Expand Down
4 changes: 4 additions & 0 deletions core/base/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
# CMakeLists.txt file for building ROOT core/base package
############################################################################

if(testing)
ROOT_ADD_TEST_SUBDIRECTORY(test)
endif()

ROOT_GLOB_HEADERS(Base_dict_headers ${CMAKE_CURRENT_SOURCE_DIR}/inc/T*.h
${CMAKE_CURRENT_SOURCE_DIR}/inc/GuiTypes.h
${CMAKE_CURRENT_SOURCE_DIR}/inc/KeySymbols.h
Expand Down
10 changes: 10 additions & 0 deletions core/base/test/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# FIXME: The tests in core should require only libCore. OTOH, TQObjectTests uses the interpreter to register the class.
# This means that if we run make CoreBaseTests the executable wouldn't be runnable because it requires libCling and
# onepcm targets to be built.
set (link_libs Core)

ROOT_ADD_GTEST(CoreBaseTests
TNamedTests.cxx
TQObjectTests.cxx
LIBRARIES ${link_libs}
)
11 changes: 11 additions & 0 deletions core/base/test/TNamedTests.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#include "gtest/gtest.h"
#include "gmock/gmock.h"

#include "TNamed.h"

TEST(TNamed, Sanity)
{
TNamed n("Name", "Title");
EXPECT_STREQ("Name", n.GetName());
EXPECT_STREQ("Title", n.GetTitle());
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,11 @@

#include "RQ_OBJECT.h"

// FIXME: We should think of a common place to put the mock objects.
class TQConnectionMock : public TQConnection {
public:
virtual ~TQConnectionMock() {} // gmock requires it for cleanup on shutdown.
MOCK_METHOD1(SetArg, void(Long_t param));
MOCK_METHOD1(SetArg, void(ULong_t param));
MOCK_METHOD1(SetArg, void(Float_t param));
MOCK_METHOD1(SetArg, void(Double_t param));
MOCK_METHOD1(SetArg, void(Long64_t param));
MOCK_METHOD1(SetArg, void(ULong64_t param));
MOCK_METHOD1(SetArg, void(const char *param));
MOCK_METHOD2(SetArg, void(const Long_t *params, Int_t nparam /* = -1*/));

MOCK_METHOD0(SendSignal, void());

// MOCK_METHOD1(SendSignal, void());
};

#define Stringify(s) Stringifyx(s)
#define Stringifyx(s) #s

// The interpreter needs to know about RQ_OBJECTTester and using this trick avoids moving this non-reusable class into
// its own header file.
#define DICT_CLASS \
class RQ_OBJECTTester : public TQObject { \
/* This will expand, adding signal/slot support to this class */ \
Expand Down
5 changes: 0 additions & 5 deletions core/test/CMakeLists.txt

This file was deleted.

6 changes: 0 additions & 6 deletions core/test/base/CMakeLists.txt

This file was deleted.