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

Simplify tests #849

Merged
merged 6 commits into from
Oct 30, 2023
Merged

Conversation

RobertWilbrandt
Copy link
Collaborator

Our runtime tests have accumulated quite a bit of duplicated code for starting the driver and interfacing with ROS interfaces. Before adding more tests (MoveIt, mock_hardware, ...) this should get refactored and cleaned up.

@RobertWilbrandt RobertWilbrandt marked this pull request as ready for review October 19, 2023 08:54
@fmauch
Copy link
Collaborator

fmauch commented Oct 19, 2023

Maybe this would be a good point to add the URSim log to the artifacts similar to UniversalRobots/Universal_Robots_Client_Library@9b695e3

@RobertWilbrandt RobertWilbrandt force-pushed the simplify_unit_tests branch 2 times, most recently from ba3dc60 to 4e72386 Compare October 23, 2023 07:32
@RobertWilbrandt
Copy link
Collaborator Author

That is not transferrable 100% - Because we start ursim from the test launch description the container is already dead when we want to copy out the logs. That is certainly solvable, but somewhat unrelated to restructuring the code. I'd prefer merging this now and adding that feature later on.

Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

This does look mostly fine except for the code duplication mentioned.

This stands in conflict with #785. But I suggest, we first get this refactoring merged and then update #785 accordingly.

ur_robot_driver/test/robot_driver.py Show resolved Hide resolved
ur_robot_driver/test/robot_driver.py Show resolved Hide resolved
Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

Thank you @RobertWilbrandt This will make future test extensions much easier.

@fmauch fmauch merged commit b28a870 into UniversalRobots:main Oct 30, 2023
10 of 11 checks passed
mergify bot pushed a commit that referenced this pull request Oct 30, 2023
* Remove duplication of launch description in tests
* Move launch and interfacing boilerplate to common file
* Move all test logs to python logging
This makes it easier to differentiate between ROS and test framework
messages
* Move waiting for a controller to test_common
* Move robot starting to dashboard interface
* Remove unused request definition

(cherry picked from commit b28a870)

# Conflicts:
#	ur_robot_driver/test/dashboard_client.py
#	ur_robot_driver/test/robot_driver.py
mergify bot pushed a commit that referenced this pull request Oct 30, 2023
* Remove duplication of launch description in tests
* Move launch and interfacing boilerplate to common file
* Move all test logs to python logging
This makes it easier to differentiate between ROS and test framework
messages
* Move waiting for a controller to test_common
* Move robot starting to dashboard interface
* Remove unused request definition

(cherry picked from commit b28a870)

# Conflicts:
#	ur_robot_driver/test/dashboard_client.py
#	ur_robot_driver/test/robot_driver.py
#	ur_robot_driver/test/urscript_interface.py
fmauch pushed a commit that referenced this pull request Apr 17, 2024
* Remove duplication of launch description in tests
* Move launch and interfacing boilerplate to common file
* Move all test logs to python logging
This makes it easier to differentiate between ROS and test framework
messages
* Move waiting for a controller to test_common
* Move robot starting to dashboard interface
* Remove unused request definition

(cherry picked from commit b28a870)
fmauch pushed a commit that referenced this pull request Apr 17, 2024
* Remove duplication of launch description in tests
* Move launch and interfacing boilerplate to common file
* Move all test logs to python logging
This makes it easier to differentiate between ROS and test framework
messages
* Move waiting for a controller to test_common
* Move robot starting to dashboard interface
* Remove unused request definition

(cherry picked from commit b28a870)
fmauch pushed a commit that referenced this pull request Apr 17, 2024
* Remove duplication of launch description in tests
* Move launch and interfacing boilerplate to common file
* Move all test logs to python logging
This makes it easier to differentiate between ROS and test framework
messages
* Move waiting for a controller to test_common
* Move robot starting to dashboard interface
* Remove unused request definition

(cherry picked from commit b28a870)

Co-authored-by: RobertWilbrandt <[email protected]>
fmauch added a commit that referenced this pull request Apr 17, 2024
* Simplify tests (#849)

* Remove duplication of launch description in tests
* Move launch and interfacing boilerplate to common file
* Move all test logs to python logging
This makes it easier to differentiate between ROS and test framework
messages
* Move waiting for a controller to test_common
* Move robot starting to dashboard interface
* Remove unused request definition

(cherry picked from commit b28a870)

* Add UR30

---------

Co-authored-by: RobertWilbrandt <[email protected]>
Co-authored-by: Felix Exner <[email protected]>
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