Skip to content

Conversation

@130s
Copy link
Contributor

@130s 130s commented May 26, 2016

May replace #224

I'm not sure if this suits the need in this repo, but industrial_ci has been used by dozens of ROS repositories and recently we added Kinetic support. Since Xenial runs on docker image we don't need to wait until Travis CI supports 16.04.

@130s 130s force-pushed the kinetic/travis_docker branch from 724ee2d to ae65a0d Compare May 26, 2016 22:41
@bmagyar
Copy link
Member

bmagyar commented May 26, 2016

Cool, thanks!
What do you guys think, @efernandez @davetcoleman ?

@130s 130s force-pushed the kinetic/travis_docker branch from ae65a0d to 94f080d Compare May 27, 2016 18:41
.travis.yml Outdated
- gcc
notifications:
email:
on_success: always
Copy link
Contributor

@efernandez efernandez May 30, 2016

Choose a reason for hiding this comment

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

This should use change instead of always, as it was before

@efernandez
Copy link
Contributor

I've re-run the jobs. It seems they're working fine, but just one test failed.

BTW, is this running only kinetic, or also indigo and jade everytime? I see up to 7 (sub)jobs.

I've commented which configuration thing is different. Curiously, I haven't received any email. Let's see after this run.

@efernandez
Copy link
Contributor

The tests failed again. They're some tests that depend somehow on the timing, but the solution for now has been to run the tests sequentially, not in parallel.

I think they are running in parallel because these is passed to the run_tests target: -p4 --make-args -j8.
Could you change the configuration to use -j1 (and -p1 if needed) instead, please?

@130s
Copy link
Contributor Author

130s commented May 31, 2016

I think they are running in parallel because these is passed to the run_tests target: -p4 --make-args -j8.
Could you change the configuration to use -j1 (and -p1 if needed) instead, please?

I think I added those options. Now only Prerelease Test jobs (Indigo, Jade) passed.

Others seem to be failing at [diff_drive_controller.rosunit-diff_drive_limits_test/testAngularJerkLimits] (eg). Any idea?

@efernandez
Copy link
Contributor

That test seems to be highly dependant on timing. I have unittest for that and I should probably replace the rostest with them, but I haven't sent any PR yet because I'd like to know why the rostest fails.

Anyway, this PR now LGTM. I've just re-run the job to see if now everything passes.

BTW, is there a way to only run the kinetic job? I mean, when a PR goes into ros-controls kinetic-devel it should only test kinetic, not all the previous releases. Indeed, those are already tested with Travis CI w/o Docker.

@130s
Copy link
Contributor Author

130s commented May 31, 2016

BTW, is there a way to only run the kinetic job? I mean, when a PR goes into ros-controls kinetic-devel it should only test kinetic, not all the previous releases. Indeed, those are already tested with Travis CI w/o Docker.

Yes, I've now removed jobs for earlier ROS distros.


I just want to be clear though; I thought running non-required jobs (i.e. allow_failure) for the branches that are not the main target ({ indigo/jade }-devel in this case) is nice for conveniently checking the validity of the PR.

But those jobs are already failing. I haven't gone into detail but with a random look at an indigo build:

[joint_trajectory_controller:make] /home/travis/ros/ws_ros_controllers/src/ros_controllers/joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller_impl.h:447:22: error: ‘class realtime_tools::RealtimeServerGoalHandle<control_msgs::FollowJointTrajectoryAction_<std::allocator > >’ has no member named ‘preallocated_feedback_’

There might have been API change so different distros don't build against each other? If so running jobs for indigo and jade no longer makes sense indeed.

@efernandez
Copy link
Contributor

Yes, kinetic-devel breaks the API wrt to previous, or probably in jade-devel. Thanks for removing them.

- cd ~/ros/ws_ros_controls/src
- wstool init .
# Download non-debian stuff
- wstool merge https://raw.github.com/ros-controls/ros_control/$ROS_DISTRO-devel/ros_control.rosinstall
Copy link
Contributor

Choose a reason for hiding this comment

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

The source overlay is not included in the new config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. I have to get ros-industrial/industrial_ci#47 move forward before I can enable source overlay.

@mathias-luedtke
Copy link
Contributor

mathias-luedtke commented Jun 1, 2016

That test seems to be highly dependant on timing. I have unittest for that and I should probably replace the rostest with them, but I haven't sent any PR yet because I'd like to know why the rostest fails.

If the *.test are run in parallel they communicate via the same topics..
I guess it would help to add a top-level group with a unique namespace to each *.test.

(I believe rostest uses dedicated ROS masters..)

@130s 130s force-pushed the kinetic/travis_docker branch from 1ca90b5 to cc5de33 Compare June 1, 2016 23:25
.travis.yml Outdated
on_success: change
on_failure: change
recipients:
- [email protected]
Copy link
Member

Choose a reason for hiding this comment

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

I don't need to be on these emails if anyone else wants to watch them...

@davetcoleman
Copy link
Member

+1 to switching to the ros-industrial CI, not clear to me why checks are failing though

@130s 130s force-pushed the kinetic/travis_docker branch from cc5de33 to 6b94b67 Compare June 6, 2016 01:52
[Travis CI] Parallel config.
[Travis CI] Run jobs for only Kinetic.
[Travis CI] Add a job for workspace overlay. Utilize common variables instead of defining them for each job.
@130s 130s force-pushed the kinetic/travis_docker branch from 6b94b67 to df02474 Compare June 6, 2016 01:54
@130s
Copy link
Contributor Author

130s commented Jun 6, 2016

I think comments are addressed. Waiting for the Travis jobs to finish.

@efernandez
Copy link
Contributor

For me it seems this can be merged. There's a rostest from diff_drive_controller failing, which has known issues: essentially the results are affected by the time it takes to run the test.

@bmagyar
Copy link
Member

bmagyar commented Jun 16, 2016

LGTM 👍 Merging...

@bmagyar bmagyar merged commit 6817ab4 into ros-controls:kinetic-devel Jun 16, 2016
@130s 130s deleted the kinetic/travis_docker branch June 16, 2016 17:04
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.

5 participants