search

open-rmf / rmf

Root repository for the RMF software
Apache License 2.0
233 stars 60 forks source link

Feature/integration test #55

Open youliangtan opened 3 years ago

youliangtan commented 3 years ago

This introduces a CI with an e2e rmf integration test (which can also run nightly).

The Integration test, tests/integration_test.py will launch 3 rmf scenarios (office, clinic, airport), and dispatch of Loop, Delivery, and Clean tasks. The script will make sure all tasks are completed successfully before the specified timeout, else raise an error.

To try the script locally: 

source rmf_ws/install/setup.bash
python3 test/integration_test.py
echo $?  ##check if run successfully

On some rare cases, the tasks will fail due to some stray reasons (coke drops during delivery, caddy not launch successfully, robot stuck at a corridor as path is too narrow...). All these will be addressed separately.

Feedbacks are welcome to identify the best method to implement this

cnboonhan commented 3 years ago

hihi! Sorry It took so long to get back to you. I have actually been working on a similar project to do testing, and hopefully we can work out how to combine the efforts. Or perhaps we can have both running as well.

This PR seems to be a superset of what this PR does and we could combine the two: https://github.com/open-rmf/rmf/pull/53, but we should check in with @ddengster ? I might be misunderstanding something.

ddengster commented 3 years ago

hihi! Sorry It took so long to get back to you. I have actually been working on a similar project to do testing, and hopefully we can work out how to combine the efforts. Or perhaps we can have both running as well.

This PR seems to be a superset of what this PR does and we could combine the two: #53, but we should check in with @ddengster ? I might be misunderstanding something.

PR #53 builds packages from the source repos and does the unit tests for each package. It seems that this PR uses a docker image, would that have any difference? Would be happy to scrap it if there isnt

cnboonhan commented 3 years ago

think the docker image comes from here: https://github.com/open-rmf/rmf_demos/blob/main/.github/workflows/build.yaml which is rebuilt every time a push to main is done. :thinking:

youliangtan commented 3 years ago

I think both build approaches provide different values. https://github.com/open-rmf/rmf/pull/53 is a cleaner approach as it uses git ros-tooling/action-ros-ci. In here, the current docker way will create an img copy here, with a tag of latest or test. This is helpful for users to rerun the same test locally, and support multi-stage build in the future.

This integration test focuses on running rmf_demos scenarios. In my mind, I am thinking that the rmf_gym project can be another demonstration extension of rmf_demos, both integration tests can coexist.

cnboonhan commented 3 years ago

@ddengster any thoughts? If its ok, we could merge this PR in and scrap the ros-tooling setup ( at expense to your efforts :bow: ) i

ddengster commented 3 years ago

@ddengster any thoughts? If its ok, we could merge this PR in and scrap the ros-tooling setup ( at expense to your efforts bow ) i

My other concern is whether we should put this integration test elsewhere; the intention of this repo seems to be a link to other rmf repos. I wonder if a dedicated org-wide ci repo is worth setting up just for that. Thoughts @mxgrey? Otherwise we can merge this.

As for #53 it's blocked by other issues. I'll attend to it later.

mxgrey commented 3 years ago

I think having all the different tests is a good thing, since they're testing different processes.

53 is important for making sure the entire stack of main branches across all the repos stay in sync. That's important for this many-repo project with so many moving parts.

This PR is good for making sure that demos work and that new users have a predictable experience with the demos.

That being said, since this PR depends on build artifacts from the rmf_demos repo, is there a reason we don't put this integration test into rmf_demos instead?

mxgrey commented 3 years ago

I wonder if a dedicated org-wide ci repo is worth setting up just for that.

I'd be open to this too. I guess it's a question of how we want to expose the integration tests to casual users. Should the integration tests be plainly available from this rmf landing page repo, or would that be an unnecessary distraction for casual users?

cnboonhan commented 3 years ago

from a visibility point of view, I think a dedicated ci repo would be helpful :+1:

cnboonhan commented 3 years ago

Well, I think we are going in the direction of using both tests, so since the other one is pending I'll go ahead and approve this for merging. Sorry it took so long!