[RFC] Running libc unit tests as integration tests

There are two testing frameworks in use today in the libc project. We call one of them the unit testing framework, and the other the integration testing framework. This RFC is about unifying them and adding the ability to run all unit tests as integration tests also.

The need for the ability to run unit tests as integration tests

The libc unittests test libc functions using a libc internal gtest-like framework. The final unit test executable is linked to the system libc. On the other hand, what we refer to as the integration tests in the libc project are really self-contained tests. As in, the test binary contains components from the libc project alone - nothing from the system libc is pulled in. In fact, other than the compiler intrinsics, the test executable consists of only bits generated from the source code living in the libc project. On platforms like the GPU and baremetal embedded devices, loading and launching such self-contained executables for testing is the only option. For, such platforms are in various stages of bring up and having the ability to execute tests in a minimal environment enables gradual bring up. Further, as the higher levels are brought up eventually, a well tested lower level allows isolating higher level bugs.

Apart from helping with bringup of minimal systems, unifying has an added benefit from the maintenance and UX point of view - after all, they are currently two different frameworks which have to be maintained, and they do not provide the same UI. By unifying the two frameworks, we not only reduce the maintenance burden, we also unify the UI/UX around them.

Proposed Steps

In this section we propose the steps that are to be taken to allow unit tests to run as integration tests. In fact, we propose that integration tests be called self-contained [1] tests going forward.

  1. The first step would be to remove or replace items used by the LibcTest library with lightweight implementations provided by the libc project. There are only two items of this kind:

    1. Replace the use of std::string with a lightweight cpp::string class implemented in
      src/__support/CPP.
    2. Replace the use of std::cout with a test only logging facility. We can implement a
      TestLogger class and add a global instance of this class named testing::log. The use of
      std::cout will be replaced with testing::log. The log object writes to the standard output.
  2. Add a new CMake rule add_libc_test which automatically adds a unit test as well as a self-contained test. On platforms where unit tests are not meaningful, the rule will not add unit tests. We will continue to have two separate rules add_libc_unittest and add_selfcontained_test (instead of the current add_integration_test). The new rule add_libc_test will be a wrapper over these two rules. Those libc components, for example threads, for which only self-contained tests are meaningful, we will use the add_selfcontained_test directly.

  3. Test target naming: Currently, test targets have a fully qualified name like, libc.test.src.string.strlen_test. Since the propsed add_libc_test rule adds two different targets, we will setup target dependency in this fashion by introducing two new targets:

    • libc.test.src.string.strlen_test depends on
      * libc.test.src.string.strlen_test.__unit__
      * libc.test.src.string.strlen_test.__selfcontained__ [1]
      Developers can continue to run ninja libc.test.src.string.strlen_test to run both the unit test and the self-contained test. They of course choose to run the unit test or the self-contained test separately with ninja libc.test.src.string.strlen_test.__unit__ or ninja libc.test.src.string.strlen_test.__selfcontained__.

Sequence of Steps

  1. Implement cpp::string and replace the uses of std::string with cpp::string.
  2. Implement a TestLogger class as described above and replace uses of std::cout with testing::log.
  3. Rename add_integration_test to add_selfcontained_test [1].
  4. Implement the new rule add_libc_test as described above.

[1] - To reduce the tongue twisting effect of saying self-contained tests, we can choose to call them hermetic tests. The rule to add a hermetic test can then be called add_hermetic_test. The target added by it will have the name fully.qualified.name.__hermetic__.

2 Likes

This sounds good overall. How much of the proposed implementation relies on a fully functioning printf? That’s difficult to implement on all platforms. Also I’m a bit more partial to the name “self-hosted tests”.

I don’t think we have to rely on printf at all. All we need are integer to string conversion utilities.

self-contained tests and self-hosted tests both are a little bit of a tongue twisters. But, if I were to pick one of these lengthier names, then I would pick self-contained because self-hosted in a general context does imply self-contained-ness but also tends to imply something different from the conventional.

Really? That sounds like a way to validate the test, but doesn’t tell you anything about libc code itself, IIUC.

Ah, I can see why it can be interpreted that way. A libc unit test executable consists of this:

the test framwork + the libc function under test + pieces from the system libc + pieces from the system C++ standard library

Few points about this:

  • The libc function under test is in the namespace __llvm_libc, hence will not collide with the equivalent from the system libc.
  • The test frame work uses the C++ standard library and that is what pulls the system libc and the system c++ standard library into the executable.
  • The startup system also comes from the system libc.

Aha. So a unit test pulls in the project function(s) under test, with a non-conflicting name, but otherwise links against the system libc. While the integration test uses only the project libc, in effect being a “freestanding” or “unhosted” program that happens to replicate most/all the functionality of libc.

So, making unit tests behave like integration tests seems plausible, and not too dissimilar from the idea of an LLVM or Clang unittest relying on big chunks of other LLVM/Clang functionality that aren’t the bit being tested.

I’ll caution against one thing: I have a memory from years ago of running into a situation with C++ library tests, where the test for A depended on functionality B, and the test for B depended on functionality A. Having A and B act as each other’s “test oracle” allowed things to slip by, where a properly layered test suite would have at least one of them verified independently.

LGTM overall. Slight preference for the hermetic wording here.

I like that add_libc_unittest declares both unit and hermetic tests at the same time but AFAIR they live in different folders right now. Do you intend to change the directory structure as well? What about the naming if we have both unit and hermetic tests?

Another question, should we add targets to execute all unit tests or all integration tests?

Also we only have unit tests in Bazel for now, do you intent to support hermetic test as well for this build system?

Yes, I missed mentioning about the directory structure: we will remove the test/integration directory.
About target naming, I mentioned above that unit tests will have a suffix of .__unit__ and hermetic tests will have a suffix of .__hermetic__.

We already do: libc_unit_tests and libc_integration_tests (will be renamed to libc_hermetic_tests).

Yes, I plan to add them in the Bazel config also soon.

1 Like

Agreed. Sometimes it is unavoidable to test a group of functions together so we are extra cautious in crafting tests for such group of functions.

I can take care of TestLogger.

Related question, We already have StreamWrapper that has the same functionality and delegates to iostream, should we replace it with TestLogger?

Step 1.1 and 1.2 handled in ⚙ D147231 [libc] Adds string and TestLogger classes, use them in LibcTest

Thanks for the patch Guillaume, I think we ended up with some duplicated work. I had filed a metabug and few child bugs for these items: Run libc unit tests as integration tests also · Issue #61751 · llvm/llvm-project · GitHub. The string patch was assigned to me (⚙ D147186 [libc] Add a libc internal equivalent of std::string and use it in unit tests.) and the test-logger was assigned to @michaelrj-google (not sure about the status of work, I was hoping he would answer your question from yesterday).

Never mind though, I will review your patch and we will take it from there.

For next time, to avoid duplicated work, I think this is what we should do:

  1. If a bug has been filed for a task, and if there is a corresponding RFC, we should post the link to the bug on the RFC thread.
  2. Active developers should subscribe to appropriate bug and discourse notifications.

Ah I’m sorry for this… I didn’t see the bugs :face_with_diagonal_mouth:
I’ll make sure to synchronize next time. My apologies for the confusion.

I haven’t started any work on TestLogger, so @gchatelet’s patch doesn’t conflict with me at all. As for your question, I think the approach you’ve taken in your patch is the correct one. Just replacing cout with TestLogger should work and be fairly simple, while also potentially allowing us to swap back with a switch.