GTest functional tests that include parameters and uORB messaging

[jkflying]

Describe problem solved by the proposed pull request
Right now our unit tests only work for the limited subset of code that doesn't depend on any parameters or uORB messages. This adds an example of building tests that depend on these.

Test data / coverage
Just some simple tests so far. Writing and reading parameters, sending and receiving uORB messages. The more complex test gets data into the CollisionPrevention library via parameters and uORB messages, which then constrains a setpoint. This is more proof of concept than trying to get good test coverage of the library.

Additional context
Having to list all of the dependencies again to get the stubs to be last is a pain. I'd love for there to be better tooling for that somehow.

This should enable full-coverage testing of the uORB and parameter API which most libraries use.

[hamishwillee]

@jkflying Given you're in the guts of this, could you check that https://dev.px4.io/master/en/test_and_ci/unit_tests.html is accurate and insert docs that show the essential things you had to do to get the uORB/param stuff working? (I am fairly sure this is not up to date, because it does not even mention any limitation w.r.t. uorb/params).

What I'd really like is a template test that might live against this template application module example created by @bkueng. Thoughts?

[jkflying]

@hamishwillee there have been some improvements by @MaEtUgR since that was written, with tests now being able to be added in the library folders and using the more industry-standard GTest framework.

There are also definitely some issues in the documentation there, eg make px4_sitl_shell doesn't actually exist anymore.

Running make tests runs the tests listed there, but for some reason they each take (on my system) 1.3s to run with RelWithDebInfo builds, and Debug builds they just hang. The GTest based tests are basically instant (a few ms maximum). I guess those tests need to set up the entire SITL environment? Seems like a big pain for running local tests, and it prevents us from having better test coverage too.

Also those tests are very fine-grained. The tests this PR enables will make sure that entire systems are working correctly together, that they read their uORB messages and parameters correctly, and we can even check that the messages published by one library can be read by another library.

Right now this is still in draft. I will update docs when it is complete 👍

[julianoes]

I guess you know my opinion. I don't think the testing should be at this layer. To me it looks (and probably is) too complex. I would prefer the testing to be on a lower level around the actual "business logic" before stuff is put in a module.

[jkflying]

I agree there are a lot of dependencies that get brought in, and it would be nice to have more self-contained tests, but it seems to me that the actual API used for most things is parameters and uORB messages. Those are what ultimately define the behavior.

So, if we really want to do functional testing in a nice easy-to-read style of

setup input
run the code
verify output

which is generally how business logic tests should work, we need to be doing this at the uORB and parameter layer. Otherwise we need to wrap everything with an extra layer just to test it, and there is a big swathe of code (the translation from the uORB API to the C/C++ API) that doesn't get tested, which is very prone to bugs due to eg. different naming in messages and C/C++ API.

TLDR: I also think this is messy, but it's the only way I see to not have gaps in our testing with a pub-sub architecture.

[hamishwillee]

Right now this is still in draft. I will update docs when it is complete

Thank you (re Re #12521 (comment))

If at all possible, please do consider adding a test module alongside Beat's example module. We're hoping more people take that and copy it as "best practise" so having good test alongside it makes sense. If not, then perhaps minimal would still be to link to the tests you think work well.

[julianoes]

The problem with this testing is that it's harder to run on NuttX, and it conflicts with ntest which would be very minimal and simple/stupid.
#12106

[jkflying]

In my view, gtest running on the build computer and ntest running on the device serve very different purposes. We need them both.

With the amount of functionality that the firmware has, I would expect to need several thousand tests to get basic coverage of the critical components (navigator, commander, flight tasks, helper libraries and maybe some select drivers). We need to design for where a single developer can easily write 20+ tests a day, to achieve this. So, the testing environment needs to be as flexible as possible so that we don't need to do much refactoring while writing tests. The tests also need to be fast to run, and every contributor needs to be able to run all of them locally, quickly (in a few seconds), to make sure nothing was broken.

Given this, I don't think adding the constraint of running them all on embedded hardware is going to be feasible. It will be slow, and we'll quickly hit the 2MB flash limit. On the other hand, most of the 'business logic' can be tested very easily on faster hardware which has more storage, with the basic assumption that the problems we are trying to prevent are due to bugs in the code itself, not things which are different between the build system and the device.

What needs to run on the device itself (eg. with ntest) are the tests which make sure the hardware platform is as expected, so that the above assumption holds true. Testing the POSIX interface. Testing the driver interfaces. Testing threading issues and that network and serialization code works with big and little endian environments correctly. This way we can confidently transfer the results of the much larger suite of tests running elsewhere to the embedded platform.

[jkflying]

@julianoes Ok, so instead of changing the existing unit tests too much I instead decided to introduce a new test class, functional tests. This way the tests which don't have dependencies on parameters or uorb can still be used with ntest, and it makes adding new functional tests easier because we don't need to list all of the dependencies each time.

@hamishwillee I added a minimal example in the parameters library which just sets parameters and publishes uOrb messages. I'll be sure to mention this and link to it in the docs when I do that.

[julianoes]

So, the testing environment needs to be as flexible as possible so that we don't need to do much refactoring while writing tests. The tests also need to be fast to run, and every contributor needs to be able to run all of them locally, quickly (in a few seconds), to make sure nothing was broken.

I agree with that!

@julianoes Ok, so instead of changing the existing unit tests too much I instead decided to introduce a new test class, functional tests. This way the tests which don't have dependencies on parameters or uorb can still be used with ntest, and it makes adding new functional tests easier because we don't need to list all of the dependencies each time.

Ok sounds like a good trade off.

[jkflying]

@dagar @MaEtUgR I'd like to get this in, do you have anything that you'd like changed?

[LorenzMeier]

Thanks, this is truly awesome!

[LorenzMeier]

Could you rebase quickly and push? Thanks!