Testing the quality of Google Test

In this post we’ll dive deeper in the quality of Google Test. We will be examining the quality checks and tests the developers put in place to make sure that every update is working and is up to par with the rest of the project. We will also take a look at what is holding the project back, the culture and what they could improve.

Software Quality

Software quality can be split into two categories.¹

The first category is the software functional quality. This category is the quality of the system with respect to its design and its intended functionality. Also a comparison can be made to competitors. If we look at this functional quality of Google Test we see that it performs better than other C++ testing frameworks². Another fact supporting this is that Visual Studio already supports the Google Test library in their software.³ Moreover, if you look at the functionality of Google Test, most of it works as it is supposed to do. Especially the basic operations work as intended. Still there are a quite a few issues open on their github page.⁴ Most of the issues are related to more advanced features of Google Test. If these features are demanded by the functional requirement, their absence will have a negative effect on the software functional quality. However if these features are not included in the functional requirements, only they are just wishes of external developers, by definition¹ they will not have any effect on the quality. Unfortunately these assumptions about software quality will remain theoretical, as we have no access to the functional requirements of Google Test. We can state that the competitors also have issues open and therefore Google Test does not lose quality with respect to its competitors on.^{5 6}

The other category of software quality is the software structural quality. This is the quality of a system with respect to its non-functional requirements such as scalability, reusability and maintainability. Later in this post the quality with regard to maintainability will be discussed. In the previous post we already discussed how easy to use and convenient Google Test is, along with its efficiency, extendibility and variability. Other non-functional requirements will not be discussed since this will be out of scope for this post.

Software quality process

To enforce software quality, a system must have a software quality process. Google Test has certain processes and multiple guidelines in place to enforce this quality. At first, they have a style guide for all the developers who want to contribute ⁷. This contains a style guide for all MarkDown documentation with guidelines on the technical level like white spaces and line endings, but also guidelines on best practices. The most important part of the style guide are the guidelines with respect to the code, for example the variable naming convention, the maximum number of lines in a function and the use of comments. All these guidelines make the code more clearer for developers since they are all in the same format.

It is the job of the developer to comply with the style guide. After commiting the code, two automated tests are performed to enforce the quality. Those tests are performed by two continuous integration (CI) tools, namely Travis CI and Appveyor. The two CI’s build the software, execute the software’s unit tests and perform multiple tests on their own. It is possible that only one of the two CI tests fail, as they test the software quality from different aspects. Both tools are compared on stackshare.⁸ Here you can see that both tools are primarily the same but described in a different way. However there are some small differences. For example that Appveyor is much more Windows oriented and generates artifacts during the build.⁹

Even though you can create a pull request with a failing commit, the pull request cannot be merged. When a pull request is created there is one additional automated check to the CI tools. This check is performed by the google bot ¹⁰. This bot checks if all contributors of the pull request signed the Contributor License Agreement. This is a typical agreement seen within open source projects. It defines the terms under which the code is contributed to a company (in this case Google). This prevents a lot of legal issues later on in the process if someone claims his or her code as his or her property. The Googlebot also checks if everyone who made a commit in a pull request consents that the code is merged to the master.

At last, there are also some similarities between the pull requests with regard to the software quality process. After a pull request is made, one of the contributors from the Google team comments on it. This contributor can request technical fixes but also request style fixes in line with the style guide. After these requests are fixed, the pull request will be reviewed by an internal Google code review team which can also request fixes regarding technical or style issues. If this team accepts the pull request, one of the members of this team will merge the pull request.

Testing process

As mentioned earlier, the continuous integration tools run the tests to make sure nothing (in the code which is tested) is broken by a new commit. This already is an important part of a testing process. They also focus on testing the framework in the contribution file so that new developers know they have to test. However the test coverage is not taken into account during the whole project. This would be a useful addition to see whether there is some untested code.

Code quality

Several aspects with respect to the code quality of this project are identified and elaborated below.

Deprecated headers and functions

As seen in some pull request^{11 12}, there are deprecated headers and functions within the code base.

The deprecated headers will cause problems when newer versions of compilers no longer support them, as is noticed by one of the users which suggests a pull request to remove them¹². However, it should be noted that removing these deprecated headers will cause problems for older compiler versions, which is why the maintainers are hesitant to remove them. But keeping deprecated headers will lead to an ever increasing amount of technical debt.

When own function names are renamed, the old function name is first marked as deprecated before being removed in a later version. This is seen as a good practice in order to not immediately cause problems for projects depending on it. This improves the code quality by not having to update many references at once, thus changes can be made easier. This is called “lowering the technical debt”.¹³

Old macros

The project uses a fair amount of macros. These were created a long time ago and are now beginning to become an issue^{14 15}. Macros are used throughout the project to make programming easier and lower the amounts of lines of code. But now they realize that the way they have been using these is not scaling well. The problem is that some of them are so ingrained that changing them in a significant way would also require reworking a lot of other pieces of code. This design choice which first reduced technical debt, now is actually increasing it. Perhaps it would be better to refactor all macros to constant global variables. This is, for example, because macros cannot be debugged and have no namespace.¹⁶

Pull request time-to-live

The time-to-live for a pull request varies significantly. Most notably one (outlier) is observed to already have a time-to-live of approximately 2.5 years when taking into account the original¹⁷ and the resubmitted¹⁸ pull request. The activity of that pull request has been low for a long time. The first comment on it, after 2 pings from the author of the pull request, was approximately half a year later. Consequently, the author of the pull request did not respond anymore later. The same pull request has now been re-submitted and might finally be merged.

Even though some relatively small pull requests¹⁹ have not yet gotten attention, all currently active pull requests have been created less then 2 months ago or are assigned to a maintainer. Therefore, currently the time-to-live for pull requests is estimated to be relatively short which improves the code quality by quickly updating the code base with fixes.

Documentation

Something which can be found in the issues²⁰ are suggestions around documentation. Though the functionalities are well documented, installation and usage can be challenging for people who are new to this type of testing. Even without this, these documents are pretty long sometimes and lack a table of content, which people have reported as an issue.²¹

Current activity

At this point the project is not looking to add major functionalities. Looking at the issues and pull requests, they are more concerned with maintaining and minor improvements. This is reflected in the todo’s mentioned in the code^{22 23 24}. The figure above shows the commit activity of last year. The project is alive but real activity is in waves. Moreover most activity appears to originate from a small selection of contributors. At this moment the two main contributors are the github accounts gennadiycivil and kuzkry. The former is a part of google, the second does not show its background.

Refactoring

It is observed that most functions have a McCabe complexity less than 10. Several have complexities been 10 and 20, while only one function has a complexity of 26. This function is called “IsValidUTF8” and is located in a googletest source file²⁵. The reason for this relatively high complexity are several branches with many conditionals. According to the comments, each if statement filters on a specific amount of bytes per character. To reduce the McCabe complexity, for each if statement the conditionals could be placed in a separate function. In such a case new functions like “Is2ByteChar” can be called, which return a boolean, in each if statement instead.

The macros mentioned earlier would also be a valuable thing to refactor. This is not something that is easy to refactor, but it would improve the system’s scalability. However, other extensions may be developed with these macros in mind, and use them as well. This would further complicate this refactorization.

File name consistency can also be improved. The way files are named is not consistent throughout the program. But this has not been a big issue so far (there have been no issues or pull-requests about it recently) and could spell horror for extensions depending on this project, so it is not likely Google Test is willing to make changes in this any time soon.

One way the naming could be somewhat addressed, is by making some documentation about it. There are multiple issues about the documentation in general already^{21 20}, so this can be a worthwhile time investment for someone wanting to support the project.