1

Using Google Test, I want to test the behaviour of a Server.AcceptRequest method:

class Server {
public:
    // Clients can call this method, want to test that it works
    Result AcceptRequest(const Request& request) {
        queue_.Add(request);
        ... blocks waiting for result ...
        return result;
    }
private:
    // Executed by the background_thread_;
    void ProcessRequestsInQueue() {
        while (true) {
            Process(queue_.PopEarliest());
        }
    }

    MySynchronizedQueue queue_;
    std::thread background_thread_ = thread([this] {ProcessRequestsInQueue();});
};

The method accepts a client request, queues it, blocks waiting for a result, returns a result when available.

The result is available when the background thread processes the corresponding request from a queue.

I have a test which looks as follows:

TEST(ServerTest, TwoRequests) {
    Server server;
    Result r1 = server.AcceptClientRequest(request1);
    Result r2 = server.AcceptClientRequest(request2);
    ASSERT_EQ(r1, correctResultFor1);
    ASSERT_EQ(r2, correctResultFor2);
}

Since the implementation of a Server class involves multiple threads, this test might pass on one attempt but fail on another. To increase the chance of capturing a bug, I run the test multiple times:

TEST_P(ListenerTest, TwoRequests) {
  ... same as before ...
}
INSTANTIATE_TEST_CASE_P(Instantiation, ServerTest, Range(0, 100));

But now make test command treats each parameterised instantiation as a separate test, and in the logs, I see 100 tests:

Test 1: Instantiation/ServerTest.TwoRequests/1
Test 2: Instantiation/ServerTest.TwoRequests/2
...
Test 100: Instantiation/ServerTest.TwoRequests/100

Given that I do not use the parameter value, is there a way to rewrite the testing code such that the make test command would log a single test executed 100 times, rather than 100 tests?

mercury0114
  • 1,341
  • 2
  • 15
  • 29
  • 1
    In my opinion you should not have non-deterministic unit tests at all. Unit tests are for testing your functionality (as opposed to the environment). If you feel your logic surrounding the http listener is significant enough that it calls for unit tests, use a mock listener that the test can fully control. – 500 - Internal Server Error Jun 19 '20 at 09:12
  • @500-InternalServerError But am I not testing the functionality here? I.e. testing that the function Listener.AcceptClientRequest returns correct output given the input. – mercury0114 Jun 19 '20 at 09:21
  • That is not what is traditionally referred to as a unit test. Unit tests test discrete units of code, not system integration. – 500 - Internal Server Error Jun 19 '20 at 09:23
  • That's confusing. Why would you have a non-deterministic code to begin with? Why the result is non-deterministic? That doesn't make sense. If by "non-deterministic" you actually mean "it behaves differently under different, external conditions" then this is not "non-deterministic" and you should simply replicate/mock those conditions and test those cases. Or maybe by "non-deterministic" you mean "some level of randomness is involved", then in that case you could run the test hundreds of times, gather results and do some statistical checks. – freakish Jun 19 '20 at 09:32
  • Test deterministic behavior is better. Eg: do you really need to check ID? – Louis Go Jun 19 '20 at 09:33
  • @LouisGo Replaced ID by something more meaningful in the question. But in general, I have a class that uses a background thread. I want to test the public method of that class. Given that the background thread could kick in at an uncontrolled time point, the public method might fail sometimes (e.g. race condition) and succeed other times for **the same** input. How to properly test that? – mercury0114 Jun 19 '20 at 09:48
  • If your problem is your code is not designed for multithread, then [force googletest to run in single thread](https://stackoverflow.com/questions/29335446/command-line-options-to-force-googletest-to-run-in-single-thread). – Louis Go Jun 19 '20 at 09:50
  • @freakish Added explanation. – mercury0114 Jun 19 '20 at 10:04
  • Umm, put a `for` loop around the code? – rustyx Jun 19 '20 at 10:44
  • @rustyx yeah that does work, I was wondering if there's a more elegant way without adding extra code to the test – mercury0114 Jun 19 '20 at 11:05

3 Answers3

6

Simple answer: use --gtest_repeat when executing tests would do the trick (default is 1).

Longer answer: unit tests shouldn't be used for this kind of tests. GTest is thread-safe by design (as stated in their README), but this doesn't mean it is a good tool to perform such tests. Maybe it is a good starting point to actually begin working on real integration tests, I really recommend Python's behave framework for this purpose.

li ki
  • 342
  • 3
  • 11
Quarra
  • 2,527
  • 1
  • 19
  • 27
  • Can gtest_repeat be added into CMake file? – mercury0114 Jun 22 '20 at 14:27
  • Never tried this, so not sure. It is runtime configuration though, like `--gtest_filter`, so I doubt it, but maybe CMake does support it. – Quarra Jun 23 '20 at 05:51
  • 1
    Even if it does, I'd recommend to have one CI job that just runs all the tests once (i.e. without `--gtest_repeat`) and another CI job that runs this particular test multiple times (i.e. using `--gtest_repeat` with `--gtest_filter=ListenerTest.TwoRequests`) because all the other tests should be small and running them multiple times doesn't make much sense. Adding special pre/suffixes to test (like `BUGCHECK`) might be helpful to filter for tests that needs to be run multiple times. – Quarra Jun 23 '20 at 05:58
  • 1
    Note: `gtest_repeat` will repeat all the tests. And if there were errors in previous iterations, they won't be reported in the end, if the last iteration was successful. You'll have to scroll or scan through all the runs and previous iterations manually. The final summary lines will only include the test numbers, failures and timings of the **last iteration**. Watch out for flaky tests. – Snackoverflow Aug 02 '23 at 13:22
1

Every test should run on a clean slate, not sharing state with other tests. In other words, if a test breaks as a result of running after another test, then your prod code might be broken, but your test code is definitely broken.

If you want to know that your listener can accept multiple requests, then that is your test. Create a clean instance of the listener and run a loop in the test, making a bunch of calls. If desired, you can create a new thread for each call (just remember to clean up at the end of the test).

Chris
  • 419
  • 5
  • 13
1

Dependency injection works well in this case.

When constructing a Server object:

Server new_server_instance;

you construct the real implementation of a background thread:

std::thread background_thread_ = thread([this] {ProcessRequestsInQueue();});

With the current implementation you can not control when the background thread will process which request.

Instead, pass the background thread as a dependency:

class Server {
     Server(std::thread background_thread): background_thread_(background_thread) {}
}

Server new_server_instance(background_thread);

This way in a unit test you can construct a server passing a fake background thread, which will process requests in any order you want. Then you can construct multiple tests with different fake background threads testing all potential race conditions.

For more info, read:

https://en.wikipedia.org/wiki/Dependency_injection

What is dependency injection?

mercury0114
  • 1,341
  • 2
  • 15
  • 29