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

[PN-68] add unit test for robot command goal complete #172

Merged

Conversation

abaker-bdai
Copy link
Contributor

This is the first of 3 PRs to refactor _robot_command_goal_complete so that is more readable and easier to maintain.

In this PR, we are adding a unit test to capture and verify the current logic of the function before refactoring it in order to prevent introducing any regressions.
Writing the unit test also caught a few bugs and missing checks for some statuses which will be address in the final PR.

2nd PR: #167

Copy link
Collaborator

@jbarry-bdai jbarry-bdai left a comment

Choose a reason for hiding this comment

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

Assuming CI passes

@abaker-bdai abaker-bdai merged commit 690fa3f into main Oct 25, 2023
@abaker-bdai abaker-bdai deleted the abaker/PN-68-add-unit-test-for-robot_command_goal_complete branch October 25, 2023 16:04
@jbarry-bdai
Copy link
Collaborator

@jiuguangw @baxelrod-bdai @amessing-bdai don't really know who to task this of but does the CI for this repo actually run the tests? i was just checking and i see it building but i'm not sure i see the test running...

@jiuguangw
Copy link
Collaborator

@jiuguangw @baxelrod-bdai @amessing-bdai don't really know who to task this of but does the CI for this repo actually run the tests? i was just checking and i see it building but i'm not sure i see the test running...

So I was the one who set up running colcon test for the bdai repo CI. Spot_ros2 CI does not run tests (because at the time there were no tests...)

https://github.com/bdaiinstitute/spot_ros2/actions/runs/6643074260/workflow?pr=167

marlow-fawn pushed a commit to marlow-fawn/spot_ros2 that referenced this pull request Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants