Refactor rendering tests to run against multiple implementations #685

mjcarroll · 2022-07-26T15:31:00Z

🎉 New feature

Addresses #683

Summary

As we have increased our number of potential configurations, we needed a bit of a refactor to be able to test all of the configurations.

This changes the tests to no longer use gtest's parameterization, but rather use ctest's

All tests that are based on the CommonRenderingTest can be parameterized via environment variables that are injected in ctest.

For a platform like Ubuntu Jammy, where we support ogre, and ogre2 (with both opengl and vulkan). The resulting list of tests for a single executable looks like:

  Test #276: UNIT_WireBox_TEST_ogre_gl3plus
  Test #278: UNIT_WireBox_TEST_ogre2_gl3plus
  Test #280: UNIT_WireBox_TEST_ogre2_vulkan

For configurations that aren't supported, there are a few macros to be able to skip the tests introduced in the CommonRenderingTest.hh header.

Test it

Build gz-rendering and run tests with ctest

For example, you could:

Test all ogre2: ctest -R ogre2
Test all vulkan: ctest -R vulkan
Test all INTEGRATION: ctest -R integration

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
Consider updating Python bindings (if the library has them)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Michael Carroll <[email protected]>

codecov · 2022-07-27T18:01:24Z

Codecov Report

Merging #685 (5be5dad) into ci_matching_branch/ogre23 (7a6931c) will increase coverage by 16.50%.
The diff coverage is n/a.

@@                      Coverage Diff                       @@
##           ci_matching_branch/ogre23     #685       +/-   ##
==============================================================
+ Coverage                      53.18%   69.69%   +16.50%     
==============================================================
  Files                            214      214               
  Lines                          21550    21550               
==============================================================
+ Hits                           11462    15019     +3557     
+ Misses                         10088     6531     -3557

Impacted Files	Coverage Δ
src/WireBox.cc	`0.00% <0.00%> (-100.00%)`	⬇️
ogre2/src/Ogre2WireBox.cc	`0.00% <0.00%> (-85.08%)`	⬇️
include/gz/rendering/base/BaseJointVisual.hh	`0.00% <0.00%> (-81.97%)`	⬇️
src/RenderingIface.cc	`7.01% <0.00%> (-64.92%)`	⬇️
include/gz/rendering/base/BaseWireBox.hh	`0.00% <0.00%> (-62.50%)`	⬇️
ogre2/src/Ogre2JointVisual.cc	`0.00% <0.00%> (-50.00%)`	⬇️
include/gz/rendering/base/BaseArrowVisual.hh	`61.76% <0.00%> (-38.24%)`	⬇️
include/gz/rendering/base/BaseAxisVisual.hh	`58.62% <0.00%> (-36.21%)`	⬇️
src/RenderEngineManager.cc	`38.33% <0.00%> (-33.21%)`	⬇️
ogre2/src/Ogre2RenderTarget.cc	`77.23% <0.00%> (-7.75%)`	⬇️
... and 58 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a6931c...5be5dad. Read the comment docs.

Signed-off-by: Michael Carroll <[email protected]>

test/common_test/CommonRenderingTest.hh

test/gz_rendering_test.cmake

test/integration/gpu_rays.cc

Signed-off-by: Michael Carroll <[email protected]>

mjcarroll · 2022-07-28T17:32:25Z

It is expected that Windows won't work (as it is using vcpkg and not conda). I'm the process of updating some of the conda recipes, but I think that's out of scope here: conda-forge/ogre-next-feedstock#8

iche033

I tested ogre and ogre2 tests on Focal with gl3plus and the tests pass for me.

I see RENDER_ENGINE_VALUES still used in a few places, e.g. scene_factory.cc so we still can't remove it yet right?

I think the instruction on running individual tests may need to be updated in the installation tutorial

iche033 · 2022-07-28T23:46:18Z

test/integration/wide_angle_camera.cc

@@ -227,28 +198,13 @@ void WideAngleCameraTest::WideAngleCamera(

  // Clean up
  engine->DestroyScene(scene);
-  gz::rendering::unloadEngine(engine->Name());


can we leave these calls in place but commented out? We can add a todo comment that it says it causes segfault / test failure so we can look at them later.

These are removed because they are handled by the test fixture now:

https://github.com/gazebosim/gz-rendering/pull/685/files#diff-9b5026daaee2123f95b0ee0a29787297a9d106b248731492d1c15796940d0fa0R85-R89

This also guarantees that any resources allocated in the test will be deallocated before the test is torn down.

mjcarroll · 2022-07-29T00:16:30Z

Merging this into #681

As we have increased our number of potential configurations, we needed a bit of a refactor to be able to test all of the configurations. This changes the tests to no longer use gtest's parameterization, but rather use ctest's Signed-off-by: Michael Carroll <[email protected]>

* Upgrade to Ogre 2.3 * Refactor rendering tests to run against multiple implementations (#685) * Port remaining tests to common framework (#694) Signed-off-by: Matias N. Goldberg <[email protected]> Signed-off-by: Michael Carroll <[email protected]> Co-authored-by: Michael Carroll <[email protected]>

mjcarroll force-pushed the ci_matching_branch/ogre23 branch from 6a65c42 to 7a6931c Compare July 26, 2022 17:45

mjcarroll added 4 commits July 27, 2022 08:32

Simple example of a rendering common test

b8a8991

Signed-off-by: Michael Carroll <[email protected]>

Remove common test from src/

4575cee

Signed-off-by: Michael Carroll <[email protected]>

Move tests

11b24ed

Signed-off-by: Michael Carroll <[email protected]>

Complete test refactor

ff84cc7

Signed-off-by: Michael Carroll <[email protected]>

mjcarroll force-pushed the mjcarroll/rendering_common_test branch from 6ed7553 to ff84cc7 Compare July 27, 2022 16:27

mjcarroll added 4 commits July 27, 2022 11:46

Lint

0ea4026

Signed-off-by: Michael Carroll <[email protected]>

Add support for vulkan/metal testing

e7761ff

Signed-off-by: Michael Carroll <[email protected]>

Add pthread linkage for focal

17714f5

Signed-off-by: Michael Carroll <[email protected]>

APPEND rather than replace environment

92f3a95

Signed-off-by: Michael Carroll <[email protected]>

chapulina added the 🌱 garden Ignition Garden label Jul 27, 2022

mjcarroll added 6 commits July 27, 2022 15:24

Refactor common cmake logic

87e2988

Signed-off-by: Michael Carroll <[email protected]>

Remove vulkan and failing macOS tests

9de1878

Signed-off-by: Michael Carroll <[email protected]>

Fix small apple typo

b9a2fd7

Signed-off-by: Michael Carroll <[email protected]>

One more apple semicolon

b4328c9

Signed-off-by: Michael Carroll <[email protected]>

Test vulkan on jammy

4facc20

Signed-off-by: Michael Carroll <[email protected]>

Disable vulkan tests until github actions supports

75e62d6

Signed-off-by: Michael Carroll <[email protected]>

mjcarroll changed the title ~~Simple example of a rendering common test~~ Refactor rendering tests to run against multiple implementations Jul 28, 2022

mjcarroll marked this pull request as ready for review July 28, 2022 14:43

mjcarroll requested a review from iche033 as a code owner July 28, 2022 14:43

ahcorde requested changes Jul 28, 2022

View reviewed changes

Address reviewer feedback

5be5dad

Signed-off-by: Michael Carroll <[email protected]>

mjcarroll requested a review from ahcorde July 28, 2022 19:57

iche033 reviewed Jul 29, 2022

View reviewed changes

mjcarroll merged commit d87751e into ci_matching_branch/ogre23 Jul 29, 2022

mjcarroll deleted the mjcarroll/rendering_common_test branch July 29, 2022 00:16

iche033 mentioned this pull request Jul 29, 2022

Merge ign-rendering6 ➡️ main #679

Merged

mjcarroll mentioned this pull request Jul 29, 2022

Port remaining tests to common framework #694

Merged

iche033 mentioned this pull request Oct 6, 2023

Run tests for available render engines #683

Closed

iche033 mentioned this pull request Dec 3, 2024

Fix test_config.h to include real engines found (gz-rendering6) #1089

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor rendering tests to run against multiple implementations #685

Refactor rendering tests to run against multiple implementations #685

mjcarroll commented Jul 26, 2022 •

edited

Loading

codecov bot commented Jul 27, 2022 •

edited

Loading

mjcarroll commented Jul 28, 2022

iche033 left a comment •

edited

Loading

iche033 Jul 28, 2022

mjcarroll Jul 29, 2022

mjcarroll commented Jul 29, 2022

Refactor rendering tests to run against multiple implementations #685

Refactor rendering tests to run against multiple implementations #685

Conversation

mjcarroll commented Jul 26, 2022 • edited Loading

🎉 New feature

Summary

Test it

Checklist

codecov bot commented Jul 27, 2022 • edited Loading

Codecov Report

mjcarroll commented Jul 28, 2022

iche033 left a comment • edited Loading

Choose a reason for hiding this comment

iche033 Jul 28, 2022

Choose a reason for hiding this comment

mjcarroll Jul 29, 2022

Choose a reason for hiding this comment

mjcarroll commented Jul 29, 2022

mjcarroll commented Jul 26, 2022 •

edited

Loading

codecov bot commented Jul 27, 2022 •

edited

Loading

iche033 left a comment •

edited

Loading