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

Added Unit Test for Republisher Node on Noetic Branch #39

Open
wants to merge 20 commits into
base: noetic-devel
Choose a base branch
from

Conversation

IrinaTerebiznik
Copy link
Contributor

@IrinaTerebiznik IrinaTerebiznik commented Dec 23, 2024

This repository contains a unit test for a ROS node that republishes messages from /input_topic to /output_topic. The test ensures the republisher node processes and republishes messages correctly.
Note that this test is non-automated and must be run manually, as explained in the README in the test/UnitTests folder.

@IrinaTerebiznik IrinaTerebiznik changed the title Add Unit Test for Republisher Node on Noetic branch Added Non-Automated Unit Test for Republisher Node on Noetic Branch Dec 23, 2024
@IrinaTerebiznik IrinaTerebiznik changed the title Added Non-Automated Unit Test for Republisher Node on Noetic Branch Added Unit Test for Republisher Node on Noetic Branch Dec 23, 2024
Copy link
Member

@leandropineda leandropineda left a comment

Choose a reason for hiding this comment

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

Reviewed! The test looks good, I added a few questions and change requests.

Also, please rename the UnitTests folder to use lowecase and underscores, as we adhere to PEP8 style guide: https://peps.python.org/pep-0008/#package-and-module-names

Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.

.gitignore Outdated Show resolved Hide resolved
inorbit_republisher/config/example.yaml Outdated Show resolved Hide resolved
inorbit_republisher/config/example.yaml Outdated Show resolved Hide resolved
inorbit_republisher/test/UnitTests/README.md Outdated Show resolved Hide resolved
inorbit_republisher/test/UnitTests/README.md Outdated Show resolved Hide resolved
inorbit_republisher/test/UnitTests/test_republisher.py Outdated Show resolved Hide resolved
inorbit_republisher/test/UnitTests/test_republisher.py Outdated Show resolved Hide resolved
inorbit_republisher/test/UnitTests/test_republisher.py Outdated Show resolved Hide resolved
Copy link
Member

@leandropineda leandropineda left a comment

Choose a reason for hiding this comment

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

It looks good! Approved after removing the usage of the internal method

Copy link
Member

@leandropineda leandropineda left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

2 participants