Skip to content

Commit

Permalink
Multiple fixes and adaption for VS test explorer (#255)
Browse files Browse the repository at this point in the history
* Multiple updates include:

* Adding support for Visual Studio Test Explorer via Google Test Adapter so the tests can be run/debugged in VS
* Fixing multiple warnings - some have major impact on Windows platform/compiler which doesn't automatically zero structs
* Fixing the flaky and explicitly broken unit tests

* Attempting to fix the Thread test TSAN run failures

* Fixing pthread build on Windows WSL2. Fixing TSAN thread test.

* clang-format the include file as the version of the tooling has changed on the backend
  • Loading branch information
MushMal committed May 21, 2024
1 parent e96735d commit acc14a2
Show file tree
Hide file tree
Showing 17 changed files with 88 additions and 114 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -207,5 +207,6 @@ install(
DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig")

if(BUILD_TEST)
enable_testing()
add_subdirectory(tst)
endif()
3 changes: 0 additions & 3 deletions src/client/src/Stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -1893,9 +1893,6 @@ STATUS putEventMetadata(PKinesisVideoStream pKinesisVideoStream, UINT32 event, P
UINT32 packagedSizes[MAX_FRAGMENT_METADATA_COUNT] = {0};
PSerializedMetadata serializedNodes[MAX_FRAGMENT_METADATA_COUNT] = {0};
UINT8 neededNodes = 0;
PSerializedMetadata pExistingSerializedMetadata;
StackQueueIterator iterator;
UINT64 data;

CHK(pKinesisVideoStream != NULL, STATUS_NULL_ARG);
pKinesisVideoClient = pKinesisVideoStream->pKinesisVideoClient;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,7 @@ typedef struct __Frame {
*
* The initializer will zero all the fields and set the EoFr flag in flags.
*/
#define EOFR_FRAME_INITIALIZER \
{ \
FRAME_CURRENT_VERSION, 0, FRAME_FLAG_END_OF_FRAGMENT, 0, 0, 0, 0, NULL, 0 \
}
#define EOFR_FRAME_INITIALIZER {FRAME_CURRENT_VERSION, 0, FRAME_FLAG_END_OF_FRAGMENT, 0, 0, 0, 0, NULL, 0}

/**
* The representation of mkv video element
Expand Down
2 changes: 1 addition & 1 deletion src/state/src/State.c
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ STATUS checkForStateTransition(PStateMachine pStateMachine, PBOOL pTransitionRea
ENTERS();
STATUS retStatus = STATUS_SUCCESS;
PStateMachineState pState = NULL;
UINT64 nextState, time;
UINT64 nextState;
UINT64 customData;
PStateMachineImpl pStateMachineImpl = (PStateMachineImpl) pStateMachine;
UINT64 errorStateTransitionWaitTime = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/utils/src/ExponentialBackoffRetryStrategy.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ STATUS normalizeExponentialBackoffConfig(PExponentialBackoffRetryStrategyConfig
pExponentialBackoffRetryStrategyConfig->jitterType, pExponentialBackoffRetryStrategyConfig->jitterFactor);

// Seed rand to generate random number for jitter
SRAND(GETTIME());
SRAND((UINT32) GETTIME());

CleanUp:
LEAVES();
Expand Down
4 changes: 3 additions & 1 deletion tst/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
cmake_minimum_required(VERSION 3.6.3)
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake;${CMAKE_MODULE_PATH}")
include(Utilities)
include(GoogleTest)
project(pic_project_tests LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread")

if (OPEN_SRC_INSTALL_PREFIX)
find_package(GTest REQUIRED PATHS ${OPEN_SRC_INSTALL_PREFIX})
Expand All @@ -33,4 +35,4 @@ if(UNIX AND NOT APPLE)
target_link_libraries(kvspic_test rt)
endif()

add_test(${PROJECT_NAME} ${PROJECT_NAME})
add_test(TARGET kvspic_test EXTRA_ARGS --arg1 "${CMAKE_CURRENT_SOURCE_DIR}")
5 changes: 3 additions & 2 deletions tst/client/ClientApiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ TEST_F(ClientApiTest, kinesisVideoClientCreateSync_Valid_Timeout)
TEST_F(ClientApiTest, kinesisVideoClientCreateSync_Store_Alloc)
{
CLIENT_HANDLE clientHandle, failedClientHandle;
UNUSED_PARAM(failedClientHandle);

mClientSyncMode = TRUE;
mDeviceInfo.clientInfo.createClientTimeout = 20 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND;
Expand Down Expand Up @@ -472,7 +473,7 @@ TEST_F(ClientApiTest, getKinesisVideoMetrics_Valid)
ClientMetrics kinesisVideoClientMetrics;
// Testing all versions
for(UINT64 i = 0; i <= CLIENT_METRICS_CURRENT_VERSION; i++) {
kinesisVideoClientMetrics.version = i;
kinesisVideoClientMetrics.version = (UINT32) i;
EXPECT_EQ(STATUS_SUCCESS, getKinesisVideoMetrics(mClientHandle, &kinesisVideoClientMetrics));
}
}
Expand Down Expand Up @@ -505,7 +506,7 @@ TEST_F(ClientApiTest, getStreamMetrics_Valid)
EXPECT_EQ(STATUS_SUCCESS, createKinesisVideoStream(mClientHandle, &mStreamInfo, &streamHandle));
// Testing all versions
for(UINT64 i = 0; i <= STREAM_METRICS_CURRENT_VERSION; i++) {
streamMetrics.version = i;
streamMetrics.version = (UINT32) i;
EXPECT_EQ(STATUS_SUCCESS, getKinesisVideoStreamMetrics(streamHandle, &streamMetrics));
}

Expand Down
3 changes: 2 additions & 1 deletion tst/client/ClientTestFixture.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ class ClientTestBase : public ::testing::Test {

void initTestMembers()
{
UINT32 logLevel = 0;
UINT32 logLevel = LOG_LEVEL_WARN;
STATUS retStatus = STATUS_SUCCESS;
auto logLevelStr = GETENV("AWS_KVS_LOG_LEVEL");
if (logLevelStr != NULL) {
Expand All @@ -382,6 +382,7 @@ class ClientTestBase : public ::testing::Test {
// Zero things out
mClientHandle = INVALID_CLIENT_HANDLE_VALUE;
MEMSET(&mDeviceInfo, 0x00, SIZEOF(DeviceInfo));
MEMSET(&mStreamInfo, 0x00, SIZEOF(StreamInfo));
MEMSET(&mClientHandle, 0x00, SIZEOF(ClientCallbacks));
MEMSET(mDeviceName, 0x00, MAX_DEVICE_NAME_LEN);
MEMSET(mStreamName, 0x00, MAX_STREAM_NAME_LEN);
Expand Down
17 changes: 8 additions & 9 deletions tst/client/StreamApiFunctionalityTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ TEST_F(StreamApiFunctionalityTest, putFrame_DescribeStreamNotExisting_CreateNotA

TEST_F(StreamApiFunctionalityTest, streamFormatChange_stateCheck)
{
UINT32 i;
BYTE tempBuffer[10000];
UINT64 timestamp;
Frame frame;
UINT32 i = 0;
BYTE tempBuffer[10000] = {0};
UINT64 timestamp = 0;
Frame frame = {0};
BYTE cpd[100];

CreateStream();
Expand Down Expand Up @@ -161,13 +161,12 @@ TEST_F(StreamApiFunctionalityTest, streamFormatChange_stateCheck)

TEST_F(StreamApiFunctionalityTest, setNalAdaptionFlags_stateCheck)
{
UINT32 i;
BYTE tempBuffer[10000];
UINT64 timestamp;
Frame frame;
UINT32 i = 0;
BYTE tempBuffer[10000] = {0};
UINT64 timestamp = 0;
Frame frame = {0};
UINT32 nalFlags = NAL_ADAPTATION_ANNEXB_NALS | NAL_ADAPTATION_ANNEXB_CPD_NALS;
PKinesisVideoStream pKinesisVideoStream;
PStreamMkvGenerator pStreamMkvGenerator;

// The default would be flags NONE
CreateStream();
Expand Down
4 changes: 2 additions & 2 deletions tst/mkvgen/MkvgenApiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ TEST_F(MkvgenApiTest, mkvgenGenerateTagsChain_OriginalAPICheck)
TEST_F(MkvgenApiTest, mkvgenGenerateTagsChain_MaxStringsCheck)
{
UINT32 size = 100000;
srand(GETTIME());
SRAND((UINT32) GETTIME());
CHAR tagName[MKV_MAX_TAG_NAME_LEN + 2] = {0};
CHAR tagValue[MKV_MAX_TAG_VALUE_LEN + 2] = {0};
CHAR temp;
Expand Down Expand Up @@ -529,7 +529,7 @@ TEST_F(MkvgenApiTest, mkvgenIncreaseTagsTagSize_FunctionalityTest)
{
UINT32 size = 1000, randomSize = 0, encodedSize = 0;
PBYTE tagsMkvHolder = (PBYTE) MEMCALLOC(1, size);
srand(GETTIME());
SRAND((UINT32) GETTIME());

EXPECT_EQ(STATUS_INVALID_ARG, mkvgenIncreaseTagsTagSize(NULL, 0));

Expand Down
9 changes: 5 additions & 4 deletions tst/utils/ExponentialBackoffUtilsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ class ExponentialBackoffUtilsTest : public UtilTestBase {
EXPECT_EQ(expectedExponentialBackoffStatus, pExponentialBackoffRetryStrategyState->status);
// Record the actual wait time for validation
EXPECT_EQ(retryWaitTimeToVerify, pExponentialBackoffRetryStrategyState->lastRetryWaitTime);
EXPECT_TRUE(inRange(retryWaitTimeToVerify/HUNDREDS_OF_NANOS_IN_A_MILLISECOND, acceptableWaitTimeRange));
EXPECT_TRUE(inRange((DOUBLE) retryWaitTimeToVerify / HUNDREDS_OF_NANOS_IN_A_MILLISECOND, acceptableWaitTimeRange));

UINT64 lastRetrySystemTimeMilliSec = pExponentialBackoffRetryStrategyState->lastRetrySystemTime/(HUNDREDS_OF_NANOS_IN_A_MILLISECOND * 1.0);
UINT64 lastRetrySystemTimeMilliSec =
(UINT64) ((DOUBLE) pExponentialBackoffRetryStrategyState->lastRetrySystemTime / (HUNDREDS_OF_NANOS_IN_A_MILLISECOND * 1.0));
UINT64 currentTimeMilliSec = GETTIME()/HUNDREDS_OF_NANOS_IN_A_MILLISECOND;
UINT64 diffInMilliSec = currentTimeMilliSec - lastRetrySystemTimeMilliSec;

Expand Down Expand Up @@ -245,7 +246,7 @@ TEST_F(ExponentialBackoffUtilsTest, testExponentialBackoffBlockingWait_Bounded)

UINT64 retryWaitTimeToVerify;
UINT32 actualRetryCount = 0;
for (int retryCount = 0; retryCount < maxTestRetryCount; retryCount++) {
for (UINT32 retryCount = 0; retryCount < maxTestRetryCount; retryCount++) {
retryWaitTimeToVerify = 0;
EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryStrategyWaitTime(&kvsRetryStrategy, &retryWaitTimeToVerify));
EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryCount(&kvsRetryStrategy, &actualRetryCount));
Expand Down Expand Up @@ -302,7 +303,7 @@ TEST_F(ExponentialBackoffUtilsTest, testExponentialBackoffBlockingWait_FullJitte

UINT64 retryWaitTimeToVerify;
UINT32 actualRetryCount = 0;
for (int retryCount = 0; retryCount < maxTestRetryCount; retryCount++) {
for (UINT32 retryCount = 0; retryCount < maxTestRetryCount; retryCount++) {
retryWaitTimeToVerify = 0;
EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryStrategyWaitTime(&kvsRetryStrategy, &retryWaitTimeToVerify));
EXPECT_EQ(STATUS_SUCCESS, getExponentialBackoffRetryCount(&kvsRetryStrategy, &actualRetryCount));
Expand Down
6 changes: 3 additions & 3 deletions tst/utils/StackQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ TEST_F(StackQueueFunctionalityTest, StackQueueEnqueueAfterValidation)
UINT64 checkIndex[5];
UINT64 indexShift[5] = {0};

srand(GETTIME());
SRAND((UINT32) GETTIME());
// Create the list
EXPECT_EQ(STATUS_SUCCESS, stackQueueCreate(&pStackQueue));

Expand All @@ -195,7 +195,7 @@ TEST_F(StackQueueFunctionalityTest, StackQueueEnqueueAfterValidation)

for (UINT64 j = 0; j < 5; j++) {
checkIndex[j] = rand()%count;
EXPECT_EQ(STATUS_SUCCESS, stackQueueEnqueueAfterIndex(pStackQueue, checkIndex[j], j));
EXPECT_EQ(STATUS_SUCCESS, stackQueueEnqueueAfterIndex(pStackQueue, (UINT32) checkIndex[j], j));
}

for (UINT64 init = 0; init < 5; init++) {
Expand All @@ -209,7 +209,7 @@ TEST_F(StackQueueFunctionalityTest, StackQueueEnqueueAfterValidation)
}

for (UINT64 k = 0; k < 5; k++) {
EXPECT_EQ(STATUS_SUCCESS, stackQueueGetAt(pStackQueue, checkIndex[k] + indexShift[k], &data));
EXPECT_EQ(STATUS_SUCCESS, stackQueueGetAt(pStackQueue, (UINT32) (checkIndex[k] + indexShift[k]), &data));
EXPECT_EQ(k, data);
}

Expand Down
2 changes: 1 addition & 1 deletion tst/utils/StringSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ auto randStr = [](SIZE_T n) -> std::string {
std::string s;

// not ideal, but it's important to always randomize rand
SRAND(GETTIME());
SRAND((UINT32) GETTIME());
for (int i = 0; i < n; i++) {
// 8 bits contain 255 characters + 1 null character
s += (CHAR)(RAND() % 255) + 1;
Expand Down
Loading

0 comments on commit acc14a2

Please sign in to comment.