From ca0a2254deb5403c95b38e6646565dfa89e27e89 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 25 Aug 2020 13:46:30 -0700 Subject: [PATCH 1/9] Notify clocks when use_sim_time parameter changes Signed-off-by: Jacob Perron --- rcljava/src/main/java/org/ros2/rcljava/time/TimeSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcljava/src/main/java/org/ros2/rcljava/time/TimeSource.java b/rcljava/src/main/java/org/ros2/rcljava/time/TimeSource.java index 44318c2d..c2d5d3c8 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/time/TimeSource.java +++ b/rcljava/src/main/java/org/ros2/rcljava/time/TimeSource.java @@ -131,7 +131,7 @@ public rcl_interfaces.msg.SetParametersResult callback(List pa for (ParameterVariant param : parameters) { if (param.getName() == "use_sim_time") { if (param.getType() == ParameterType.PARAMETER_BOOL) { - this.timeSource.rosTimeIsActive = param.asBool(); + this.timeSource.setRosTimeIsActive(param.asBool()); } else { result.setSuccessful(false); result.setReason("'use_sim_time' parameter must be a boolean"); From 0c607ff76fff09cc99866b3ac74d21f05fc6dacc Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 25 Aug 2020 13:47:45 -0700 Subject: [PATCH 2/9] Add unit tests for TimeSource class Introduce Mockito dependency to help with creating mock objects. Signed-off-by: Jacob Perron --- rcljava/CMakeLists.txt | 33 +++ .../org/ros2/rcljava/time/TimeSourceTest.java | 188 ++++++++++++++++++ 2 files changed, 221 insertions(+) create mode 100644 rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index 7b31d515..9a2925f7 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -190,6 +190,36 @@ if(BUILD_TESTING) find_package(std_msgs REQUIRED) ament_lint_auto_find_test_dependencies() + # Find mockito and it's dependencies + # TODO(jacobperron): Wrap this into cmake helper + find_jar(MOCKITO_JAR NAME mockito-core) + find_jar(BYTE_BUDDY_JAR NAME byte-buddy) + find_jar(BYTE_BUDDY_AGENT_JAR NAME byte-buddy-agent) + find_jar(OBJENESIS_JAR NAME objenesis) + + if(NOT MOCKITO_JAR) + message(FATAL_ERROR "mockito installation not found") + endif() + + if(NOT BYTE_BUDDY_JAR) + message(FATAL_ERROR "byte-buddy installation not found (mockito dependency)") + endif() + + if(NOT BYTE_BUDDY_AGENT_JAR) + message(FATAL_ERROR "byte-buddy-agent installation not found (mockito dependency)") + endif() + + if(NOT OBJENESIS_JAR) + message(FATAL_ERROR "objenesis installation not found (mockito dependency)") + endif() + + set(MOCKITO_JARS + "${MOCKITO_JAR}" + "${BYTE_BUDDY_JAR}" + "${BYTE_BUDDY_AGENT_JAR}" + "${OBJENESIS_JAR}" + ) + set(${PROJECT_NAME}_message_files "msg/BoundedArrayNested.msg" "msg/BoundedArrayPrimitives.msg" @@ -243,6 +273,7 @@ if(BUILD_TESTING) # "src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java" "src/test/java/org/ros2/rcljava/publisher/PublisherTest.java" "src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java" + "src/test/java/org/ros2/rcljava/time/TimeSourceTest.java" "src/test/java/org/ros2/rcljava/timer/TimerTest.java" ) @@ -258,6 +289,7 @@ if(BUILD_TESTING) # "org.ros2.rcljava.parameters.SyncParametersClientTest" "org.ros2.rcljava.publisher.PublisherTest" "org.ros2.rcljava.subscription.SubscriptionTest" + "org.ros2.rcljava.time.TimeSourceTest" "org.ros2.rcljava.timer.TimerTest" ) @@ -330,6 +362,7 @@ if(BUILD_TESTING) "${rosgraph_msgs_JARS}" "${_${PROJECT_NAME}_jar_file}" "${_${PROJECT_NAME}_messages_jar_file}" + "${MOCKITO_JARS}" APPEND_LIBRARY_DIRS "${_deps_library_dirs}" ) diff --git a/rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java b/rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java new file mode 100644 index 00000000..e1a2c7b5 --- /dev/null +++ b/rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java @@ -0,0 +1,188 @@ +/* Copyright 2020 Open Source Robotics Foundation, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.ros2.rcljava.time; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +// import static org.mockito.Mockito; +import static org.mockito.Mockito.*; + +import org.ros2.rcljava.consumers.Consumer; +import org.ros2.rcljava.interfaces.MessageDefinition; +import org.ros2.rcljava.RCLJava; +import org.ros2.rcljava.node.Node; +import org.ros2.rcljava.parameters.ParameterType; +import org.ros2.rcljava.parameters.ParameterVariant; +import org.ros2.rcljava.subscription.Subscription; +import org.ros2.rcljava.time.TimeSource; + +public class TimeSourceTest { + private Node mockedNode; + + @BeforeClass + public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + + RCLJava.rclJavaInit(); + } + + @AfterClass + public static void tearDownOnce() { + RCLJava.shutdown(); + } + + @Before + public void setUp() { + mockedNode = mock(Node.class); + } + + @Test + public final void testEmptyConstructor() { + TimeSource timeSource = new TimeSource(); + assertFalse(timeSource.getRosTimeIsActive()); + } + + @Test + public final void testConstructorWithNode() { + when(mockedNode.getParameter("use_sim_time")).thenReturn(new ParameterVariant("use_sim_time", false)); + + TimeSource timeSource = new TimeSource(mockedNode); + assertFalse(timeSource.getRosTimeIsActive()); + } + + @Test + public final void testAttachNodeUseSimTimeFalse() { + when(mockedNode.getParameter("use_sim_time")).thenReturn(new ParameterVariant("use_sim_time", false)); + + TimeSource timeSource = new TimeSource(); + timeSource.attachNode(mockedNode); + assertFalse(timeSource.getRosTimeIsActive()); + } + + @Test + public final void testAttachNodeUseSimTimeTrue() { + when(mockedNode.getParameter("use_sim_time")).thenReturn(new ParameterVariant("use_sim_time", true)); + + TimeSource timeSource = new TimeSource(); + timeSource.attachNode(mockedNode); + assertTrue(timeSource.getRosTimeIsActive()); + } + + @Test + public final void testAttachNodeTwice() { + when(mockedNode.getParameter("use_sim_time")).thenReturn(new ParameterVariant("use_sim_time", true)); + + TimeSource timeSource = new TimeSource(); + timeSource.attachNode(mockedNode); + assertTrue(timeSource.getRosTimeIsActive()); + + // Attach the same node again + timeSource.attachNode(mockedNode); + assertTrue(timeSource.getRosTimeIsActive()); + } + + @Test + public final void testDetachNode() { + when(mockedNode.getParameter("use_sim_time")).thenReturn(new ParameterVariant("use_sim_time", true)); + + // Attaches node with ROS time active + TimeSource timeSource = new TimeSource(mockedNode); + assertTrue(timeSource.getRosTimeIsActive()); + + timeSource.detachNode(); + assertFalse(timeSource.getRosTimeIsActive()); + + // Calling detach again shouldn't change anything + timeSource.detachNode(); + assertFalse(timeSource.getRosTimeIsActive()); + } + + @Test + public final void testAttachClock() { + Clock mockedClock = mock(Clock.class); + when(mockedClock.getClockType()).thenReturn(ClockType.ROS_TIME); + + TimeSource timeSource = new TimeSource(); + // Attaching a clock should notifiy the clock + timeSource.attachClock(mockedClock); + verify(mockedClock).setRosTimeIsActive(false); + + // Setting ROS time active should notify clock + timeSource.setRosTimeIsActive(true); + verify(mockedClock).setRosTimeIsActive(true); + } + + @Test(expected = IllegalArgumentException.class) + public final void testAttachClockInvalidType() { + Clock mockedClock = mock(Clock.class); + + TimeSource timeSource = new TimeSource(); + timeSource.attachClock(mockedClock); + } + + @Test + public final void testDetachClock() { + Clock mockedClock = mock(Clock.class); + when(mockedClock.getClockType()).thenReturn(ClockType.ROS_TIME); + + TimeSource timeSource = new TimeSource(); + timeSource.attachClock(mockedClock); + timeSource.detachClock(mockedClock); + + // Setting ROS time active should not notify a detached clock + timeSource.setRosTimeIsActive(true); + verify(mockedClock, never()).setRosTimeIsActive(true); + } + + @Test + public final void testSetRosTimeIsActiveNoNode() { + TimeSource timeSource = new TimeSource(); + timeSource.setRosTimeIsActive(false); + assertFalse(timeSource.getRosTimeIsActive()); + timeSource.setRosTimeIsActive(true); + assertTrue(timeSource.getRosTimeIsActive()); + } + + @Test + public final void testSetRosTimeIsActiveWithNode() { + when(mockedNode.getParameter("use_sim_time")).thenReturn(new ParameterVariant("use_sim_time", false)); + Subscription mockSubscription = mock(Subscription.class); + when(mockedNode.createSubscription(eq(rosgraph_msgs.msg.Clock.class), anyString(), any(Consumer.class))) + .thenReturn(mockSubscription); + + TimeSource timeSource = new TimeSource(mockedNode); + timeSource.setRosTimeIsActive(false); + assertFalse(timeSource.getRosTimeIsActive()); + timeSource.setRosTimeIsActive(true); + assertTrue(timeSource.getRosTimeIsActive()); + // Expect subscription for the "/clock" topic when set active + verify(mockedNode).createSubscription(eq(rosgraph_msgs.msg.Clock.class), eq("/clock"), any(Consumer.class)); + timeSource.setRosTimeIsActive(false); + assertFalse(timeSource.getRosTimeIsActive()); + // Expect subscription removed when set not active + verify(mockedNode).removeSubscription(any(Subscription.class)); + } +} From 1b1d62800935404e788f0f3edcc3fda07ae593bd Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 25 Aug 2020 14:15:13 -0700 Subject: [PATCH 3/9] Add test dependency on libmockito-java Depends on https://github.com/ros/rosdistro/pull/26308 Signed-off-by: Jacob Perron --- rcljava/package.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/rcljava/package.xml b/rcljava/package.xml index 9a7bcbb4..ebdc97f8 100644 --- a/rcljava/package.xml +++ b/rcljava/package.xml @@ -40,6 +40,7 @@ ament_lint_auto ament_lint_common builtin_interfaces + libmockito-java rcl_interfaces rcljava_common rmw_implementation_cmake From ef7bf225adbe2ec1674d8f825c20b9dfbebc1324 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 25 Aug 2020 14:32:47 -0700 Subject: [PATCH 4/9] Minor refactor - Switch to using @Mock annotation. - Cleanup Signed-off-by: Jacob Perron --- .../org/ros2/rcljava/time/TimeSourceTest.java | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java b/rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java index e1a2c7b5..84958b86 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/time/TimeSourceTest.java @@ -15,32 +15,37 @@ package org.ros2.rcljava.time; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; -import org.junit.After; import org.junit.AfterClass; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -// import static org.mockito.Mockito; +import org.junit.runner.RunWith; + import static org.mockito.Mockito.*; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import org.ros2.rcljava.consumers.Consumer; -import org.ros2.rcljava.interfaces.MessageDefinition; import org.ros2.rcljava.RCLJava; import org.ros2.rcljava.node.Node; -import org.ros2.rcljava.parameters.ParameterType; import org.ros2.rcljava.parameters.ParameterVariant; import org.ros2.rcljava.subscription.Subscription; import org.ros2.rcljava.time.TimeSource; +@RunWith(MockitoJUnitRunner.StrictStubs.class) public class TimeSourceTest { + @Mock private Node mockedNode; + @Mock + private Clock mockedClock; + + @Mock + private Subscription mockSubscription; + @BeforeClass public static void setupOnce() throws Exception { // Just to quiet down warnings @@ -54,11 +59,6 @@ public static void tearDownOnce() { RCLJava.shutdown(); } - @Before - public void setUp() { - mockedNode = mock(Node.class); - } - @Test public final void testEmptyConstructor() { TimeSource timeSource = new TimeSource(); @@ -122,7 +122,6 @@ public final void testDetachNode() { @Test public final void testAttachClock() { - Clock mockedClock = mock(Clock.class); when(mockedClock.getClockType()).thenReturn(ClockType.ROS_TIME); TimeSource timeSource = new TimeSource(); @@ -137,15 +136,12 @@ public final void testAttachClock() { @Test(expected = IllegalArgumentException.class) public final void testAttachClockInvalidType() { - Clock mockedClock = mock(Clock.class); - TimeSource timeSource = new TimeSource(); timeSource.attachClock(mockedClock); } @Test public final void testDetachClock() { - Clock mockedClock = mock(Clock.class); when(mockedClock.getClockType()).thenReturn(ClockType.ROS_TIME); TimeSource timeSource = new TimeSource(); @@ -169,7 +165,6 @@ public final void testSetRosTimeIsActiveNoNode() { @Test public final void testSetRosTimeIsActiveWithNode() { when(mockedNode.getParameter("use_sim_time")).thenReturn(new ParameterVariant("use_sim_time", false)); - Subscription mockSubscription = mock(Subscription.class); when(mockedNode.createSubscription(eq(rosgraph_msgs.msg.Clock.class), anyString(), any(Consumer.class))) .thenReturn(mockSubscription); From a18ce85b54f018f5bd394a700f54a1899467c271 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 28 Aug 2020 16:52:28 -0700 Subject: [PATCH 5/9] Depend on mockito_vendor package Signed-off-by: Jacob Perron --- rcljava/CMakeLists.txt | 33 ++------------------------------- rcljava/package.xml | 2 +- 2 files changed, 3 insertions(+), 32 deletions(-) diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index 9a2925f7..6b65e4c8 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -188,38 +188,9 @@ ament_export_jars("share/${PROJECT_NAME}/java/${PROJECT_NAME}.jar") if(BUILD_TESTING) find_package(ament_lint_auto REQUIRED) find_package(std_msgs REQUIRED) + find_package(mockito_vendor REQUIRED) ament_lint_auto_find_test_dependencies() - # Find mockito and it's dependencies - # TODO(jacobperron): Wrap this into cmake helper - find_jar(MOCKITO_JAR NAME mockito-core) - find_jar(BYTE_BUDDY_JAR NAME byte-buddy) - find_jar(BYTE_BUDDY_AGENT_JAR NAME byte-buddy-agent) - find_jar(OBJENESIS_JAR NAME objenesis) - - if(NOT MOCKITO_JAR) - message(FATAL_ERROR "mockito installation not found") - endif() - - if(NOT BYTE_BUDDY_JAR) - message(FATAL_ERROR "byte-buddy installation not found (mockito dependency)") - endif() - - if(NOT BYTE_BUDDY_AGENT_JAR) - message(FATAL_ERROR "byte-buddy-agent installation not found (mockito dependency)") - endif() - - if(NOT OBJENESIS_JAR) - message(FATAL_ERROR "objenesis installation not found (mockito dependency)") - endif() - - set(MOCKITO_JARS - "${MOCKITO_JAR}" - "${BYTE_BUDDY_JAR}" - "${BYTE_BUDDY_AGENT_JAR}" - "${OBJENESIS_JAR}" - ) - set(${PROJECT_NAME}_message_files "msg/BoundedArrayNested.msg" "msg/BoundedArrayPrimitives.msg" @@ -360,9 +331,9 @@ if(BUILD_TESTING) "${builtin_interfaces_JARS}" "${rcl_interfaces_JARS}" "${rosgraph_msgs_JARS}" + "${mockito_vendor_JARS}" "${_${PROJECT_NAME}_jar_file}" "${_${PROJECT_NAME}_messages_jar_file}" - "${MOCKITO_JARS}" APPEND_LIBRARY_DIRS "${_deps_library_dirs}" ) diff --git a/rcljava/package.xml b/rcljava/package.xml index ebdc97f8..9b7de88e 100644 --- a/rcljava/package.xml +++ b/rcljava/package.xml @@ -40,7 +40,7 @@ ament_lint_auto ament_lint_common builtin_interfaces - libmockito-java + mockito_vendor rcl_interfaces rcljava_common rmw_implementation_cmake From 919f0e137fb22c02711e51f1cc55c6ab81d88e3d Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 28 Aug 2020 16:54:07 -0700 Subject: [PATCH 6/9] Add mockito_vendor to repos file This should make CI green. Signed-off-by: Jacob Perron --- ros2_java_desktop.repos | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ros2_java_desktop.repos b/ros2_java_desktop.repos index 3a79af5d..ab924472 100644 --- a/ros2_java_desktop.repos +++ b/ros2_java_desktop.repos @@ -19,6 +19,10 @@ repositories: type: git url: https://github.com/ros2/unique_identifier_msgs version: master + ros2_java/mockito_vendor: + type: git + url: https://github.com/ros2-java/mockito_vendor.git + version: add_package ros2_java/ros2_java: type: git url: https://github.com/osrf/ros2_java.git From 2e5ccb8d99a53a005f2660ace7ed660fd1fee360 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 8 Sep 2020 09:34:13 -0700 Subject: [PATCH 7/9] Update branch for mockito_vendor Signed-off-by: Jacob Perron --- ros2_java_desktop.repos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ros2_java_desktop.repos b/ros2_java_desktop.repos index ab924472..b9772e22 100644 --- a/ros2_java_desktop.repos +++ b/ros2_java_desktop.repos @@ -22,7 +22,7 @@ repositories: ros2_java/mockito_vendor: type: git url: https://github.com/ros2-java/mockito_vendor.git - version: add_package + version: main ros2_java/ros2_java: type: git url: https://github.com/osrf/ros2_java.git From 35e926e6354714562aa5126d6d2eac0a0e9baa7d Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 9 Sep 2020 11:48:27 -0700 Subject: [PATCH 8/9] Try out action-ros-ci patch Signed-off-by: Jacob Perron --- .github/workflows/build_and_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 676de760..0d0a6a2e 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -14,7 +14,7 @@ jobs: - uses: ros-tooling/setup-ros@0.0.25 with: required-ros-distributions: foxy - - uses: ros-tooling/action-ros-ci@0.0.19 + - uses: jacobperron/action-ros-ci@remove_vcs_dir with: package-name: rosidl_generator_java rcljava_common rcljava target-ros2-distro: foxy From c4e8ed73aa9d78d22d98101aabb4d6bed74f8ee4 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Wed, 16 Sep 2020 16:58:27 -0700 Subject: [PATCH 9/9] Switch back to upstream version of GitHub action Signed-off-by: Jacob Perron --- .github/workflows/build_and_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 0d0a6a2e..12a165a0 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -14,7 +14,7 @@ jobs: - uses: ros-tooling/setup-ros@0.0.25 with: required-ros-distributions: foxy - - uses: jacobperron/action-ros-ci@remove_vcs_dir + - uses: ros-tooling/action-ros-ci@master with: package-name: rosidl_generator_java rcljava_common rcljava target-ros2-distro: foxy