Skip to content

Commit

Permalink
ZOOKEEPER-4479: C tests: Avoid some jitter which results in flaky tests
Browse files Browse the repository at this point in the history
With these commits, the various tests in `TestOperations.cc` set the `last_sent` and `last_recv` fields of the "force-connected" ZooKeeper handle to deterministic values (which match the initial value of the time mock).

When that value is not specified, these fields get set to the "real" time at the end of the  `forceConnected` function, which can be sufficiently different from the value held in the mock for some of the tests to fail.

Author: Damien Diederen <ddiederen@apache.org>

Reviewers: Enrico Olivelli <eolivelli@apache.org>

Closes #1827 from ztzg/ZOOKEEPER-4479-flaky-c-test-operations
  • Loading branch information
ztzg authored and eolivelli committed Mar 1, 2022
1 parent a5b6c38 commit 640b6dd
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 15 deletions.
22 changes: 11 additions & 11 deletions zookeeper-client/zookeeper-client-c/tests/TestOperations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture
zh=zookeeper_init("localhost:2121",watcher,10000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
// simulate connected state
forceConnected(zh);
forceConnected(zh, &timeMock.tv);

int fd=0;
int interest=0;
Expand Down Expand Up @@ -173,7 +173,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture
zh=zookeeper_init("localhost:2121",watcher,10000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
// simulate connected state
forceConnected(zh);
forceConnected(zh, &timeMock.tv);

int fd=0;
int interest=0;
Expand Down Expand Up @@ -216,7 +216,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture
zh=zookeeper_init("localhost:2121",watcher,10000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
// simulate connected state
forceConnected(zh);
forceConnected(zh, &timeMock.tv);

int fd=0;
int interest=0;
Expand Down Expand Up @@ -267,7 +267,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture
zh=zookeeper_init("localhost:1234",watcher,TIMEOUT*1000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
// simulate connected state
forceConnected(zh);
forceConnected(zh, &timeMock.tv);

int fd=0;
int interest=0;
Expand Down Expand Up @@ -344,7 +344,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture
zh=zookeeper_init("localhost:1234",watcher,TIMEOUT*1000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
// simulate connected state
forceConnected(zh);
forceConnected(zh, &timeMock.tv);

int fd=0;
int interest=0;
Expand Down Expand Up @@ -379,7 +379,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture
zh=zookeeper_init("localhost:1234",watcher,TIMEOUT*1000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
// simulate connected state
forceConnected(zh);
forceConnected(zh, &timeMock.tv);

int fd=0;
int interest=0;
Expand Down Expand Up @@ -426,7 +426,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture
zh=zookeeper_init("localhost:1234",watcher,TIMEOUT*1000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
// simulate connected state
forceConnected(zh);
forceConnected(zh, &now.tv);

// queue up a request; keep it pending (as if the server is busy or has died)
AsyncGetOperationCompletion res1;
Expand Down Expand Up @@ -481,7 +481,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture

zh=zookeeper_init("localhost:2121",watcher,10000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
forceConnected(zh);
forceConnected(zh, &timeMock.tv);
zhandle_t* savezh=zh;

// issue a request
Expand Down Expand Up @@ -520,7 +520,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture

zh=zookeeper_init("localhost:2121",watcher,10000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
forceConnected(zh);
forceConnected(zh, &timeMock.tv);
zhandle_t* savezh=zh;

// will handle completion on request #1 and issue request #2 from it
Expand Down Expand Up @@ -594,7 +594,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture

zh=zookeeper_init("localhost:2121",watcher,10000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
forceConnected(zh);
forceConnected(zh, &timeMock.tv);
zhandle_t* savezh=zh;

// issue a multi request
Expand Down Expand Up @@ -639,7 +639,7 @@ class Zookeeper_operations : public CPPUNIT_NS::TestFixture

zh=zookeeper_init("localhost:2121",watcher,10000,TEST_CLIENT_ID,0,0);
CPPUNIT_ASSERT(zh!=0);
forceConnected(zh);
forceConnected(zh, &timeMock.tv);
zhandle_t* savezh=zh;

// these shall persist during the test
Expand Down
11 changes: 8 additions & 3 deletions zookeeper-client/zookeeper-client-c/tests/ZKMocks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ void ZookeeperServer::notifyBufferSent(const std::string& buffer){
addRecvResponse(e);
}

void forceConnected(zhandle_t* zh){
void forceConnected(zhandle_t* zh, const struct timeval *last_recv_send){
// simulate connected state
zh->state=ZOO_CONNECTED_STATE;

Expand All @@ -536,8 +536,13 @@ void forceConnected(zhandle_t* zh){
zh->addrs.next++;

zh->input_buffer=0;
gettimeofday(&zh->last_recv,0);
gettimeofday(&zh->last_send,0);
if (last_recv_send) {
zh->last_recv = *last_recv_send;
zh->last_send = *last_recv_send;
} else {
gettimeofday(&zh->last_recv,0);
gettimeofday(&zh->last_send,0);
}
}

void terminateZookeeperThreads(zhandle_t* zh){
Expand Down
2 changes: 1 addition & 1 deletion zookeeper-client/zookeeper-client-c/tests/ZKMocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
// sets internal zhandle_t members to certain values to simulate the client
// connected state. This function should only be used with the single-threaded
// Async API tests!
void forceConnected(zhandle_t* zh);
void forceConnected(zhandle_t* zh, const struct timeval *last_recv_send = NULL);

/**
* Gracefully terminates zookeeper I/O and completion threads.
Expand Down

0 comments on commit 640b6dd

Please sign in to comment.