Initial unit tests based on GoogleTest and GoogleMock

[uweseimet]

@akuker @rdmark The feature_google_test branch contains some initial unit tests based on GoogleTest, which I would like to be commented on before more time is spent on them. The relevant changes only affect the Makefile and add one additional source file in the new "test" folder. Some notes:

• There does not seem to be a pre-compiled GoogleTest for Raspberry Pi OS. I don't see an issue with that because as long as the unit tests are hardware-independent (otherwise they might rather be classified as integration tests) they run on any platform. Currently I run them on a regular Linux PC, which is a better development environment than the Pi anyway.
• The tests are built and run with the "make test" target. The default "make" target tdoes not build them, so there are no compile-time issues (see above) on the Pi, and compiling on the Pi does not take longer than before.
• In order to test not just a small part of the code but to have a substantial amount of tests, a lot of refactoring of the existing code may be required. The current code still lacks the required granularity and encapsulation. But it is normal the adding tests has an influence on your code structure, and you have to start somewhere ;-).
• https://github.com/akuker/RASCSI/issues/455 may have to be revisited at some point in time, but only if there are ever enough tests to justify spending time on https://github.com/akuker/RASCSI/issues/455. There are already too many tickets that were started but are now in a rather undefined/unfinished state ...

>make test
./bin/fullspec/rascsi_test
Running main() from /var/tmp/portage/dev-cpp/gtest-1.11.0/work/googletest-release-1.11.0/googletest/src/gtest_main.cc
[==========] Running 2 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 2 tests from ModePagesTest
[ RUN      ] ModePagesTest.SCSIHD_AddModePages
[       OK ] ModePagesTest.SCSIHD_AddModePages (0 ms)
[ RUN      ] ModePagesTest.ModePageDevice_AddModePages
[       OK ] ModePagesTest.ModePageDevice_AddModePages (0 ms)
[----------] 2 tests from ModePagesTest (0 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 2 tests.

[rdmark]

@uweseimet It doesn't seem like x86 Debian comes with pre-compiled GoogleTest libraries either. The googletest and google-mock packages simply install source code into /usr/src/googletest and then expects you to include the code to build with your C++ project.

I tried pulling in the headers with the -I parameter but the process fails at linking because I don't think compiled libraries are available.

CPLUS_INCLUDE_PATH+="/usr/src/googletest/googletest/include:/usr/src/googletest/googlemock/include" make test

What Linux distro are you using for development?

[uweseimet]

@rdmark I am using Gentoo. But even if not all Linux distributions out of the box support GoogleTest, this should not block the PR because the tests are not compiled by default anyway, i.e. if they do not compile they do not break anything. On most distributions https://www.eriksmistad.no/getting-started-with-google-test-on-ubuntu/ should work.

[rdmark]

Thanks for this guide. Going through this, and then repeating the same for libgmock* got the tests to link. Confirmed working on Debian Sid bleeding edge. :)

[rdmark]

@uweseimet have you thought about tooling for measuring code coverage, e.g. line coverage metrics?

[uweseimet]

@rdmark I don't think I will spend time on this soon. Unfortunately everything related to C++ appears to be more or less a one-man-show, which is not so good ... If somebody was adding a ticket for code coverage she/he should also be prepared to work on this ticket. Saying this, I think we should be more consequent in rejecting tickets. There are feature tickets that require major efforts (not only in C++ coding), and those who created the tickets for new features are not willing to work on them, or lack the knowledge to work on them. Such tickets should be rejected, because the project is obviously lacking resources.

[rdmark]

@rdmark I don't think I will spend time on this soon. Unfortunately everything related to C++ appears to be more or less a one-man-show, which is not so good ... If somebody was adding a ticket for code coverage she/he should also be prepared to work on this ticket. Saying this, I think we should be more consequent in rejecting tickets. There are feature tickets that require major efforts (not only in C++ coding), and those who created the tickets for new features are not willing to work on them, or lack the knowledge to work on them. Such tickets should be rejected, because the project is obviously lacking resources.

In my opinion there is value to having a backlog of improvements and new features, even if there isn't anyone who can immediately work on them. It helps show the community a future potential roadmap, as well as serve as starter tickets for potential code contributors. That said, I'm not opposed to closing out lower importance improvement ideas to reduce the clutter.

Now, I'd be interested in investigating code coverage tooling myself. I asked just because I was curious if you already had something in mind.

[uweseimet]

@rdmark Actually, if I were adding tickets for all potential improvements I can think of there would be a flood of new tickets ;-). We could add tickets for any SCSI device type not yet implemented. In theory these kind of tickets would be legitimate, but in practice ... The more tickets we have, the harder it gets to keep the overview. If more and more tickets are added and not resolved, this is an indicator that there is a problem with a project. Regarding starter tickets: There are several of them (labelled accordingly) already. A backlog or roadmap requires prioritizing tickets and spending more time on project managment ;-). If you have tickets dealing with support for new hardware, for instance, the community may initially like this. But when months later nothing has happend it looks like there are a lot of ambitions, but nothing gets done. Just my two cents worth.

[uweseimet]

@rdmark I have not yet commented on code coverage tools. I have not investigated this. As long is there is no substantial amount of unit tests, collecting code coverage metrics is not that useful. In addition having the existing (2 years ago) GitHub actions automation ticket resolved would be helpful in this context, in order to have automated building, testing and collecting metrics.

Having a decent number of (C++) unit tests may be out of scope for this project anyway. Who is going to write and maintain these tests and refactor the legacy code to become testable? This is not necessarily something you can do on-the-fly. Look at the SASI removal and the related changes and refactorings which affected the SCSI code. If there had been unit tests for the SCSI code, updating these tests would have required quite some time. Nevertheless unit tests are a good thing, of course. For rascsi in particular, because it is likely that some of the platforms in the compatibility list do not work anymore. A developer cannot test them all, and there do not seem to be many users who test their hardware with the develop branch, often not even with recent releases.

Regarding code coverage, which percentage would you want to achieve? 70-80% is a typical goal I think. Once I wanted to learn whether you could reach 100% in some of the Java projects I was involved with. I was surprised that this was actually possible, but how well this works depends on the nature of the project. Multithreading is not helpful here ;-).

[uweseimet]

@rdmark Bear with me for being that talkative, but I just remembered that I had a look at how to use SonarQube for C++ some time ago, because I also use it for other languages. Besides code coverage metrics SonarQube provides a lot of additional (code qualitiy) metrics. The free SonaQube edition does not support C++ code analysis. For open source projects one can use https://sonarcloud.io/explore/projects, but you would still need the C++ code scanner in your build environment, and the scanner does not appear to be free.