RDKEMW-12350: Add unit tests for timeZoneDST fallback path in getTimezone()#329
RDKEMW-12350: Add unit tests for timeZoneDST fallback path in getTimezone()#329Copilot wants to merge 6 commits intocopilot/improve-zone-null-checksfrom
Conversation
Agent-Logs-Url: https://github.com/rdkcentral/telemetry/sessions/2b39401a-79a0-4d6a-a81f-523a6997a872 Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rdkcentral/telemetry/sessions/7f6ea2d9-36fa-4120-a7dc-e7bfe71e9f8a Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Google Test coverage for the getTimezone() fallback path that reads /opt/persistent/timeZoneDST (reached after the JSON retry loop), exercised indirectly via appendRequestParams().
Changes:
- Add 5 new unit tests covering
timeZoneDSTfallback failure modes and a success case. - Add a shared test helper to drive execution into the fallback path.
- Adjust GitHub Actions PR branch filters for L1/L2 workflows.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
source/test/xconf-client/xconfclientTest.cpp |
Adds tests and a helper to force getTimezone() into the /opt/persistent/timeZoneDST fallback path. |
.github/workflows/L1-tests.yml |
Updates which target branches trigger the L1 workflow on pull requests. |
.github/workflows/L2-tests.yml |
Updates which target branches trigger the L2 workflow on pull requests. |
| curl_url_set(requestURL, CURLUPART_URL, | ||
| "https://mockxconf:50050/loguploader/getT2DCMSettings", 0); | ||
| EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL)); | ||
| curl_free(requestURL); |
There was a problem hiding this comment.
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.
| curl_url_set(requestURL, CURLUPART_URL, | ||
| "https://mockxconf:50050/loguploader/getT2DCMSettings", 0); | ||
| EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL)); | ||
| curl_free(requestURL); |
There was a problem hiding this comment.
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.
| curl_url_set(requestURL, CURLUPART_URL, | ||
| "https://mockxconf:50050/loguploader/getT2DCMSettings", 0); | ||
| EXPECT_EQ(T2ERROR_SUCCESS, appendRequestParams(requestURL)); | ||
| curl_free(requestURL); |
There was a problem hiding this comment.
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.
| curl_free(requestURL); | |
| curl_url_cleanup(requestURL); |
| buf[10] = '\0'; | ||
| return 1; | ||
| })) | ||
| .WillOnce(Return(0)); |
There was a problem hiding this comment.
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).
| .WillOnce(Return(0)); | |
| .WillOnce(Return(EOF)); |
| 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; | ||
| })) | ||
| .WillOnce(Return(EOF)); | ||
|
|
There was a problem hiding this comment.
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).
| curl_url_set(requestURL, CURLUPART_URL, | ||
| "https://mockxconf:50050/loguploader/getT2DCMSettings", 0); | ||
| EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL)); | ||
| curl_free(requestURL); |
There was a problem hiding this comment.
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.
| curl_url_set(requestURL, CURLUPART_URL, | ||
| "https://mockxconf:50050/loguploader/getT2DCMSettings", 0); | ||
| EXPECT_EQ(T2ERROR_FAILURE, appendRequestParams(requestURL)); | ||
| curl_free(requestURL); |
There was a problem hiding this comment.
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.
| on: | ||
| pull_request: | ||
| branches: [ develop, support/1.8 ] | ||
| branches: [ develop ] |
There was a problem hiding this comment.
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).
| branches: [ develop ] | |
| branches: [ develop, support/1.8 ] |
Agent-Logs-Url: https://github.com/rdkcentral/telemetry/sessions/03d1c8d7-2587-443b-bbe5-50a78ff7f073 Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com>
…o break in getBuildType Agent-Logs-Url: https://github.com/rdkcentral/telemetry/sessions/d94df585-444b-4056-a136-bc1f7415c614 Co-authored-by: yogeswaransky <166126056+yogeswaransky@users.noreply.github.com>
.WillOnce(Return(EOF))fromSetupReachTimeZoneDSTFallbackinxconfclientTest.cpp—getBuildTypebreaks out of thefscanfloop as soon as it finds "BUILD_TYPE", sofscanfis only called once, not twice