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.

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

Jump to bottom

Support of googletest using colcon [12009] #2054

Merged
merged 8 commits into from
Jul 9, 2021
Merged

Support of googletest using colcon [12009] #2054

merged 8 commits into from
Jul 9, 2021

Conversation

richiware
Copy link
Member

No description provided.

richiware added 2 commits July 7, 2021 07:53
@richiware
Refs #11961. New support of googletest using colcon
71ceba7 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware
Refs #11961. Fix compilation errors
6f527b9 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware richiware changed the title Support of googletest using colcon Support of googletest using colcon [12009] Jul 7, 2021
@richiware richiware requested a review from MiguelCompany July 7, 2021 14:13
richiware added 6 commits July 8, 2021 07:34
@richiware
Refs #11961. Fix compilation error
4dd638d 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware
Refs #11961. Including asio in a good way
1bdf483 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware
Refs #11961. gmock exports gtest
796803c 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware
Refs #11961. Forget to remove one include_directories
9f3cf89 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware
Refs #11961. Little change
2d13824 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@richiware
Refs #11961. Fix error
f245ed9 
Signed-off-by: Ricardo González <ricardo@richiware.dev>
@MiguelCompany MiguelCompany requested review from IkerLuengo and removed request for MiguelCompany July 8, 2021 13:43
IkerLuengo
IkerLuengo approved these changes Jul 8, 2021
View reviewed changes
Copy link
Contributor

@IkerLuengo IkerLuengo left a comment

Choose a reason for hiding this comment

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

LGTM,

Just a couple of comments, mostly because I'm curious

test/unittest/statistics/rtps/CMakeLists.txt
${PROJECT_SOURCE_DIR}/include
${PROJECT_BINARY_DIR}/include
${PROJECT_SOURCE_DIR}/src/cpp
${Asio_INCLUDE_DIR}
)

target_link_libraries(RTPSStatisticsTests fastrtps fastcdr ${GTEST_LIBRARIES} ${GMOCK_LIBRARIES})
target_link_libraries(RTPSStatisticsTests fastrtps fastcdr GTest::gtest GTest::gmock)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
target_link_libraries(RTPSStatisticsTests fastrtps fastcdr GTest::gtest GTest::gmock)
target_link_libraries(RTPSStatisticsTests fastrtps fastcdr GTest::gmock)

Is not enough with gmock? I see in the rest of cases where both Gtest and Gmock were added, only gmock is added in your changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was reading the cmake config file that googletest-distribution generates, and the target GTest::gmock imports also GTest::gtest and Threads

test/unittest/rtps/flowcontrol/CMakeLists.txt
@@ -44,7 +42,7 @@ if(NOT ((MSVC OR MSVC_IDE) AND EPROSIMA_INSTALLER))
${PROJECT_SOURCE_DIR}/include ${PROJECT_BINARY_DIR}/include
${PROJECT_SOURCE_DIR}/src/cpp
)
target_link_libraries(ThroughputControllerTests ${GTEST_LIBRARIES} ${GTEST_LIBRARIES} ${GMOCK_LIBRARIES})
target_link_libraries(ThroughputControllerTests GTest::gtest GTest::gmock)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Member Author

Choose a reason for hiding this comment

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

Here will enough with only GTest::gmock.

@MiguelCompany MiguelCompany added this to the v2.3.4 milestone Jul 9, 2021
@richiware
Copy link
Member Author

Failed tests not related.

@richiware richiware merged commit 4f9f7e9 into master Jul 9, 2021
@richiware richiware deleted the feature/11961 branch July 9, 2021 06:12
@MiguelCompany MiguelCompany modified the milestone: v2.3.4 Jul 9, 2021
@MiguelCompany
Copy link
Member

@Mergifyio backport 2.1.x

mergify bot pushed a commit that referenced this pull request Mar 22, 2022
@richiware
Support of googletest using colcon [12009] (#2054)
a3acef8 
* Refs #11961. New support of googletest using colcon

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11961. Fix compilation errors

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11961. Fix compilation error

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11961. Including asio in a good way

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11961. gmock exports gtest

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11961. Forget to remove one include_directories

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11961. Little change

Signed-off-by: Ricardo González <ricardo@richiware.dev>

* Refs #11961. Fix error

Signed-off-by: Ricardo González <ricardo@richiware.dev>
(cherry picked from commit 4f9f7e9)

# Conflicts:
#	examples/C++/DDS/DeadlineQoSExample/CMakeLists.txt
#	examples/C++/DeadlineQoSExample/CMakeLists.txt
#	test/blackbox/CMakeLists.txt
#	test/communication/CMakeLists.txt
#	test/performance/CMakeLists.txt
#	test/performance/latency/CMakeLists.txt
#	test/performance/throughput/CMakeLists.txt
#	test/performance/video/CMakeLists.txt
#	test/profiling/CMakeLists.txt
#	test/realtime/CMakeLists.txt
#	test/unittest/dds/collections/CMakeLists.txt
#	test/unittest/dds/core/condition/CMakeLists.txt
#	test/unittest/dds/core/entity/CMakeLists.txt
#	test/unittest/dds/publisher/CMakeLists.txt
#	test/unittest/dds/status/CMakeLists.txt
#	test/unittest/dynamic_types/CMakeLists.txt
#	test/unittest/logging/CMakeLists.txt
#	test/unittest/logging/log_macros/CMakeLists.txt
#	test/unittest/rtps/common/CMakeLists.txt
#	test/unittest/rtps/flowcontrol/CMakeLists.txt
#	test/unittest/rtps/network/CMakeLists.txt
#	test/unittest/rtps/persistence/CMakeLists.txt
#	test/unittest/rtps/resources/timedevent/CMakeLists.txt
#	test/unittest/rtps/security/CMakeLists.txt
#	test/unittest/rtps/writer/CMakeLists.txt
#	test/unittest/security/accesscontrol/CMakeLists.txt
#	test/unittest/security/authentication/CMakeLists.txt
#	test/unittest/statistics/dds/CMakeLists.txt
#	test/unittest/statistics/rtps/CMakeLists.txt
#	test/unittest/transport/CMakeLists.txt
#	test/unittest/utils/CMakeLists.txt
#	test/unittest/xmlparser/CMakeLists.txt
#	test/unittest/xtypes/CMakeLists.txt
#	test/xtypes/CMakeLists.txt
@mergify mergify bot mentioned this pull request Mar 22, 2022
Closed
@mergify
Copy link
Contributor

mergify bot commented Mar 22, 2022

backport 2.1.x

✅ Backports have been created

JLBuenoLopez added a commit that referenced this pull request Jul 19, 2022
@JLBuenoLopez
Refs #15163: backport #2054 to 2.1.x
fba6e46 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
MiguelCompany pushed a commit that referenced this pull request Jul 21, 2022
@JLBuenoLopez
Support of googletest using colcon (#2852)
00d2bf7 
* Refs #15163: backport #2054 to 2.1.x

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #15163: fix RTPSWriter unittest build

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #15163: fix RTPSWriter unittest build (second attempt)

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs #15163: use correct variable name

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IkerLuengo IkerLuengo IkerLuengo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.3.4
Development

Successfully merging this pull request may close these issues.
3 participants
@richiware @MiguelCompany @IkerLuengo