Add unittests

[jaebaek]

I believe that unittests will reduce unexpected bugs of VkRunner. How do you think about adopting googletest for unittests?

[bpeel]

Yes, it’s getting pretty urgent that we start doing some unit tests to test all the fiddly parsing corner cases. It would probably be good to look at other frameworks as well. If we had something that was C-based where we could put small tests directly into the C sources we could avoid the problem of not being able to call static functions.

[jaebaek]

Sure. VkRunner uses static functions in many places. If we can test them easily, it would be nice. Please let me know your opinion after thinking of it.

[bpeel]

Perhaps we could just use a very simple custom framework. If we use the test reporting stuff from CMake it doesn’t need too much code. I experimented with this in this branch.

[dneto0]

Please don't make a custom unit test framework.

[bpeel]

@dneto0 Can you please elaborate on your arguments against using a custom framework? Do you have another framework in mind that provides some particular features that we need?

[dneto0]

Reusing code is almost always better than writing new code.

googletest is well established, powerful, and convenient.

Being well established means new people can contribute tests and understand existing tests with low friction. It has also sorted out lots of things like proper test isolation.

googletest is also portable, running at least on Linux (clang and GCC), macOS (clang), and Windows (using both MSVC and MinGW compilers). Your custom code uses attribute((constructor)). That's a GCC-specific syntax.

Over time your test suite is likely to grow, and you might want ever more of the power of googletest. For example, SPIRV-Tools currently has 25410 unit tests driven via googletest, organized into 95 executables, and parallelized at the executable level by ctest. They run in under 10s serially on my workstation, and 4s when run 20-way parallel.

You mentioned parsing corner cases as lacking in tests. My Effcee library has a lot of parsing functionality in it. (https://github.com/google/effcee) It has 239 unit tests driven by googletest, in one executable, running in 0.04s. For example the parsing of check rules starts here: https://github.com/google/effcee/blob/master/effcee/check_test.cc#L96
I hope you can see that the expression of a unit test is simple and direct.

Both SPIRV-Tools and Effcee also extensively use parameterized tests to check matrices of possibilities over the same functionality. E.g. lots of combinations of tests can be written as a table of cases. E.g. see https://github.com/google/effcee/blob/master/effcee/check_test.cc#L265
That's a pretty sophisticated pattern but makes adding new tests easy once you work out the first case, and therefore contributes greatly to the coverage and quality of your tests.

You are likely to want to use this generality in the tools, and you're unlikely to build it in as nicely as a pre-existing framework that other people have worked on for more than a decade.

[bpeel]

As far as I can tell, googletest requires C++ so I think it will be difficult to shoe-horn into a C-based project. It will mean we can’t write the tests directly in the corresponding source files and all of the internal headers will be restricted to using the subset of C that is also common to C++. I think if we don’t want to use the custom framework idea then we will still have to look at other frameworks.

The constructor attribute also works on clang. I think it should be possible to use an equivalent thing with MSVC if need be. The unit tests are only optionally compiled so it doesn’t break building on MSVC. The tests are only meant to test the correctness of the code so I don’t think supporting all compilers is particularly important.

The test framework is still using CMake so ctest and its parallelisation still works. It is also compiling all of the tests into a single executable. The unit tests with this approach are defined in a similar way to googletest, with just a single macro call to contain the code and the name which can be added anywhere in the source.

The framework is very minimal so I think it would be simple to swap it out for something else if the project outgrows it later.

[ianromanick]

We use googletest in both the C and C++ parts of Mesa without any difficulty. It is robust and feature rich. We may not need all the features now, but predicting the future is a tough business to be in. I wouldn't write my on string formatting function just because I don't need some feature that printf has. I don't understand the problem.

[bpeel]

The disadvantages of using googletest are mentioned above. It means all of the internal headers have to be compatible with C++ and that we have to add a second language to the project. If everyone thinks that that is a worthwhile trade off then we could use it.

Another option might be to use Criterion. It seems quite nice and it supports C99 directly. I’ve made an experimental branch for it.

[samuelig]

I think googletest is a well established project. As Ian said, it is used for testing C and C++ parts of Mesa, so we can do the same here. I know it is adding a second language to the project, but I don't see it is too much of a hassle at this moment.

[bpeel]

Ok, I guess I am outvoted. Let’s try to do a proper review of Jaebaek’s PR then and get it merged.

[samuelig]

Repository moved to https://gitlab.freedesktop.org/mesa/vkrunner. Please re-submit this pull request there.