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 automated testing (via catkin_virtualenv) #136

Merged

Conversation

ggaessler
Copy link
Contributor

Since the ROS buildfarm currently doesn't handle PIP test dependencies correctly (ros-infrastructure/ros_buildfarm#895) and there are currently no resources to fix it (neither on OSRF nor on Bosch side) I've changed the PR (#130) to use catkin_virtualenv for the PIP testing dependencies. I preferred the other way, but this way it is still ensured that no PIP dependencies end up in the package.


Porting the first feature from the boschresearch fork back to upstream: testing

Automatically test the code on every push or PR by playing back data to the driver and test whether the expected amount of messages is outputted by the driver and the messages contain the expected data.

The tester can cover more functionality that the current version of the nmea_navsat_driver supports, for these features new PRs will be created. The full usage can be observed in the boschresearch fork.

Warning: Github Actions are not very reliable in combination with ROS right now. The tests sometimes randomly get stale and run into the timeout. In these cases the tests should be re-run manually. Since the test is expected to take less than 5 minutes I've set to timeout to 10 minutes, otherwise it would take the full 6 hours, which is the Github Actions default setting.

Signed-off-by: Gabriel Gaessler <[email protected]>
Signed-off-by: Gabriel Gaessler <[email protected]>
@evenator
Copy link
Collaborator

I'm hoping to finally get a chance to take a look at this PR this week.

@evenator evenator self-assigned this Oct 26, 2021
@ggaessler
Copy link
Contributor Author

Are there any updates?

@evenator
Copy link
Collaborator

evenator commented Jan 2, 2022

Hey, sorry I took so long to review this. I think it looks good to merge. Thanks for the contribution!

@evenator evenator merged commit 9413fe0 into ros-drivers:master Jan 2, 2022
@ggaessler
Copy link
Contributor Author

Unfortunately the automated testing is not working right now since ros-tooling is broken since some days and therefore can't setup a testing environment: ros-tooling/setup-ros#460
The proposed workaround unfortunately can't be easily integrated into the ci.yaml. Hopefully the bug will be fixed soon.

@ggaessler
Copy link
Contributor Author

@evenator please re-run the action via the three dots at the top-right of https://github.com/ros-drivers/nmea_navsat_driver/actions/runs/1646715390. ros-tooling should be fixed in the meantime. Thanks!

@evenator
Copy link
Collaborator

@ggaessler Apparently one can only re-run a workflow for 30 days, so I can't re-run that.

@ggaessler
Copy link
Contributor Author

@ggaessler Apparently one can only re-run a workflow for 30 days, so I can't re-run that.

Ok. Thanks for trying. Create a new PR to "re-run" it: #144
Unfurtunately the Github runner was way slower than it used to be and just idled the first 9 minutes, causing the timeout to trigger after 1 more minute. This is fixed here:
#145

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants