-
Couldn't load subscription status.
- Fork 0
Integrity unit tests framework #111
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
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
|
|
||
| GenericFindDependency( | ||
| TARGET albatross | ||
| SYSTEM_INCLUDES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these have been included in the customer scripts, I'm not in favor of doing this since this is our code and we should be linting, but I'm not going to attempt to open that can of worms right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little unclear what you mean here. We shouldn't be setting the options in our find scripts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@isaactorz We should only be using SYSTEM_INCLUDE for code that doesn't comply with our coding standards. For example, the optional library (see here) is something that we've imported into our code base from an external source and it might not comply with our strict coding guidelines. Swiftlets on the other hand is something that we internally developed (see here) and therefore must comply.
Internally this SYSTEM_INCLUDES flag makes sure that all header files public from these third party targets are included with -isystem when compiling with GCC and Clang rather than the standard -I. So for example, if we wanted to link optional, it would compile it with something like this:
gcc -o main main.cc ... -L third_party/optional -loptional -isystem /third_party/optional/include ...rather than the usual
gcc -p main main.cc ... -L third_party/optional -loptional -I /third_party/optional/include ...Any warnings from header files under -isystem would be ignored rather than cause an error during compilation. Likewise the clang-tidy would ignore any issues under -isystem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, thanks for clarifying.
| message(FATAL_ERROR "Both INTEGRATION_TEST and UNIT_TEST option were specified, you can only specify one") | ||
| endif() | ||
|
|
||
| if (NOT ${PROJECT_NAME}_BUILD_TESTS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was moved after the add_executable but before the creation of the do-* custom targets so that user can still build these targets manually but it won't be included into the do-all-tests target
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm but one question.
What's the purpose for having both an ENABLE_TESTS and a BUILD_TESTS option?
|
@isaactorz
From what I recall |








Background
In looking to introduce unit/integration unit testing into Orion Engine for the Bosch integrity work, I've stumbled up on a few issues in Orion and friends. Firstly there aren't any
find_packgescripts fororion-engine,orion_proto,sip, andalbatrosin this cmake repository, that isn't a problem since each repository has a custom script to include them in thecmake/directory rather than thecmake/common. The problem is that each of these scripts have not included the necessaryoption(${PROJECT_NAME}_ENABLE_${FEATURE} "" false)statements which is needed to control the inclusion of submodule test executable.This causes issues when running
do-all-testsinorion-engineas unit tests from bothsip(uses swift_add_tests) andstarling(ex: the inclusion of test-starling-output-analyzer isn't controlled bystarling_BUILD_TESTShere) are executed. So to fix this, this PR does two things:swift_add_testandswift_add_test_runnerfunctions so that even if the targets aren't protected by a${PROJECT}_BUILD_TESTSvariable (and variants), it won't be included into thedo-all-teststarget.Testing
Ran the following script in Orion and than tried to compile the submodules (orion-engine, sip, orion_proto) after I removed the local find_package scripts.
Also to verify that this won't have any impact on Starling, I created this PR.