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.

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

Add instructions for debugging C++ tests in GDB when using colcon #3899

Merged
merged 5 commits into from
Sep 5, 2023

Conversation

Ryanf55
Copy link
Contributor

@Ryanf55 Ryanf55 commented Sep 2, 2023

This stack overflow wasn't answered, and I ran into the same issue. Here is my approach to debugging failed C++ tests with gdb.
https://stackoverflow.com/questions/76589443/run-colcon-test-with-gdb-backtrack

Since colcon is lacking any references on using gdb, we can add information to the ROS 2 tutorials.

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@Ryanf55 Ryanf55 changed the title Add instructions for debugging tests in GDB Add instructions for debugging C++ tests in GDB when using colcon Sep 2, 2023
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@Ryanf55 thanks for creating PR.

i think it will be helpful for user to add this documentation. in fact, i would like to hear from @SteveMacenski about this doc enhancement.

since Nav2 doc has good explanation about this https://navigation.ros.org/tutorials/docs/get_backtrace.html, how about the following approach?

  • move the ROS 2 general explanation for backtrace from Nav2 to ROS 2 tutorial. (ROS 2 general part should be explained in ROS 2 documentation)
  • Nav2 specific doc will be staying where it is now. and it can also has a link to ROS 2 backtrace tutorial if needed.

what do you think?

CC: @clalancette

@SteveMacenski
Copy link
Contributor

I think its a simple additional example to show for debugging. I don't think this has anything in particular to do with Nav2, Ryan is just showing the example as a nav2_utils test, that could be replaced with any arbitrary thing.

I'd just merge this without further discussion. Its a simple enough clarification / additional steps that can be helpful for those running and then fixing crashing tests. No need to overthink it

@Ryanf55
Copy link
Contributor Author

Ryanf55 commented Sep 4, 2023

I think its a simple additional example to show for debugging. I don't think this has anything in particular to do with Nav2, Ryan is just showing the example as a nav2_utils test, that could be replaced with any arbitrary thing.

I'd just merge this without further discussion. Its a simple enough clarification / additional steps that can be helpful for those running and then fixing crashing tests. No need to overthink it

This was my intent. While I agree @fujitatomoya on relocating some NAV2's (well-written) documentation on testing to here, it's out of the scope of this PR. This one just introduces the simple concept of using gdb to debug tests. It's not covered anywhere in the ecosystem, is pretty simple to do, and is likely not apparent to users as a potential way to fix failing tests.

@fujitatomoya
Copy link
Collaborator

If Nav2 keeps that ROS 2 generic gdb explanation, which is totally fine for me. Lets forget about the relocation.

I would use simpler example with ROS 2 tutorial but Nav2 example here.

@manumerous
Copy link

Thanks you Ryan for answering my question and for putting in the effort of documenting the solution!

@Ryanf55
Copy link
Contributor Author

Ryanf55 commented Sep 5, 2023

If Nav2 keeps that ROS 2 generic gdb explanation, which is totally fine for me. Lets forget about the relocation.

I would use simpler example with ROS 2 tutorial but Nav2 example here.

If I use the tutorials, it would look like this:
gdb -ex run ./build/my_package/test/my_package_tutorial_test

Do you prefer that?

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Overall, I like the change. I've left a few suggestions for improvement.

source/Tutorials/Intermediate/Testing/CLI.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Testing/CLI.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Testing/CLI.rst Outdated Show resolved Hide resolved
Ryanf55 and others added 3 commits September 5, 2023 09:48
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Copy link
Contributor

Thanks for all of the iterations! @fujitatomoya does this look good to you now?

@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Sep 5, 2023
@fujitatomoya fujitatomoya merged commit e3ff07e into ros2:rolling Sep 5, 2023
3 checks passed
mergify bot pushed a commit that referenced this pull request Sep 5, 2023
)

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit e3ff07e)
mergify bot pushed a commit that referenced this pull request Sep 5, 2023
)

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit e3ff07e)
@Ryanf55 Ryanf55 deleted the ctest-gdb branch September 5, 2023 18:36
fujitatomoya pushed a commit that referenced this pull request Sep 5, 2023
) (#3904)

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit e3ff07e)

Co-authored-by: Ryan <ryanfriedman5410+github@gmail.com>
fujitatomoya pushed a commit that referenced this pull request Sep 5, 2023
) (#3903)

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit e3ff07e)

Co-authored-by: Ryan <ryanfriedman5410+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants