Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/L1-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: L1 Unit Tests

on:
pull_request:
branches: [ develop,support/1.8 ]
branches: [ develop, support/1.8, copilot/improve-zone-null-checks ]
Comment thread
yogeswaransky marked this conversation as resolved.

env:
AUTOMATICS_UNAME: ${{ secrets.AUTOMATICS_UNAME }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/L2-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: L2 Integration Tests

on:
pull_request:
branches: [ develop, support/1.8 ]
branches: [ develop ]
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the pull_request.branches filter from [ develop, support/1.8 ] to only [ develop ] means L2 integration tests will no longer run for PRs targeting support/1.8. If support/1.8 PRs are still expected to be validated by L2 in this repo, this should be kept (or replaced with the desired set).

Suggested change
branches: [ develop ]
branches: [ develop, support/1.8 ]

Copilot uses AI. Check for mistakes.

env:
AUTOMATICS_UNAME: ${{ secrets.AUTOMATICS_UNAME }}
Expand Down
192 changes: 192 additions & 0 deletions source/test/xconf-client/xconfclientTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -762,3 +762,195 @@ TEST(STOPXCONFCLIENT, success_check)
{
EXPECT_EQ(T2ERROR_SUCCESS, stopXConfClient());
}

/*
* Tests for the getTimezone() timeZoneDST fallback path.
*
* getTimezone() is a static function exercised indirectly via appendRequestParams().
* The fallback to /opt/persistent/timeZoneDST is reached after the JSON-based
* primary path exhausts its 10 retries without finding a timezone.
*
* Preconditions for each test:
* - All getParameterValue calls succeed (6 calls with dummy values).
* - getBuildType succeeds: fopen(DEVICE_PROPERTIES) returns a fake handle, fscanf
* fills "BUILD_TYPE=PROD" and then returns EOF.
* - getTimezone CPU_ARCH read: fopen(DEVICE_PROPERTIES) returns NULL (no CPU_ARCH).
* - getTimezone JSON loop: fopen("/opt/output.json") returns NULL (10 retries fail).
* After these preconditions the timeZoneDST fallback is exercised.
*/
#if !defined(ENABLE_RDKB_SUPPORT) && !defined(ENABLE_RDKC_SUPPORT)

// Helper to set up the mocks needed to reach the timeZoneDST fallback in getTimezone().
// Requires g_fileIOMock and m_xconfclientMock to be set.
static void SetupReachTimeZoneDSTFallback(FILE* devicePropsHandle)
{
using namespace testing;

// All 6 getParameterValue calls succeed with dummy values.
EXPECT_CALL(*m_xconfclientMock, getParameterValue(_, _))
.Times(AtLeast(6))
.WillRepeatedly(Invoke([](const char*, char** val) -> T2ERROR {
*val = strdup("dummy");
return T2ERROR_SUCCESS;
}));

// getBuildType: fopen(DEVICE_PROPERTIES) -> devicePropsHandle, fscanf fills
// "BUILD_TYPE=PROD" and breaks (no EOF call), fclose returns 0.
EXPECT_CALL(*g_fileIOMock, fopen(StrEq("/etc/device.properties"), _))
.WillOnce(Return(devicePropsHandle)) // getBuildType
.WillOnce(Return(nullptr)); // getTimezone CPU_ARCH read

// getBuildType breaks out of its fscanf loop as soon as it finds "BUILD_TYPE",
// so fscanf is called exactly once.
EXPECT_CALL(*g_fileIOMock, fscanf(devicePropsHandle, _, _))
.WillOnce(Invoke([](FILE*, const char*, va_list args) -> int {
char* buf = va_arg(args, char*);
strncpy(buf, "BUILD_TYPE=PROD", 254);
buf[254] = '\0';
return 1;
}));

Comment on lines +805 to +812
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetupReachTimeZoneDSTFallback configures fscanf(devicePropsHandle, ...) to be called twice (second call returns EOF). In getBuildType() the loop breaks as soon as "BUILD_TYPE" is found, so with the mocked buffer containing "BUILD_TYPE=PROD" it will typically call fscanf only once. This makes the expectation overspecified and can cause these tests to fail; adjust the expectation to match a single call or allow additional calls only when the loop continues (e.g., by using a repeated EOF fallback instead of requiring a second call).

Copilot uses AI. Check for mistakes.
EXPECT_CALL(*g_fileIOMock, fclose(devicePropsHandle))
.WillOnce(Return(0));

// getTimezone JSON loop: fopen("/opt/output.json") returns NULL all 10 retries.
EXPECT_CALL(*g_fileIOMock, fopen(StrEq("/opt/output.json"), _))
.Times(10)
.WillRepeatedly(Return(nullptr));
}

// Test 1: fopen("/opt/persistent/timeZoneDST") returns NULL.
// The fallback cannot open the file; timezone stays NULL; appendRequestParams returns FAILURE.
TEST_F(xconfclientTestFixture, getTimezone_timeZoneDST_fopen_null)
{
FILE* devicePropsHandle = reinterpret_cast<FILE*>(0x1234);
PREVENT_GTEST_LOGGING_DEADLOCK();
SetupReachTimeZoneDSTFallback(devicePropsHandle);

EXPECT_CALL(*g_fileIOMock, fopen(StrEq("/opt/persistent/timeZoneDST"), _))
.WillOnce(Return(nullptr));

CURLU* requestURL = curl_url();
curl_url_set(requestURL, CURLUPART_URL,
"https://mockxconf:50050/loguploader/getT2DCMSettings", 0);
EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL));
curl_free(requestURL);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestURL is a CURLU* created by curl_url(), which should be released with curl_url_cleanup(). Using curl_free() on a CURLU* is not the documented cleanup API and can lead to leaks or undefined behavior.

Copilot uses AI. Check for mistakes.
requestURL = NULL;
}

// Test 2: ftell returns -1 (unreadable/error).
// The fallback opens the file but ftell fails; file is closed; timezone stays NULL.
TEST_F(xconfclientTestFixture, getTimezone_timeZoneDST_ftell_negative)
{
FILE* devicePropsHandle = reinterpret_cast<FILE*>(0x1234);
FILE* tzFile = reinterpret_cast<FILE*>(0x5678);
PREVENT_GTEST_LOGGING_DEADLOCK();
SetupReachTimeZoneDSTFallback(devicePropsHandle);

EXPECT_CALL(*g_fileIOMock, fopen(StrEq("/opt/persistent/timeZoneDST"), _))
.WillOnce(Return(tzFile));
EXPECT_CALL(*g_fileIOMock, fseek(tzFile, 0, SEEK_END))
.WillOnce(Return(0));
EXPECT_CALL(*g_fileIOMock, ftell(tzFile))
.WillOnce(Return(-1L));
EXPECT_CALL(*g_fileIOMock, fclose(tzFile))
.WillOnce(Return(0));

CURLU* requestURL = curl_url();
curl_url_set(requestURL, CURLUPART_URL,
"https://mockxconf:50050/loguploader/getT2DCMSettings", 0);
EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL));
curl_free(requestURL);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestURL is a CURLU* created by curl_url(), which should be released with curl_url_cleanup(). Using curl_free() on a CURLU* is not the documented cleanup API and can lead to leaks or undefined behavior.

Copilot uses AI. Check for mistakes.
requestURL = NULL;
}

// Test 3: ftell returns 0 (empty file).
// The fallback opens the file but it is empty; file is closed; timezone stays NULL.
TEST_F(xconfclientTestFixture, getTimezone_timeZoneDST_empty_file)
{
FILE* devicePropsHandle = reinterpret_cast<FILE*>(0x1234);
FILE* tzFile = reinterpret_cast<FILE*>(0x5678);
PREVENT_GTEST_LOGGING_DEADLOCK();
SetupReachTimeZoneDSTFallback(devicePropsHandle);

EXPECT_CALL(*g_fileIOMock, fopen(StrEq("/opt/persistent/timeZoneDST"), _))
.WillOnce(Return(tzFile));
EXPECT_CALL(*g_fileIOMock, fseek(tzFile, 0, SEEK_END))
.WillOnce(Return(0));
EXPECT_CALL(*g_fileIOMock, ftell(tzFile))
.WillOnce(Return(0L));
EXPECT_CALL(*g_fileIOMock, fclose(tzFile))
.WillOnce(Return(0));

CURLU* requestURL = curl_url();
curl_url_set(requestURL, CURLUPART_URL,
"https://mockxconf:50050/loguploader/getT2DCMSettings", 0);
EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL));
curl_free(requestURL);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestURL is a CURLU* created by curl_url(), which should be released with curl_url_cleanup(). Using curl_free() on a CURLU* is not the documented cleanup API and can lead to leaks or undefined behavior.

Copilot uses AI. Check for mistakes.
requestURL = NULL;
}

// Test 4: ftell returns a value exceeding the 256-byte limit.
// The fallback rejects oversized files; file is closed; timezone stays NULL.
TEST_F(xconfclientTestFixture, getTimezone_timeZoneDST_file_too_large)
{
FILE* devicePropsHandle = reinterpret_cast<FILE*>(0x1234);
FILE* tzFile = reinterpret_cast<FILE*>(0x5678);
PREVENT_GTEST_LOGGING_DEADLOCK();
SetupReachTimeZoneDSTFallback(devicePropsHandle);

EXPECT_CALL(*g_fileIOMock, fopen(StrEq("/opt/persistent/timeZoneDST"), _))
.WillOnce(Return(tzFile));
EXPECT_CALL(*g_fileIOMock, fseek(tzFile, 0, SEEK_END))
.WillOnce(Return(0));
EXPECT_CALL(*g_fileIOMock, ftell(tzFile))
.WillOnce(Return(512L));
EXPECT_CALL(*g_fileIOMock, fclose(tzFile))
.WillOnce(Return(0));

CURLU* requestURL = curl_url();
curl_url_set(requestURL, CURLUPART_URL,
"https://mockxconf:50050/loguploader/getT2DCMSettings", 0);
EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL));
curl_free(requestURL);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestURL is a CURLU* created by curl_url(), which should be released with curl_url_cleanup(). Using curl_free() on a CURLU* is not the documented cleanup API and can lead to leaks or undefined behavior.

Copilot uses AI. Check for mistakes.
requestURL = NULL;
}

// Test 5: Valid timezone string read from /opt/persistent/timeZoneDST.
// The fallback reads a valid timezone; appendRequestParams returns SUCCESS.
TEST_F(xconfclientTestFixture, getTimezone_timeZoneDST_valid_timezone)
{
FILE* devicePropsHandle = reinterpret_cast<FILE*>(0x1234);
FILE* tzFile = reinterpret_cast<FILE*>(0x5678);
PREVENT_GTEST_LOGGING_DEADLOCK();
SetupReachTimeZoneDSTFallback(devicePropsHandle);

EXPECT_CALL(*g_fileIOMock, fopen(StrEq("/opt/persistent/timeZoneDST"), _))
.WillOnce(Return(tzFile));
EXPECT_CALL(*g_fileIOMock, fseek(tzFile, 0, SEEK_END))
.WillOnce(Return(0));
EXPECT_CALL(*g_fileIOMock, ftell(tzFile))
.WillOnce(Return(10L));
EXPECT_CALL(*g_fileIOMock, fseek(tzFile, 0, SEEK_SET))
.WillOnce(Return(0));
// fscanf fills the zone buffer with "US/Eastern" and then signals end-of-file.
EXPECT_CALL(*g_fileIOMock, fscanf(tzFile, _, _))
.WillOnce(::testing::Invoke([](FILE*, const char*, va_list args) -> int {
char* buf = va_arg(args, char*);
strncpy(buf, "US/Eastern", 10);
buf[10] = '\0';
return 1;
}))
.WillOnce(Return(0));
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the second fscanf call "signals end-of-file", but the mock returns 0. In getTimezone() the loop condition is fscanf(...) == 1, so returning EOF would better reflect end-of-file and keep the comment accurate (or update the comment to match the 0 return).

Suggested change
.WillOnce(Return(0));
.WillOnce(Return(EOF));

Copilot uses AI. Check for mistakes.
EXPECT_CALL(*g_fileIOMock, fclose(tzFile))
.WillOnce(Return(0));

CURLU* requestURL = curl_url();
curl_url_set(requestURL, CURLUPART_URL,
"https://mockxconf:50050/loguploader/getT2DCMSettings", 0);
EXPECT_EQ(T2ERROR_SUCCESS, appendRequestParams(requestURL));
curl_free(requestURL);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestURL is a CURLU* created by curl_url(), which should be released with curl_url_cleanup(). Using curl_free() on a CURLU* is not the documented cleanup API and can lead to leaks or undefined behavior.

Suggested change
curl_free(requestURL);
curl_url_cleanup(requestURL);

Copilot uses AI. Check for mistakes.
requestURL = NULL;
}

#endif /* !defined(ENABLE_RDKB_SUPPORT) && !defined(ENABLE_RDKC_SUPPORT) */
Loading