search

ros-infrastructure / ros_buildfarm

ROS buildfarm based on Docker
Apache License 2.0
83 stars 97 forks source link

Add test_require_gpu to ci jobs #1059

Open j-rivero opened 5 months ago

j-rivero commented 5 months ago

Thew PR implements the test_require_gpu parameter for the CI kind of jobs in the buildfarm.

The first commit 4e0466e7c09eeb4c5a514047059f123198327b13 adds the necessary code to support the test_require_gpu parameter in the ci build files and the docker run necessary arguments to the ci_job template when running "build and test".

The second commit d6a39c3612b3d6617fae0b8a728f1dd3cb09aa62 fixes current code existing in devel jobs used by the ci jobs.

Tested: the code was used for testing gz-rendering OpenGL tests in the testing buildfarm of the Gazebo team. https://citest.build.osrfoundation.org/view/Rci/job/Rci__nightly-release_ubuntu_noble_amd64/68/testReport/projectroot.test/integration/

P.D: there is a need to audit the existing gpu code in the devel jobs that I plan to do in a different PR.

cottsay commented 4 months ago

It looks like this rolls back some of #624, which was merged over 4 years ago. Should we tick-tock some of the public-facing changes? In particular, the script argument changes would cause a regression if someone was already using them.

nuclearsandwich commented 4 months ago

It looks like this rolls back some of #624, which was merged over 4 years ago. Should we tick-tock some of the public-facing changes? In particular, the script argument changes would cause a regression if someone was already using them.

I agree. It's profoundly unlikely that we have users on the previous setup but even if they never got it working I think emitting deprecation warnings rather than rendering configs unparseable is good practice.

nuclearsandwich commented 4 months ago

Although we don't actually do releases of ros_buildfarm too frequently ATM so I'm not sure what the criteria for tocking out would be.

j-rivero commented 4 months ago

It looks like this rolls back some of #624, which was merged over 4 years ago. Should we tick-tock some of the public-facing changes? In particular, the script argument changes would cause a regression if someone was already using them.

I agree. It's profoundly unlikely that we have users on the previous setup but even if they never got it working I think emitting deprecation warnings rather than rendering configs unparseable is good practice.

Understood. Parameters are back in b367013 with a deprecation warning.