Skip to content

Add tests for TimeSource #22

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
Merged
merged 9 commits into from
Sep 17, 2020
Merged

Add tests for TimeSource #22

merged 9 commits into from
Sep 17, 2020

Conversation

@jacobperron
Copy link

@jacobperron jacobperron commented Aug 25, 2020

Also fixed what I think was a bug in ca0a225.

This change introduces a new dependency mockito, which I think is super useful for writing tests with mock objects. I've proposed a rosdep rule for it in ros/rosdistro#26308.
Note, if we want to eventually support Windows, we'll have to investigate installing the jars for mockito and its dependencies (perhaps like how we handle JUnit).

@ivanpauno
I first wanted to get your thoughts on introducing this dependency before putting more effort into supporting Windows and resolving the TODO in the CMakeLists.txt.

@jacobperron
Notify clocks when use_sim_time parameter changes
ca0a225 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Add unit tests for TimeSource class
0c607ff 
Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Add test dependency on libmockito-java
1b1d628 
Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron self-assigned this Aug 25, 2020
@jacobperron jacobperron requested a review from ivanpauno August 25, 2020 21:22
@jacobperron
Minor refactor
ef7bf22 
- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Author

jacobperron commented Aug 26, 2020

Note, CI is dependent on the rosdistro PR. Alternatively, I can implement what I proposed for Windows and conditionally download and install the mockito jars if a sufficient version is not already installed on the system.

@ivanpauno
Copy link
Collaborator

ivanpauno commented Aug 26, 2020

I first wanted to get your thoughts on introducing this dependency before putting more effort into supporting Windows and resolving the TODO in the CMakeLists.txt.

The new dependency looks great to me, Mockito seems to be very powerful.

Note, if we want to eventually support Windows, we'll have to investigate installing the jars for mockito and its dependencies (perhaps like how we handle JUnit).

Maybe we should have a "vendor package" for those extra jars.
That vendor package could set MOCKITO_JARS to the correct thing when find packaged (I'm not quite sure of cmake variable naming conventions for java projects).

@jacobperron
Copy link
Author

jacobperron commented Aug 26, 2020

Sounds good. I'll create a vendor package and refactor this PR to use it. I'm not sure about naming conventions either (I don't think Java is typically built with CMake), but I'll try to be consistent with what is already in the ros2-java project.

@jacobperron jacobperron mentioned this pull request Aug 28, 2020
Merged
@jacobperron
Copy link
Author

jacobperron commented Aug 28, 2020

I've created a vendor package in ros2-java/mockito_vendor#1, I'll update this PR to use it

@jacobperron
Depend on mockito_vendor package
a18ce85 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Add mockito_vendor to repos file
919f0e1 
This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron
jacobperron commented Aug 28, 2020
View reviewed changes
@jacobperron jacobperron requested review from ivanpauno and removed request for ivanpauno August 28, 2020 23:55
ivanpauno
ivanpauno approved these changes Aug 31, 2020
View reviewed changes
Copy link
Collaborator

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM

@jacobperron
Update branch for mockito_vendor
2e5ccb8 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Author

jacobperron commented Sep 8, 2020

I'm not sure why CI is failing... mockito_vendor appears to be cloned to the workspace, but it is not seen by rosdep or colcon 🤔

@jacobperron
Try out action-ros-ci patch
35e926e 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Author

jacobperron commented Sep 9, 2020

Looks like it was a bug with the Action action-ros-ci (ros-tooling/action-ros-ci#342).

I've made a patch to the action and updated the workflow here to see if it resolves the issue 35e926e.

@ivanpauno
Copy link
Collaborator

ivanpauno commented Sep 9, 2020

Looks like it was a bug with the Action action-ros-ci (ros-tooling/action-ros-ci#342).

That's a fun bug 😂

@jacobperron
Switch back to upstream version of GitHub action
c4e8ed7 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron force-pushed the jacob/time_source_tests branch from 801bc3c to c4e8ed7 Compare September 17, 2020 16:50
ivanpauno
ivanpauno approved these changes Sep 17, 2020
View reviewed changes
.github/workflows/build_and_test.yml
with:
required-ros-distributions: foxy
- uses: ros-tooling/action-ros-ci@0.0.19
- uses: ros-tooling/action-ros-ci@master
Copy link
Collaborator

@ivanpauno ivanpauno Sep 17, 2020

Choose a reason for hiding this comment

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

ros-tooling/action-ros-ci#343 🎉

@jacobperron jacobperron merged commit 00ab08b into feature/ros_time Sep 17, 2020
@jacobperron jacobperron deleted the jacob/time_source_tests branch September 17, 2020 18:10
jacobperron added a commit that referenced this pull request Sep 18, 2020
@jacobperron
Add tests for TimeSource (#22)
04c6849 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Sep 18, 2020
@jacobperron
Add tests for TimeSource (#22)
0ac09d6 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Sep 18, 2020
@jacobperron
Add tests for TimeSource (#22)
c31cccc 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit that referenced this pull request Sep 18, 2020
@jacobperron
Add tests for TimeSource (#22)
9c77e3b 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
ivanpauno pushed a commit that referenced this pull request May 17, 2021
@jacobperron @ivanpauno
Add tests for TimeSource (#22)
63ed397 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 13, 2022
@jacobperron
Add tests for TimeSource (osrf#22)
4f279ce 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 13, 2022
@jacobperron
Add tests for TimeSource (osrf#22)
0822281 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 13, 2022
@jacobperron
Add tests for TimeSource (osrf#22)
6275ff5 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2-java/ros2_java that referenced this pull request May 17, 2022
@jacobperron
Add tests for TimeSource (osrf#22)
e8837c8 
* Notify clocks when use_sim_time parameter changes

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add unit tests for TimeSource class

Introduce Mockito dependency to help with creating mock objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add test dependency on libmockito-java

Depends on ros/rosdistro#26308

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Minor refactor

- Switch to using @mock annotation.
- Cleanup

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on mockito_vendor package

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add mockito_vendor to repos file

This should make CI green.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update branch for mockito_vendor

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ivanpauno ivanpauno ivanpauno approved these changes

Assignees

@jacobperron jacobperron

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jacobperron @ivanpauno