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.

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

Fail test if no scene can be created (gz-rendering6) #1090

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Dec 3, 2024

🦟 Bug fix

Summary

Replace current print by a FAIL in Gtest to make the test that can not create an scene to fail. This should help to avoid false positives in testing.

Note that is going to make the current build of ign-rendering6 to fail many tests.

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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.

Replace current print by a FAIL in Gtest to make the test
that can not create an scene to fail. This should help to
avoid false positives in testing.

Signed-off-by: Jose Luis Rivero <[email protected]>
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.

should we wrap the LoadOgre_TEST.cc and LoadOgre2_TEST.cc tests with HAVE_OGRE and HAVE_OGRE2?

Signed-off-by: Jose Luis Rivero <[email protected]>
@j-rivero
Copy link
Contributor Author

j-rivero commented Dec 4, 2024

should we wrap the LoadOgre_TEST.cc and LoadOgre2_TEST.cc tests with HAVE_OGRE and HAVE_OGRE2?

Ouch sorry I did not want to include them in this PR but create another one for them. Removed in 5254dd4

@azeey azeey self-requested a review December 9, 2024 19:49
@j-rivero
Copy link
Contributor Author

@azeey friendly ping to merge this as agreed on the last PMC meeting, thanks!

@iche033
Copy link
Contributor

iche033 commented Dec 13, 2024

removed required from gz_rendering-pr-win jobs for ign-rendering6 branch.

Merging.

@iche033 iche033 merged commit 5901faa into ign-rendering6 Dec 13, 2024
9 of 10 checks passed
@iche033 iche033 deleted the jrivero/fail_test_if_no_scene branch December 13, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants