Add system tests into CI

[CihatAltiparmak]

No description provided.

[CihatAltiparmak]

Hello @Yadunund ,

I've made some work to integrate system tests like test_rclcpp . But i have a bunch of question.

1. While running colcon test --packages-select test_rclcpp inside rclcpp_integration.test.py file, we have to set the current working directory of subprocess as workspace directory. Otherwise colcon cannot find the workspace automatically. I've solved by using launch arguments. I'm ready to hear some better ideas from you.
2. I couldn't add some packages like test_rclcpp, launch_ros to package.xml due to the fact that CI fails in command colcon list --paths-only --packages-up-to rmw_zenoh_cpp zenoh_c_vendor by throwing out [1.821s] ERROR:colcon:colcon list: Unable to order packages topologically: Do you have any idea?
3. This time CI takes ~30 minutes for rolling source build and tests. I am yet to be able to decrease CI time. Are there some alternatives like osrf/ros2::nightly you know?
4. Should we add tests for jazzy and iron?

I'm ready to get some review.

[ahcorde]

Hello @Yadunund ,

I've made some work to integrate system tests like test_rclcpp . But i have a bunch of question.

1. While running colcon test --packages-select test_rclcpp inside rclcpp_integration.test.py file, we have to set the current working directory of subprocess as workspace directory. Otherwise colcon cannot find the workspace automatically. I've solved by using launch arguments. I'm ready to hear some better ideas from you.
2. I couldn't add some packages like test_rclcpp, launch_ros to package.xml due to the fact that CI fails in command colcon list --paths-only --packages-up-to rmw_zenoh_cpp zenoh_c_vendor by throwing out [1.821s] ERROR:colcon:colcon list: Unable to order packages topologically: Do you have any idea?
3. This time CI takes ~30 minutes for rolling source build and tests. I am yet to be able to decrease CI time. Are there some alternatives like osrf/ros2::nightly you know?
4. Should we add tests for jazzy and iron?

I'm ready to get some review.

Related to question:

2. I assume this is happening when using debs, test_rclcpp should not be available in the workspace that's why you see this error.

3. Compiling up to test_rclcpp requires to compile many packages and the resources of the workers are not high. I think it's a resonable time. Using osrf/ros2:nighly will require to release a package to get the changes, while compiling from source it's just when we merge PRs. I think we should keep this behaviour. What do you think @Yadunund ?

4. Yes, we should do the same with jazzy and iron. We might include some behavioral changes and we should be able to detect them

[CihatAltiparmak]

Yeah you can ignore first qustion in #271 (comment) . More quetions:

1. Do you want to run test in only source builds, or you also want to run tests for binary builds ?
2. Which test packages do you wanna run (test_rclcpp, rcl, test_communication and so on)?
3. Which files should be uploaded via upload artifact?

[Yadunund]

Hi @CihatAltiparmak,

Sorry for the delay in my review.

Do you want to run test in only source builds, or you also want to run tests for binary builds ?

We need to build system_tests from source as the binaries are not available. Since we already have a job that builds Rolling from source, we can just run the tests in the same workflow.

I wonder if we should not run the source job on pull request. Instead we can have it run nightly. We can add a Rolling binary job to the on pull request workflow.

Which test packages do you wanna run (test_rclcpp, rcl, test_communication and so on)?

imo test_rclcpp and test_communication would be good ones to run (we should include the -DSKIP_MULTI_RMW_TESTS=On cmake arg to the build command) but there is a known issue there #275

Which files should be uploaded via upload artifact?

colcon test should generate a test artifact (.gtest.xml) which you can find in the logs.

[CihatAltiparmak]

Hi @CihatAltiparmak,

Sorry for the delay in my review.

Do you want to run test in only source builds, or you also want to run tests for binary builds ?

We need to build system_tests from source as the binaries are not available. Since we already have a job that builds Rolling from source, we can just run the tests in the same workflow.

I wonder if we should not run the source job on pull request. Instead we can have it run nightly. We can add a Rolling binary job to the on pull request workflow.

Which test packages do you wanna run (test_rclcpp, rcl, test_communication and so on)?

imo test_rclcpp and test_communication would be good ones to run (we should include the -DSKIP_MULTI_RMW_TESTS=On cmake arg to the build command) but there is a known issue there #275

Which files should be uploaded via upload artifact?

colcon test should generate a test artifact (.gtest.xml) which you can find in the logs.

Don't worry. At the moment, we can add only binary builds to matrix when github.event_name is not scheduled. I've also added other suggestions like -DSKIP_MULTI_RMW_TESTS=On . The selected_system_tests arg can be passed to rmw_zenoh_integration.test.py It's okay for review after i pass the env.RMW_MIDDLEWARE_TEST_PACKAGES to selected_system_tests (Additionaly we should test the source builds by changing this line https://github.com/CihatAltiparmak/rmw_zenoh/blob/8b8a9b5df269286c6b32aad9834ee9e067392bb9/.github/workflows/build.yaml#L22 before merging. It can be necessary to decrease the version of upload-artifact, i will look at it)

[CihatAltiparmak]

Hello @Yadunund , would you take a look at latest changes here? In latest version of this PR, we can run system test by giving argument to launch file and then load artifacts for each ros distro. Don't merge at the moment but it's ready.

https://github.com/CihatAltiparmak/rmw_zenoh/blob/c326da645f83f859e0b723a26bcca41ccbcf29b8/.github/workflows/build.yaml#L22

After testing source based builds, we can modify this contidion. I'm aware of this change and it's temporary.

[Yadunund]

@CihatAltiparmak the changes here looks great to me!

[CihatAltiparmak]

@CihatAltiparmak the changes here looks great to me!

Cool, i'm leaving to you which systems tests it will be run.

[ahcorde]

we should test: rcl rcl_action, rcl_lifecycle, rclcpp, rclcpp_action, rclcpp_lifecycle, test_communication, test_qos, test_rclcpp

@clalancette and @Yadunund any other package that we should include?

@CihatAltiparmak anyhow some tests at least in rcl should fail, do you know why tests are skipped ? I can see this message in the action Skipping tests

[clalancette]

@clalancette and @Yadunund any other package that we should include?

I'm honestly not sure whether we should do this at all.

The goal here is to integrate rmw_zenoh into the main ros2.repos. Once we do that, the nightly jobs on https://ci.ros2.org will automatically be building and testing rmw_zenoh overnight. Is it worth it to also have this in a GitHub action? I'm not sure.

[CihatAltiparmak]

@ahcorde Could you show which tests are skipped so that i can investigate this issue easier? Seems you already have merged PR that skips test_event related tests : ros2/rcl#1180 and i see that all of skipped tests are related to this PR. But i will also run rcl tests in my computer and i will give feedback asap. If you decide to add this CI, i will add your suggestions. You can also add it, allow maintainers to edit option is ON.

@clalancette

Once we do that, the nightly jobs on https://ci.ros2.org/ will automatically be building and testing rmw_zenoh overnight.

The default configuration of rmw_zenoh needs zenoh router to run separately. So i implemented rmw_zenoh_integration.test.py as well. to only run colcon test command doesn't work for the default configuration of rmw_zenoh. If you think this CI is not necessary, it can be deleted build.yaml file completely. I don't know how https://ci.ros2.org/ runs. Btw this changes are temporary as @Yadunund states here.

Now i come up with some additional idea: workflow_call. You add to your ros2.repos url and the branch you wanna test thanks to this option.