[SERVER-69521] Add a feature like the jstest `assert.soon()` to C++ unit tests Created: 08/Sep/22  Updated: 05/Dec/22

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Jordi Olivares Provencio Assignee: Backlog - Service Architecture
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Service Arch
Participants:

 Description   

There are some C++ unit tests that need to check that a set of values eventually converge to another set of values. One example is the ticketholder test that needs to wait until a certain condition is met. In other suites they've had to implement their own version of assertSoon in their fixtures.

Considering that there is a method in the JS tests called assert.soon() that checks the given criteria repeatedly until it passes, would it be possible to add something similar to C++ unit tests?



 Comments   
Comment by Billy Donahue [ 14/Oct/22 ]

I needed something like this for a test I'm writing.
I wrote it as an ASSERT_THAT matcher.

namespace m = unittest::match;
 
/**
 * Matcher for a function object `x`.
 *
 * True if `x()` evaluates to true.  Otherwise `x()` is re-evaluated repeatedly
 * with an exponential backoff, up to some limit, after which the match fails.
 */
template <typename M>
class SoonResultIs : public m::Matcher {
private:
    template <typename T>
    static auto _fibonacciGenerator() {
        return [seq = std::array{T{0}, T{1}}]() mutable {
            auto r = seq[0];
            seq[0] = seq[1];
            seq[1] = r + seq[0];
            return r;
        };
    }
 
public:
    explicit SoonResultIs(M&& m, int retries = 20) : _m{std::forward<M>(m)}, _retries{retries} {}
 
    std::string describe() const {
        return "SoonResultIs({})"_format(_m.describe());
    }
 
    template <typename X>
    m::MatchResult match(const X& x) const {
        auto fib = _fibonacciGenerator<Milliseconds>();
        m::MatchResult mr;
        for (int retries = _retries; retries--;) {
            if (mr = _m.match(x()); mr)
                return mr;
            sleepFor(fib());
        }
        return {false, "No result matched after {} retries{}"_format(_retries, mr.message())};
    }
 
private:
    M _m;
    int _retries;
};

Usage:

    ASSERT_THAT([&] { return getStats(executor); },
                SoonResultIs(IsExecStats(m::Eq(expThreads()), m::Eq(expClients()))));

Comment by Jason Chan [ 29/Sep/22 ]

Putting into backlog until someone flags this as something more urgent as service architecture is still unclear on the motivation and urgency.

Comment by Jason Chan [ 23/Sep/22 ]

jordi.olivares-provencio@mongodb.com Could you clarify why the usage of failpoints may be discouraged here? This is new to most people on Service Arch and we would like to know what the limitations are before we introduce a new API. Ideally, we would like to standardize the synchronization methods in unit testing across the codebase.

Comment by Jordi Olivares Provencio [ 13/Sep/22 ]

jason.chan@mongodb.com I'll use the ticketholder tests as an example of why we would want this.

Metrics returned by serverStatus are usually implemented using atomics modified with relaxed ordering and loading. This means that in some cases numbers returned will be wrong for brief periods of time. There's also the chance of two threads being scheduled such that if one updates metrics and another reads (test validator), the one that reads will get stale data sometimes if we don't want to add more synchronization between the two.

If you believe this is better to consider on a case by case basis to avoid misuse in unit tests I will be satisfied as I'm also in the determinism camp. But I've seen it in a few places and thought that maybe we would want to provide one way of doing it rather than having to re-implement it when needed.

Comment by Jason Chan [ 12/Sep/22 ]

jordi.olivares-provencio@mongodb.com, we are concerned about introducing this utility specifically for unit tests because from our perspective, unit tests should not have to rely on "Eventual consistency" and should be expected to be deterministic. However, we are more open to the idea of adding this to our C++ integration tests if that is something StoreEx would consider.

Comment by Billy Donahue [ 08/Sep/22 ]

Please not as a macro.
I rewrote the description here to leave the implementation unspecified.

Generated at Thu Feb 08 06:13:41 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.