Add Unit Tests

[wimrijnders]

This PR adds CATCH2 as testing framework. It has been added to the project as a single include file UnitTests/catch.hpp.

Run the unit tests with make test.

The initial version has a demo unit test taken from the CATCH2 'Getting Started' page. The demo test will be removed as soon as serious tests are added.

The directory name UnitTests is a temporary name. This will be renamed to Tests as soon as the current Tests directory has been renamed to Examples.

---

Example output

When all tests succeed:

QPULib> make test
===============================================================================
All tests passed (8 assertions in 1 test case)

When one or more tests fail:

QPULib> make test

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
runTests is a Catch v2.2.3 host application.
Run with -? for options

-------------------------------------------------------------------------------
Example unit test taken from the CATCH2 github
  Factorials are computed
-------------------------------------------------------------------------------
UnitTests/testMain.cpp:27
...............................................................................

UnitTests/testMain.cpp:28: FAILED:
  REQUIRE( Factorial(0) == 1 )
with expansion:
  0 == 1

===============================================================================
test cases: 1 | 1 failed
assertions: 6 | 5 passed | 1 failed

[mn416]

Hi @wimrijnders,

A couple of points on this:

1. In the license file, can you exclude UnitTests/catch.hpp? You'll see there are already some files in the repo being excluded in the license.

2. I'd like to see some actual QPULib unit tests before merging this

Thanks

[wimrijnders]

About tests:

• An obvious thing to do is move AutoTest to the unit tests. This is straightforward.
• Another thing I thought of, is to compare the outputs of the Rot3D kernels. These should match exactly.
• The source/target output is something to be tested as well. However, I can not add this before #25 is merged.
• Mailbox functions deserve unit tests as well. However, this is hardware dependent so should execute conditionally. Thinking about this.

Other than that, I'm a bit stumped for relevant tests right now. Can you suggest something, preferably trivial, that would require testing?

I intend to make the tests first without changing the current code base. After that, the code should be DRY'd.

[wimrijnders]

I've added unit tests for AutoTest and Rot3D. This is for illustrative purposes, the intention is to DRY the code copied to the unit tests.

The test "Multiple kernel definitions should be possible" in file "Tests/testRot3d.cpp" currently fails with:

QPULib: heap overflow (increase EMULATOR_HEAP_SIZE)

This is in emulation mode on a 64-bit Intel linux.

From what I see, it's the same thing as discussed in #25, namely there's no deallocation in heap emuHeap as used by SharedArray.

What long-term solution would you suggest? The easy way out is to do what the error says, namely increase EMULATOR_HEAP_SIZE. But a more structural solution would be better.

[wimrijnders]

Ran the unit tests on the Pi 1 with make QPU=1, Following happened:

Tests/testRot3D.cpp:131
...............................................................................

Tests/testRot3D.cpp:110: FAILED:
  REQUIRE( x_scalar[i] == x[i] )
with expansion:
  -3.00001f == -3.00001f
with message:
  Comparing Rot3D_1 for index 3

This compares the output of the scalar version with the Rot3D_1 example. There appear to be minute differences in the floats. Catch2 has provisions for this.

But the output of the different kernel versions should be totally identical. Will adjust the tests for this.

In addition, the unit tests also need sudo to run the QPU's. Will add this as soon as there is a reliable way of determining the platform.

---

I also have to report that the time to run the tests is large.

On intel i7 machine:

> time make test
...
real    0m8.102s
user    0m7.451s
sys     0m0.646s

So this is for a full compile from scratch + run the unit tests.

On the Pi 1:

> time make QPU=1 test

real    7m19.744s
user    6m35.780s
sys     0m10.820s

This is compilation only; it failed to run the tests because 'sudo' is missing. So these times are for compile from scratch only.

Redo from this point with sudo:

real    2m18.613s
user    2m7.574s
sys     0m0.346s

This comes after previous make, so these values are for running unit tests only.

Result

So the i7 ran the full tests about 75 times faster.

I guess this is to be expected. It's really a totally unfair comparison. It's something to just take into account.

Speaking for myself, I have love for the Pi platform and fully accept this. I'm looking forward to the moment I can show that the dinky $50,- Pi's using the VideoCore make a fool of the $1600 i7 laptop I have for daily use.

[wimrijnders]

Previous BTW was a check to see if there were any build issues on the Pi wrt CATCH. Till now I was testing on the laptop.

I am happy to report that, despite the compilation time needed, the build and test run went without a hitch.

[wimrijnders]

All unit tests pass now. If you can accept current state, I will DRY the code.

[wimrijnders]

Is this enough unit tests to start off with, BTW?

[mn416]

Your new unit tests look great -- very nice! Very happy to merge these.

My view on catch is that I'm happy to accept new tests that use the catch framework, but I may not use it for my own tests, e.g. AutoTest. In other words, I'm happy for catch to be used but I don't think it should be enforced. So, can we keep AutoTest independent of catch (it's not really a unit test anyway)?

Minor things:

Is sudo needed even when running tests in EMULATION_MODE? Would be nice to only require sudo when actually needed.

I see there is a reference toAutoStart in AutoTest.cpp -- maybe rename this?

[wimrijnders]

Adding unit tests: I do not intend to add full coverage in the unit tests from the start. I will add tests in the following cases:

• For new functionality, of which it is not obvious it will work correctly
• For testing fixes on bugs which are considered to be non-trivial, i.e. regression testing
• For parts of the code which I consider non-obvious and important, and which might just change in the future.

These are the situations where I regard unit tests to be essential. For the rest, unit tests may be added.

I hope you can maintain a similar discipline, but I won't demand it from you. I believe I'm more paranoid than you, so chances are I will add unit tests myself for any changes you might make in the code.

AutoTest: OK, I understand this. However, I think it's a very useful test to run regularly, you never know how the code generation will change in the future. It's a good 'canary' IMHO for catching broken things.

As a compromise, I will retain the current AutoTest as is and call it in the unit tests via a system call. Best of both worlds and DRY'd.

AutoStart: I don't know where this comes from! My fingers type that automatically when I want to type AutoTest.

[wimrijnders]

@mn416 Unit test PR read for review again.

• Addresses all review points
• DRY's the code of the Rot3D kernels
• Adds dependency generation for the include files in directory Examples.