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

New macros to help with filter google-test in some platforms #102

Merged
merged 2 commits into from
Aug 7, 2020

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Jul 9, 2020

Across the ignition software we have many cases where tests are not prepared to work on Windows, Mac or both. This PR implements a header providing two macros to be used instead of TEST macro from google test:

  • IGN_UTILS_TEST_DISABLED_ON_WIN32 creates a google test that will not be run on Windows (although it will be compiled).
  • IGN_UTILS_TEST_ENABLE_ONLY_LINUX creates a google test that will only be run on Linux (although it will be compiled on all platforms).

Testing: using this PR together with ign-gui in: Build Status
(needs more test filtering but already disabled most of the failures).

@j-rivero j-rivero requested a review from mxgrey as a code owner July 9, 2020 22:44
@github-actions github-actions bot added 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Jul 9, 2020
@j-rivero j-rivero force-pushed the add_test_macros_ign_cmake2 branch from b0bd4c7 to a432891 Compare July 10, 2020 10:21
@chapulina
Copy link
Contributor

The ABI checker failed, I think ign-cmake may need some special care since it doesn't have "C++ code"

Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM! Tested by manually defining __APPLE__ and _WIN32 before including the header.

include/ignition/utilities/ExtraTestMacros.hh Outdated Show resolved Hide resolved
The new file provides macros that help with filtering googletest
on some of the platforms.

Signed-off-by: Jose Luis Rivero <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina force-pushed the add_test_macros_ign_cmake2 branch from 75a07e1 to 4255cdd Compare August 3, 2020 02:19
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM, I'd just like to see a successful round of CI for gazebosim/gz-gui#76 before merging this, but that will be tough without an ign-cmake2 release.

@j-rivero
Copy link
Contributor Author

j-rivero commented Aug 7, 2020

This run on Windows check these changes together with ign-gui support: Build Status Seems to do what it is expected to do, no test failures.

@j-rivero j-rivero merged commit 7419c27 into ign-cmake2 Aug 7, 2020
@j-rivero j-rivero deleted the add_test_macros_ign_cmake2 branch August 7, 2020 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants