Fix unittest check #14765
Merged
Fix unittest check #14765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
googletest now follows the "Abseil Live at HEAD" philosophy, which means they recommend using the latest commit on the master branch and always compiling from source. They recommend this to avoid version mismatch issues and "diamond dependency" problems which are common in dependency graphs with pinned versions. Google make the "promise" that future changes won't break downstream code if it follows the "Abseil compatability guidelines". Upping the version to master also fixes some CMake configure time warnings that were present with the older tagged releases of googletest: CMake Deprecation Warning at __build/_deps/googletest-src/CMakeLists.txt:4 (cmake_minimum_required): Compatibility with CMake < 2.8.12 will be removed from a future version of CMake. Update the VERSION argument <min> value or use a ...<max> suffix to tell CMake that the project does not need compatibility with older versions. CMake Deprecation Warning at __build/_deps/googletest-src/googlemock/CMakeLists.txt:45 (cmake_minimum_required): Compatibility with CMake < 2.8.12 will be removed from a future version of CMake. Update the VERSION argument <min> value or use a ...<max> suffix to tell CMake that the project does not need compatibility with older versions. CMake Deprecation Warning at __build/_deps/googletest-src/googletest/CMakeLists.txt:56 (cmake_minimum_required): Compatibility with CMake < 2.8.12 will be removed from a future version of CMake. Update the VERSION argument <min> value or use a ...<max> suffix to tell CMake that the project does not need compatibility with older versions.
Typically when adding a unit test directory to a CMake project a check will be used to ensure the subdirectory is added only if the following are true: * The BUILD_TESTING option is set to ON. * The current CMake project is the top-level project. The reason being, if a downstream project includes our project they generally don't want to build our unit tests. In mbed-os, we do correctly specify the above condition before adding the central UNITTEST subdirectory, which fetches googletest and adds the "stub" libraries the unit tests depend on. However, we only check if CMAKE_CROSSCOMPILING is OFF (or undefined) before actually adding the unit tests. This mismatched logic would lead to unexpected build failures in various scenarios. One likely case could be: a downstream project including mbed-os happens to set CMAKE_CROSSCOMPILING to OFF/undefined for any reason (possibly to build its own unit tests). mbed-os would go ahead and attempt to build its tests without fetching googletest or adding the required stub targets. To fix the issue replace the check for CMAKE_CROSSCOMPILING in the unit tests with the same BUILD_TESTING idiom we use for adding the central UNITTESTS subdirectory.
ciarmcom
added
the
release-type: patch
|
@rwalton-arm, thank you for your changes. |
0xc0170
approved these changes
Jun 11, 2021
|
CI started |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
mbedmain
added
release-version: 6.12.0
Release-pending
and removed
release-type: patch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Typically when adding a unit test directory to a CMake project a check
will be used to ensure the subdirectory is added only if the following
are true:
The reason being, if a downstream project includes our project they
generally don't want to build our unit tests.
In mbed-os, we do correctly specify the above condition before adding
the central UNITTEST subdirectory, which fetches googletest and adds the
"stub" libraries the unit tests depend on. However, we only check if
CMAKE_CROSSCOMPILING is OFF (or undefined) before actually adding the
unit tests. This mismatched logic would lead to unexpected build
failures in various scenarios. One likely case could be: a downstream
project including mbed-os happens to set CMAKE_CROSSCOMPILING to
OFF/undefined for any reason (possibly to build its own unit tests).
mbed-os would go ahead and attempt to build its tests without fetching
googletest or adding the required stub targets.
To fix the issue replace the check for CMAKE_CROSSCOMPILING in the unit
tests with the same BUILD_TESTING idiom we use for adding the central
UNITTESTS subdirectory.
While I'm here, fix some CMake warnings produced by googletest because we're
using an outdated version. googletest recommend using the latest commit from
master
Fixes #14764
Impact of changes
None
Migration actions required
None
Documentation
None
Pull request type
Test results
Reviewers