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

Fix skip logic for integration tests #264

Merged

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Fixes build failure reported in #263 when dependencies for av component are not available.

Summary

Since gazebosim/gz-cmake#165, we need to check both the SKIP_av and INTERNAL_SKIP_av before building integration tests that require the av component. Otherwise the integration tests fail to build, as reported in #263.

/build/source/test/integration/video_encoder.cc:21:10: fatal error: ignition/common/VideoEncoder.hh: No such file or directory
   21 | #include "ignition/common/VideoEncoder.hh"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

Check both SKIP_av and INTERNAL_SKIP_av before building
integration tests that require av component.
Reported in gazebosim#263.

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters requested a review from mjcarroll as a code owner October 28, 2021 19:40
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels Oct 28, 2021
@codecov
Copy link

codecov bot commented Oct 28, 2021

Codecov Report

Merging #264 (e58446d) into ign-common3 (df11d6e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-common3     #264   +/-   ##
============================================
  Coverage        76.55%   76.55%           
============================================
  Files               73       73           
  Lines            10386    10386           
============================================
  Hits              7951     7951           
  Misses            2435     2435           

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 df11d6e...e58446d. Read the comment docs.

@mjcarroll mjcarroll merged commit 46ef74c into gazebosim:ign-common3 Oct 28, 2021
@scpeters scpeters deleted the fix_skip_integration_tests branch October 29, 2021 01:11
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-01-24-citadel-edifice-fortress/1241/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel 🔮 dome Ignition Dome Gazebo 1️1️ Dependency of Gazebo classic version 11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants