Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Port remaining tests to common framework #694

Merged
merged 7 commits into from
Jul 29, 2022
Merged

Port remaining tests to common framework #694

merged 7 commits into from
Jul 29, 2022

Conversation

mjcarroll
Copy link
Contributor

Follow up to #685.

There were two remaining tests that didn't get ported over to utilize the common framework. These have been updated and we can remove the test_config.hh

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

@mjcarroll mjcarroll self-assigned this Jul 29, 2022
@mjcarroll mjcarroll requested a review from iche033 as a code owner July 29, 2022 01:04
@iche033
Copy link
Contributor

iche033 commented Jul 29, 2022

one more RENDER_ENGINE_VALUES here:
https://github.com/gazebosim/gz-rendering/blob/ign-rendering6/.github/ci/after_make.sh#L9

@iche033
Copy link
Contributor

iche033 commented Jul 29, 2022

I think something changed. The tests are failing: https://build.osrfoundation.org/job/ignition_rendering-ci-pr_any-ubuntu_auto-amd64/2580/

@mjcarroll
Copy link
Contributor Author

I think something changed. The tests are failing: https://build.osrfoundation.org/job/ignition_rendering-ci-pr_any-ubuntu_auto-amd64/2580/

Yeah, I messed something up, looking at it right now.

mjcarroll added 3 commits July 28, 2022 20:53
@mjcarroll
Fix faulty logic
6dd3b00 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
Remove environment from actions
57931b8 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
Work around clang sensitivity to deallocation
8d53b76 
Signed-off-by: Michael Carroll <[email protected]>
@codecov
Copy link

codecov bot commented Jul 29, 2022 •
edited
Loading

Codecov Report

Merging #694 (2817ebe) into ci_matching_branch/ogre23 (d87751e) will increase coverage by 0.57%.
The diff coverage is n/a.

@@                      Coverage Diff                      @@
##           ci_matching_branch/ogre23     #694      +/-   ##
=============================================================
+ Coverage                      69.69%   70.26%   +0.57%     
=============================================================
  Files                            214      214              
  Lines                          21550    21550              
=============================================================
+ Hits                           15019    15142     +123     
+ Misses                          6531     6408     -123
Impacted Files Coverage Δ
ogre/src/OgreRenderEngine.cc 74.33% <0.00%> (+0.58%) ⬆️
src/RenderEngineManager.cc 71.54% <0.00%> (+33.20%) ⬆️
src/RenderingIface.cc 71.92% <0.00%> (+64.91%) ⬆️

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 d87751e...2817ebe. Read the comment docs.

@mjcarroll
Copy link
Contributor Author

I have disabled the remaining 3 tests that fail on macOS metal and tracked them in #695

@mjcarroll mjcarroll merged commit 0225827 into ci_matching_branch/ogre23 Jul 29, 2022
@mjcarroll mjcarroll deleted the mjcarroll/rendering_common_test2 branch July 29, 2022 04:33
iche033
iche033 reviewed Jul 29, 2022
View reviewed changes
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

I was testing this with ogre and ogre2 on Focal and they pass locally. Changes look good to me.

mjcarroll added a commit that referenced this pull request Jul 29, 2022
@mjcarroll
Port remaining tests to common framework (#694)
c3b5daf 
* Port remaining tests to common framework

Signed-off-by: Michael Carroll <[email protected]>
mjcarroll added a commit that referenced this pull request Jul 29, 2022
@mjcarroll
Port remaining tests to common framework (#694)
37da046 
* Port remaining tests to common framework

Signed-off-by: Michael Carroll <[email protected]>
mjcarroll added a commit that referenced this pull request Jul 29, 2022
@darksylinc @mjcarroll
Port gz-rendering to Ogre 2.3 (#553)
dd95d0b 
* 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]>
@iche033 iche033 mentioned this pull request Oct 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iche033 iche033 iche033 left review comments

Assignees

@mjcarroll mjcarroll

Labels
None yet
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mjcarroll @iche033