Add tutorial on integration testing #4881

abaeyens · 2024-12-01T10:37:09Z

This PR adds a tutorial on integration testing with launch_testing and complements the two existing tutorials on unit testing. This topic was first discussed in #4827 and thereafter on ROS discourse. This is my first PR for the ROS 2 project, all feedback welcome!

Resolves #4827 and closes ros2/launch#812.

Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

ros-discourse · 2024-12-01T10:39:31Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/towards-a-rosdoc-integation-testing-tutorial/40701/2

clalancette

First of all, thank you for the tutorial. This is documentation we really need.

I've left quite a number of comments inline. Please take a look when you get a chance.

source/Tutorials/Intermediate/Testing/Integration.rst

clalancette · 2024-12-01T23:35:23Z

source/Tutorials/Intermediate/Testing/Integration.rst

+* Extend the two basic active tests with one that checks
+  whether the turtle is responsive to twist commands,
+  and another that verifies whether the spawn service
+  sets the turtle to the intended pose
+  (see the `Turtlesim introduction tutorial <../../Beginner-CLI-Tools/Introducing-Turtlesim/Introducing-Turtlesim>`).
+* Instead of Turtlesim, launch the
+  :doc:`Gazebo simulator <../../Advanced/Simulators/Gazebo/Gazebo>`
+  and simulate *your* robot in there, automating tests
+  that would otherwise depend on manually operating your physical robot.
+* Go through the
+  `launch_testing documentation <https://docs.ros.org/en/{DISTRO}/p/launch_testing/index.html>`_
+  and explore the many possibilities for integration testing it offers.


This is basically "homework", and while I don't necessarily disagree with it, this isn't what we do in our other tutorials. I would suggest just removing this for now.

Ah, I see. The 'Next steps' sections in other tutorials point explicitly to other tutorials, while this is indeed more "homework". Removed in 07e9415.

Ryanf55

Nice to see this tutorial written up. How do you deal with the DDS matching of topics taking longer than expected and causing test failures?

Ryanf55 · 2024-12-02T04:38:25Z

source/Tutorials/Intermediate/Testing/Integration.rst

+
+      def test_publishes_pose(self, proc_output):
+          """Check whether pose messages published"""
+          msgs_rx = []


This pattern is what WaitForTopics attempts to help simplify.
ros2/launch_ros#348

Thanks for pointing this out, I'll look into that.

Mentioned the launch_testing_ros package and WaitForTopics in c28db38.

Timple · 2024-12-02T06:49:38Z

Great initiative!

Is there any reason you chose the unittest framework over the pytest framework?

In my experience the pytest framework is much more complete. And the integration with launch_pytest using fixtures works fine.

The README there even suggests it's superior to launch_testing:

launch_testing is an standalone testing tool, which lacks many features:

It's impossible to filter test cases by name and run only some.

It's impossible to mark a test as skipped or xfail.

The error reporting of the tool was custom, and the output wasn't as nice as the output generated by other testing frameworks such as unittest and pytest.

So if we're going to write an intetgration testing tutorial, I'd start off by recommending the recommended tool 🙂

clalancette · 2024-12-02T17:24:56Z

Is there any reason you chose the unittest framework over the pytest framework?

While I agree with you that it would be nice to also have documentation on the launch_pytest framework, at least the core still heavily uses the unittest framework. Thus, I'm fine with taking this documentation and then expanding it later to include the pytest one.

kscottz

I don't have much to add here that @clalancette hasn't already said. There is some inconsistency with the capitalization of turtlesim that needs to be resolved. Assuming the suggestions are merged, and the latter section on color rendering gets broken out to another tutorial (this could be a subsequent pull request), this is good enough to be merged.

source/Tutorials/Intermediate/Testing/Integration.rst

kscottz · 2024-12-02T19:14:00Z

source/Tutorials/Intermediate/Testing/Integration.rst

+A suitable tool to visualize them all together is the
+`NodeJS package Xunit Viewer <https://github.com/lukejpreston/xunit-viewer>`_.
+It converts the XUnit files to HTML or straight into the terminal.
+For example, command and response (without highlighting):
+
+.. code-block:: console
+
+  $ xunit-viewer -r build/app/test_results -c
+    app.test_integration.launch_tests
+        ✗ test_publishes_pose time=0.52
+          - Traceback (most recent call last):
+    File "/home/user/ros_workspace/src/app/test/test_integration.py", line 67, in test_publishes_pose
+        assert len(msgs_rx) > 100
+            ^^^^^^^^^^^^^^^^^^
+    AssertionError
+        ✓ test_exit_codes time=0.0
+        ✓ test_logs_spawning time=0.197
+
+  1 failure, 2 passed
+  Written to: /home/user/ros_workspace/index.html


I would go with @Ryanf55 here and say that we link it as a follow on tutorial at the end.

source/Tutorials/Intermediate/Testing/Integration.rst

kscottz · 2024-12-02T19:19:42Z

source/Tutorials/Intermediate/Testing/Integration.rst

+  sets the turtle to the intended pose
+  (see the `Turtlesim introduction tutorial <../../Beginner-CLI-Tools/Introducing-Turtlesim/Introducing-Turtlesim>`).
+* Instead of Turtlesim, launch the
+  :doc:`Gazebo simulator <../../Advanced/Simulators/Gazebo/Gazebo>`


I am not sure this is necessary. The link is just an intro Gazebo and is a bit unsatisfying. If we had a tutorial on running integration tests with Gazebo that would be worth including.

Removed this section as part of #4881 (comment).

fujitatomoya

besides the comments, linter/doc8 are not happy https://github.com/ros2/ros2_documentation/actions/runs/12104682983/job/33807471087?pr=4881. can you also address those errors?

source/Tutorials/Intermediate/Testing/Integration.rst

abaeyens · 2024-12-07T11:39:22Z

@clalancette
Greatly appreciated that you took your time to review thoroughly. One question related to style, I see that a few of your suggestions (for example this one) remove linebreaks and don't change the content, could you briefly elaborate on the reason behind this?

As-is, I wrapped the lines to 80 characters allowing for clean git diffs. This style guide suggest wrapping, but doesn't mention a desired line length for documentation. Is a 100-character limit preferred for documentation?

@Ryanf55

How do you deal with the DDS matching of topics taking longer than expected and causing test failures?

Such things can happen, though I don't immediately see how this relates specifically to testing. The DDS taking too long to match topics can also arise when deployed, in which case it's a bug, and therefore it's desirable that the integration tests point out that part of our system is failing?

@kscottz
Concerning the section on visualizing test output (#4881 (comment), https://github.com/ros2/ros2_documentation/pull/4881/files#r1865054539), shall we make it part of the existing tutorial Running Tests in ROS 2 from the command line? Or do you prefer a short but separate tutorial? Proposal in 05f16a7.

clalancette · 2024-12-07T13:21:14Z

As-is, I wrapped the lines to 80 characters allowing for clean git diffs. This style guide suggest wrapping, but doesn't mention a desired line length for documentation. Is a 100-character limit preferred for documentation?

Good question. Our style is generally one-sentence-per-line; that allows future diffs to be easier to read. However, we do allow breaking a single sentence into multiple lines if necessary. We don't have an exact number for line-length here, but I think 80 characters is too short. Generally, my feeling is that if a particular sentence is too long to comfortably fit on one line, then usually the sentence is too long (there are exceptions to this, including when we are embedding URLs into the sentence, in which case the written sentence can get very long and we break it up).

I think the best thing to do is look at some of the other tutorials for reference. While they aren't 100% consistent, that should give you a feeling about what we typically do.

Thanks for the many suggestions and feedback! Applying the suggestions in multiple batches... Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Katherine Scott <katherineAScott@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Katherine Scott <katherineAScott@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

…whitespace Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

…up sentence Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

…isting tutorial Suggestion, or prefer to put this somewhere else? Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

Follow https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Code-Style-Language-Versions.html Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

Should cover all packages, not just the 'app' package which may not even exist in the user's workspace. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

clalancette

A few more things to fix up. This is getting closer to being merged.

clalancette · 2024-12-09T15:33:26Z

source/Tutorials/Intermediate/Testing/CLI.rst

+The above command suppresses the test output and exposes little about which tests succeed and which fail.
+Therefore while developing tests the ``--event-handlers`` option is useful to print all test output while the tests are running:
+
+.. code-block:: console
+
+  colcon test --event-handlers console_direct+
+


This particular change doesn't really have to do with this PR, so let's discuss this in a separate PR.

Sure, addressed in 620dbab.

source/Tutorials/Intermediate/Testing/CLI.rst

source/Tutorials/Intermediate/Testing/Integration.rst

… PR. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

clalancette

This looks great, thank you for iterating.

* tutorial/integration_testing: first draft Signed-off-by: Arne Baeyens <mail@arnebaeyens.com> Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Co-authored-by: Katherine Scott <katherineAScott@gmail.com> (cherry picked from commit 3d4b2ef)

* tutorial/integration_testing: first draft Signed-off-by: Arne Baeyens <mail@arnebaeyens.com> Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com> Co-authored-by: Katherine Scott <katherineAScott@gmail.com> (cherry picked from commit 3d4b2ef) Co-authored-by: abaeyens <abaeyens@users.noreply.github.com>

abaeyens · 2024-12-15T17:03:43Z

Thanks everyone for their efforts it getting this completed and merged, especially @clalancette! I'm glad I could contribute to the ROS documentation.

tutorial/integration_testing: first draft

3537d6a

Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

abaeyens requested review from clalancette and audrow as code owners December 1, 2024 10:37

abaeyens mentioned this pull request Dec 1, 2024

[Feedback desired] ros2/ros2_documentation tutorial on launch_testing ros2/launch#812

Closed

clalancette requested changes Dec 1, 2024

View reviewed changes

Ryanf55 reviewed Dec 2, 2024

View reviewed changes

kscottz approved these changes Dec 2, 2024

View reviewed changes

fujitatomoya reviewed Dec 3, 2024

View reviewed changes

source/Tutorials/Intermediate/Testing/Integration.rst Outdated Show resolved Hide resolved

abaeyens and others added 6 commits December 7, 2024 15:22

tutorial/integration_testing: apply suggestions from code review

0a01d38

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Katherine Scott <katherineAScott@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

tutorial/integration_testing: apply suggestions from code review

2ded82f

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

tutorial/integration_testing: apply suggestions from code review

d134ece

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Katherine Scott <katherineAScott@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

tutorial/integration_testing: fix turtlesim capitalization

b572725

Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

tutorial/integration_testing: remove "homework" section and trailing …

07e9415

…whitespace Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

abaeyens force-pushed the tutorial/integration_testing branch from 8bd4e48 to 07e9415 Compare December 7, 2024 14:43

Arne Baeyens added 4 commits December 7, 2024 17:24

tutorial/integration_testing: delete include launch file ref + split …

5f46b89

…up sentence Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

tutorial/integration_testing: move test running and examination to ex…

05f16a7

…isting tutorial Suggestion, or prefer to put this somewhere else? Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

tutorial/integration_testing: fix whitespace

13140f5

Follow https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Code-Style-Language-Versions.html Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

tutorial/integration_testing: link to launch_testing_ros package

c28db38

Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

abaeyens requested a review from clalancette December 8, 2024 14:03

tutorial/integration_testing: fix path

b9278e7

Should cover all packages, not just the 'app' package which may not even exist in the user's workspace. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

clalancette requested changes Dec 9, 2024

View reviewed changes

Arne Baeyens and others added 2 commits December 9, 2024 20:15

tutorial/integration_testing: Leave changes to CLI tutorial for other…

620dbab

… PR. Signed-off-by: Arne Baeyens <mail@arnebaeyens.com>

Update source/Tutorials/Intermediate/Testing/Integration.rst

2270b2e

Co-authored-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: abaeyens <abaeyens@users.noreply.github.com>

abaeyens requested a review from clalancette December 9, 2024 19:16

Minor formatting fixes.

29bc966

Signed-off-by: Chris Lalancette <clalancette@gmail.com>

clalancette approved these changes Dec 9, 2024

View reviewed changes

clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Dec 9, 2024

clalancette merged commit 3d4b2ef into ros2:rolling Dec 9, 2024
4 checks passed

mergify bot mentioned this pull request Dec 9, 2024

Add tutorial on integration testing (backport #4881) #4898

Merged

mergify bot mentioned this pull request Dec 9, 2024

Add tutorial on integration testing (backport #4881) #4899

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tutorial on integration testing #4881

Add tutorial on integration testing #4881

abaeyens commented Dec 1, 2024 •

edited

Loading

ros-discourse commented Dec 1, 2024

clalancette left a comment

clalancette Dec 1, 2024

abaeyens Dec 7, 2024

Ryanf55 left a comment

Ryanf55 Dec 2, 2024

abaeyens Dec 7, 2024

abaeyens Dec 8, 2024

Timple commented Dec 2, 2024

clalancette commented Dec 2, 2024

kscottz left a comment

kscottz Dec 2, 2024

kscottz Dec 2, 2024

abaeyens Dec 7, 2024

fujitatomoya left a comment

abaeyens commented Dec 7, 2024 •

edited

Loading

clalancette commented Dec 7, 2024

clalancette left a comment

clalancette Dec 9, 2024

abaeyens Dec 9, 2024

clalancette left a comment

abaeyens commented Dec 15, 2024

Add tutorial on integration testing #4881

Add tutorial on integration testing #4881

Conversation

abaeyens commented Dec 1, 2024 • edited Loading

ros-discourse commented Dec 1, 2024

clalancette left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryanf55 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Timple commented Dec 2, 2024

clalancette commented Dec 2, 2024

kscottz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fujitatomoya left a comment

Choose a reason for hiding this comment

abaeyens commented Dec 7, 2024 • edited Loading

clalancette commented Dec 7, 2024

clalancette left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clalancette left a comment

Choose a reason for hiding this comment

abaeyens commented Dec 15, 2024

abaeyens commented Dec 1, 2024 •

edited

Loading

abaeyens commented Dec 7, 2024 •

edited

Loading