RDKEMW-14462 : Move New L1 Test Cases to feature/dev_sprint_pli Branch#124
RDKEMW-14462 : Move New L1 Test Cases to feature/dev_sprint_pli Branch#124rekhap2kandhavelan wants to merge 18 commits intofeature/dev_sprint_plifrom
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…middleware-player-interface into feature/RDKEMW-14462
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
rekhap2kandhavelan
requested review from
Copilot and
narenr94
and removed request for
a team and
Copilot
There was a problem hiding this comment.
Pull request overview
This PR integrates a large set of new Level-1/unit test suites into test/utests, adds/updates per-suite CMake targets, and includes a handful of production-code hardening changes (null/bounds checks and small API additions) to support the new tests and platform coverage builds.
Changes:
- Added multiple new utest executables (new test runners + suite CMakeLists) across subtitles/subtec, externals, DRM, vendor, closed-captions, and GStreamer plugin areas.
- Updated utests build/coverage plumbing (
player_utest_add_cov), plus several suite CMakeLists link/include adjustments (including Apple vs Linux linking paths). - Production code tweaks for robustness/observability (e.g., null guards, new getters, bounds checks, base64 input validation).
Reviewed changes
Copilot reviewed 59 out of 152 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utests/tests/WebVttSubtecParserTests/WebVttSubtecParserPlayer.cpp | Add gtest runner main for WebVttSubtecParserTests |
| test/utests/tests/WebVttSubtecParserTests/CMakeLists.txt | Add CMake target for WebVttSubtecParserTests |
| test/utests/tests/WebvttSubtecDevInterfaceTests/WebvttSubtecDevInterfacePlayer.cpp | Add gtest runner main for WebvttSubtecDevInterfaceTests |
| test/utests/tests/WebvttSubtecDevInterfaceTests/CMakeLists.txt | Add CMake target for WebvttSubtecDevInterfaceTests |
| test/utests/tests/VendorTests/PlayerVendorRun.cpp | Add gtest runner main for VendorTests |
| test/utests/tests/VendorTests/CMakeLists.txt | Add CMake target for VendorTests |
| test/utests/tests/TtmlSubtecParser/TtmlSubtecParserPlayer.cpp | Add gtest runner main for TtmlSubtecParserTests |
| test/utests/tests/TtmlSubtecParser/CMakeLists.txt | Add CMake target for TtmlSubtecParserTests |
| test/utests/tests/TextStyleAttributes/CMakeLists.txt | Add mp4demux include + link additional sources + switch to coverage helper + PRIVATE linkage |
| test/utests/tests/SubtitleTests/SubtitleRun.cpp | Add gtest runner main for SubtitleTests |
| test/utests/tests/SubtitleTests/CMakeLists.txt | Add CMake target for SubtitleTests |
| test/utests/tests/SubtecTests/PlayerSubtecTests.cpp | Add gtest runner main for PlayerSubtecTests |
| test/utests/tests/SubtecTests/CMakeLists.txt | Add CMake target for PlayerSubtecTests |
| test/utests/tests/PluginsTests/CMakeLists.txt | Update GStreamer pkg-config usage, platform linking logic, and coverage helper |
| test/utests/tests/PlayerLogManagerTests/PlayerLogManagerRun.cpp | Add gtest runner main for PlayerLogManagerTests |
| test/utests/tests/PlayerLogManagerTests/CMakeLists.txt | Add CMake target for PlayerLogManagerTests |
| test/utests/tests/PlayerJsonObjectTests/PlayerJsonObjectRun.cpp | Add gtest runner main for PlayerJsonObjectTests |
| test/utests/tests/PlayerJsonObjectTests/CMakeLists.txt | Add CMake target for PlayerJsonObjectTests |
| test/utests/tests/PlayerIsoBmffTests/PlayerIsoBmffTestRunner.cpp | Add gtest runner main for PlayerIsoBmffTests |
| test/utests/tests/PlayerIsoBmffTests/CMakeLists.txt | Add CMake target for PlayerIsoBmffTests |
| test/utests/tests/PlayerExternalsTests/PlayerRFCFun.cpp | Add RFCSettings L1 tests |
| test/utests/tests/PlayerExternalsTests/PlayerMacros.h | Add local gtest helper macros header |
| test/utests/tests/PlayerExternalsTests/PlayerExternalUtilsFun.cpp | Add PlayerExternalUtils L1 tests |
| test/utests/tests/PlayerExternalsTests/PlayerExternalsRun.cpp | Add gtest runner main for PlayerExternalsTests |
| test/utests/tests/PlayerExternalsTests/PlayerExternalsInterfaceBaseFun.cpp | Add tests for PlayerExternalsInterfaceBase behaviors |
| test/utests/tests/PlayerExternalsTests/CMakeLists.txt | Add CMake target for PlayerExternalsTests |
| test/utests/tests/PlayerCommonTests/PlayerMetaDataFun.cpp | Add PlayerMetadata L1 tests |
| test/utests/tests/PlayerCommonTests/PlayerCommonRun.cpp | Add gtest runner main for PlayerCommonTests |
| test/utests/tests/PlayerCommonTests/gstPlayerTaskPoolFun.cpp | Add gstplayertaskpool L1 test |
| test/utests/tests/PlayerCommonTests/CMakeLists.txt | Add CMake target for PlayerCommonTests with gstreamer pkg-config linkage |
| test/utests/tests/OCDMSessionAdapter/CMakeLists.txt | Adjust includes, coverage helper, and PRIVATE linkage |
| test/utests/tests/OcdmBasicSessionAdapterTests/FunctionalTests.cpp | Refine matcher indentation, mock init ordering, and expectations |
| test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt | Expand includes, coverage helper, and PRIVATE linkage |
| test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp | Mark one injector/segment test as skipped |
| test/utests/tests/GstPluginsTests/GstwidevinedecryptorFun.cpp | Add gstwidevinedecryptor L1 tests |
| test/utests/tests/GstPluginsTests/GstverimatrixdecryptorFun.cpp | Add gstverimatrixdecryptor L1 tests |
| test/utests/tests/GstPluginsTests/GstPlugingRun.cpp | Add gtest runner main for GstPluginsTests |
| test/utests/tests/GstPluginsTests/GstplayreadydecryptorFun.cpp | Add gstplayreadydecryptor L1 tests |
| test/utests/tests/GstPluginsTests/GstclearkeydecryptorFun.cpp | Add gstclearkeydecryptor L1 tests |
| test/utests/tests/GstPluginsTests/GstcdmidecryptorFun.cpp | Add gstcdmidecryptor L1 tests |
| test/utests/tests/GstPluginsTests/CMakeLists.txt | Add CMake target for GstPluginsTests |
| test/utests/tests/GstPlugingSubtecTests/GstvipertransformFun.cpp | Add gstvipertransform L1 tests |
| test/utests/tests/GstPlugingSubtecTests/GstsubtecsinkFun.cpp | Add gstsubtecsink L1 tests |
| test/utests/tests/GstPlugingSubtecTests/Gstsubtecmp4transformFun.cpp | Add gstsubtecmp4transform L1 tests |
| test/utests/tests/GstPlugingSubtecTests/GstsubtecbinFun.cpp | Add gstsubtecbin L1 tests |
| test/utests/tests/GstPlugingSubtecTests/GstPlugingSubtecRun.cpp | Add gtest runner main for GstPlugingSubtecTests |
| test/utests/tests/GstPlugingSubtecTests/CMakeLists.txt | Add CMake target for GstPlugingSubtecTests |
| test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp | Adjust frame-step property mocking and video sink naming |
| test/utests/tests/GstPlayer/CMakeLists.txt | Add gstreamer pkg-config, include paths, GstUtils source, and platform link logic |
| test/utests/tests/GstHandlerControlTests/CMakeLists.txt | Switch coverage helper and PRIVATE linkage |
| test/utests/tests/FireBoltTests/FireBoltRun.cpp | Add gtest runner main for FireBoltTests |
| test/utests/tests/FireBoltTests/CMakeLists.txt | Add CMake target for FireBoltTests |
| test/utests/tests/DrmUrlTests/CMakeLists.txt | Update include paths and coverage helper + PRIVATE linkage |
| test/utests/tests/DrmTests/HlsDrmSessionInterfaceTests.cpp | Add HlsDrmSessionManager L1 tests |
| test/utests/tests/DrmTests/DrmTestsRun.cpp | Add gtest runner main for DrmTests |
| test/utests/tests/DrmTests/DrmMemorySystemTests.cpp | Add DrmMemorySystem L1 tests |
| test/utests/tests/DrmTests/DrmHLSTests.cpp | Add ProcessContentProtection tests for HLS key formats |
| test/utests/tests/DrmTests/CMakeLists.txt | Add CMake target for DrmTests |
| test/utests/tests/DrmSecureClient/CMakeLists.txt | Update platform linking (Apple framework vs Linux libs) + coverage helper |
| test/utests/tests/DrmOcdmTests/DrmOcdmTestsRun.cpp | Add gtest runner main for DrmOcdmTests |
| test/utests/tests/DrmOcdmTests/CMakeLists.txt | Add CMake target for DrmOcdmTests |
| test/utests/tests/DrmOcdm/DrmUtilsTests.cpp | Remove older DrmUtilsTests (moved/renamed into new suites) |
| test/utests/tests/DrmHelperTests/DrmHelperTestsRun.cpp | Add gtest runner main for DrmHelperTests |
| test/utests/tests/DrmHelperTests/CMakeLists.txt | Add CMake target for DrmHelperTests |
| test/utests/tests/DrmAes/DrmTestsRun.cpp | Add gtest runner main for DrmAes |
| test/utests/tests/DrmAes/CMakeLists.txt | Rename exec, add AES sources/includes, link OpenSSL::Crypto |
| test/utests/tests/ContentsecuritymanagerTests/PlayerContentSecurityRun.cpp | Add gtest runner main for ContentSecurityManagerTests |
| test/utests/tests/ContentsecuritymanagerTests/CMakeLists.txt | Add CMake target for ContentSecurityManagerTests |
| test/utests/tests/CMakeLists.txt | Register many new test subdirectories |
| test/utests/tests/ClosedCaptionsTests/PlayerCCTrackInfoTests.cpp | Add CCTrackInfo L1 test |
| test/utests/tests/ClosedCaptionsTests/PlayerCCManagerTests.cpp | Add gtest runner main for ClosedCaptionsTests |
| test/utests/tests/ClosedCaptionsTests/CMakeLists.txt | Add CMake target for closed-captions/subtec tests |
| test/utests/tests/Base64PLAYER/CMakeLists.txt | Switch coverage helper + PRIVATE linkage |
| test/utests/run.sh | Add gcda cleanup and update coverage baseline discovery logic |
| test/utests/mocks/MockPacketSender.h | Add PacketSender mock singleton |
| test/utests/mocks/MockHlsDrmSession.h | Add HlsDrmBase mock implementation |
| test/utests/mocks/MockDrmSession.h | Expand DrmSession mock methods and default keySystem |
| test/utests/mocks/MockDrmCallbacks.h | Add DrmCallbacks mock (new header) |
| test/utests/fakes/FakeSubtecConnecter.cpp | Add fake SubtecConnector API layer for tests |
| test/utests/fakes/FakeSocUtils.cpp | Add SocUtils::isGstSubtecEnabled() fake |
| test/utests/fakes/FakeSocInterface.cpp | Add null guards and implement IsVideoMaster query via GObject property |
| test/utests/fakes/FakePlayerJsonObject.cpp | Rework fake to provide cJSON stubs for tests |
| test/utests/fakes/FakeGStreamer.cpp | Add missing GType getters for new plugin tests |
| test/utests/fakes/FakeGstHandlerControl.cpp | Add guard for non-positive waits |
| test/utests/fakes/FakeCrypto.cpp | Add minimal EVP decrypt function stubs for tests |
| test/utests/fakes/FakeContentProtectionFireBolt.cpp | Add fake ContentProtectionFirebolt implementation |
| test/utests/CMakeLists.txt | Add player_utest_add_cov() helper function |
| playerisobmff/playerisobmffbuffer.h | Add getters and introduce getChunkedBox() API (with deprecated alias) |
| playerisobmff/playerisobmffbuffer.cpp | Implement new buffer getters and chunked-box API/alias |
| playerisobmff/playerisobmffbox.cpp | Harden SencIsoBmffBox::truncate() bounds checking |
| InterfacePlayerRDK.cpp | Null-guard socInterface when setting new-segment applied_rate |
| GstHandlerControl.cpp | Treat negative delay as invalid immediate timeout |
| externals/PlayerExternalsInterface.h | Add getters/setters on fake + add createInstance() (UBUNTU-guarded) |
| drm/helper/VerimatrixHelper.cpp | Add null guard + improve logging and setData calls |
| baseConversion/_base64.cpp | Add null/zero-length input guards for base64 encode/decode |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 152 changed files in this pull request and generated 20 comments.
Comments suppressed due to low confidence (1)
test/utests/tests/DrmAes/CMakeLists.txt:121
- Linking
OpenSSL::Cryptowhile also linking the sharedfakestarget (which provides stub implementations ofEVP_*APIs inFakeCrypto.cpp) can cause the fake symbols to interpose the real OpenSSL functions. That would make these tests not actually exercise OpenSSL/AES code paths. Consider removing the OpenSSL stubs for this target (or not linkingfakeshere), so the AES tests validate the real crypto implementation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 152 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ------------------ cJSON stubs ------------------ | ||
| extern "C" { | ||
| struct cJSON; // forward declaration | ||
|
|
||
| // Creation / Deletion | ||
| cJSON* cJSON_CreateObject() { return (cJSON*)calloc(1, sizeof(cJSON)); } | ||
| cJSON* cJSON_CreateArray() { return (cJSON*)calloc(1, sizeof(cJSON)); } | ||
| cJSON* cJSON_CreateString(const char* str) { | ||
| cJSON* item = (cJSON*)calloc(1, sizeof(cJSON)); | ||
| if (str) item->valuestring = strdup(str); | ||
| return item; | ||
| } | ||
| cJSON* cJSON_CreateNumber(double num) { | ||
| cJSON* item = (cJSON*)calloc(1, sizeof(cJSON)); | ||
| item->valuedouble = num; | ||
| item->valueint = (int)num; | ||
| return item; | ||
| } | ||
| cJSON* cJSON_CreateBool(int b) { | ||
| cJSON* item = (cJSON*)calloc(1, sizeof(cJSON)); | ||
| item->valueint = (b != 0); | ||
| return item; | ||
| } | ||
| void cJSON_Delete(cJSON* item) { | ||
| if (!item) return; | ||
| if (item->valuestring) free(item->valuestring); | ||
| free(item); | ||
| } | ||
| cJSON * cJSON_GetObjectItem(const cJSON * const object, const char * const string) { | ||
| if (!object || !string) return nullptr; | ||
| // For simplicity, assume object has a child with matching string | ||
| cJSON* child = object->child; | ||
| while (child) { | ||
| if (child->string && strcmp(child->string, string) == 0) { | ||
| return child; | ||
| } | ||
| child = child->next; |
There was a problem hiding this comment.
This fake cJSON implementation won’t compile as written: struct cJSON is only forward-declared, but the code uses sizeof(cJSON) and accesses members like item->valuestring, object->child, etc. Include the real cJSON header and use the real struct, or define a minimal struct cJSON with the fields you access (valuestring, valuedouble, valueint, string, child, next) so the stubs are well-formed.
| } | ||
|
|
||
| # "corrupt arc tag" | ||
| (find . -name "*.gcda" -print0 | xargs -0 rm) || true |
There was a problem hiding this comment.
Using find ... | xargs rm without xargs -r/--no-run-if-empty can invoke rm with no arguments (platform-dependent), producing noisy errors. Consider switching to find ... -delete (already used later) or add -r to xargs to avoid running when no files are found.
| (find . -name "*.gcda" -print0 | xargs -0 rm) || true | |
| (find . -name "*.gcda" -delete) || true |
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
| include_directories(${PLAYER_ROOT}/subtec/subtecparser) | ||
| include_directories(${PLAYER_ROOT}/playerLogManager) | ||
| include_directories(${PLAYER_ROOT}/playerjsonobject) | ||
| include_directories(${PLAYER_ROOT}/baseConversion) | ||
| include_directories(${PLAYER_ROOT}/playerisobmff) |
There was a problem hiding this comment.
include_directories(${PLAYER_ROOT}/playerjsonobject) uses the wrong directory casing. The repo directory is playerJsonObject/ (capital J/O), so this will break includes on case-sensitive filesystems (Linux). Update the path to ${PLAYER_ROOT}/playerJsonObject.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 152 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include "PacketSender.hpp" | ||
| #include <gmock/gmock.h> | ||
|
|
||
| class MockPacketSender : public PacketSender { | ||
| public: | ||
| static PacketSender* Instance() { | ||
| static MockPacketSender instance; | ||
| return &instance; | ||
| } | ||
|
|
||
| bool initSocket(const char* socket_path) override { | ||
| std::cout << "[Mocked] initSocket called for " << socket_path << std::endl; | ||
| return true; // Pretend success | ||
| } | ||
|
|
||
| bool Init() override { | ||
| std::cout << "[Mocked] Init called" << std::endl; | ||
| return true; |
There was a problem hiding this comment.
This mock uses std::cout but doesn’t include , and PacketSender.hpp also doesn’t include it. Add the proper standard header to avoid compile failures depending on transitive includes.
| // ------------------ cJSON stubs ------------------ | ||
| extern "C" { | ||
| struct cJSON; // forward declaration | ||
|
|
||
| // Creation / Deletion | ||
| cJSON* cJSON_CreateObject() { return (cJSON*)calloc(1, sizeof(cJSON)); } | ||
| cJSON* cJSON_CreateArray() { return (cJSON*)calloc(1, sizeof(cJSON)); } | ||
| cJSON* cJSON_CreateString(const char* str) { | ||
| cJSON* item = (cJSON*)calloc(1, sizeof(cJSON)); | ||
| if (str) item->valuestring = strdup(str); | ||
| return item; | ||
| } | ||
| cJSON* cJSON_CreateNumber(double num) { | ||
| cJSON* item = (cJSON*)calloc(1, sizeof(cJSON)); | ||
| item->valuedouble = num; | ||
| item->valueint = (int)num; | ||
| return item; | ||
| } | ||
| cJSON* cJSON_CreateBool(int b) { | ||
| cJSON* item = (cJSON*)calloc(1, sizeof(cJSON)); | ||
| item->valueint = (b != 0); | ||
| return item; | ||
| } | ||
| void cJSON_Delete(cJSON* item) { | ||
| if (!item) return; | ||
| if (item->valuestring) free(item->valuestring); | ||
| free(item); | ||
| } | ||
| cJSON * cJSON_GetObjectItem(const cJSON * const object, const char * const string) { | ||
| if (!object || !string) return nullptr; | ||
| // For simplicity, assume object has a child with matching string | ||
| cJSON* child = object->child; | ||
| while (child) { | ||
| if (child->string && strcmp(child->string, string) == 0) { | ||
| return child; | ||
| } | ||
| child = child->next; | ||
| } | ||
| return nullptr; | ||
| } | ||
|
|
||
| void cJSON_free(void *object){ | ||
| free(object); | ||
| } | ||
|
|
||
| // Type checking | ||
| int cJSON_IsArray(const cJSON* item) { (void)item; return 0; } | ||
| int cJSON_IsString(const cJSON* item) { (void)item; return 0; } | ||
| int cJSON_IsNumber(const cJSON* item) { (void)item; return 0; } | ||
| int cJSON_IsObject(const cJSON* item) { (void)item; return 0; } | ||
|
|
||
| // Printing | ||
| char* cJSON_Print(const cJSON* item) { (void)item; char* res = (char*)malloc(16); strcpy(res, "{}"); return res; } | ||
| char* cJSON_PrintUnformatted(const cJSON* item) { (void)item; char* res = (char*)malloc(8); strcpy(res, "{}"); return res; } | ||
|
|
||
| // Get string value | ||
| char* cJSON_GetStringValue(const cJSON* item) { | ||
| if (!item) return nullptr; | ||
| return item->valuestring; | ||
| } | ||
|
|
||
| // Parsing stub | ||
| cJSON* cJSON_Parse(const char* str) { (void)str; return cJSON_CreateObject(); } | ||
|
|
||
| // Object / Array manipulation stubs | ||
| cJSON_bool cJSON_AddItemToObject(cJSON* object, const char* string, cJSON* item) { | ||
| (void)object; (void)string; (void)item; | ||
| return 1; // success | ||
| } | ||
|
|
||
| cJSON_bool cJSON_AddItemToArray(cJSON* array, cJSON* item) { | ||
| (void)array; (void)item; | ||
| return 1; // success | ||
| } |
There was a problem hiding this comment.
This fake overrides core cJSON APIs (cJSON_Parse/Create*/AddItem*, etc.). That means PlayerJsonObjectTests won’t exercise the real cJSON behavior (e.g., parse failures, object/array linkage, printing), and some tests may pass/fail for the wrong reasons. Prefer linking the real cJSON implementation for PlayerJsonObjectTests, or implement the stubs so they preserve expected cJSON semantics (child/next/string wiring, type flags, parse error handling).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 152 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| extern "C" { | ||
| struct cJSON; // forward declaration | ||
|
|
||
| // Creation / Deletion | ||
| cJSON* cJSON_CreateObject() { return (cJSON*)calloc(1, sizeof(cJSON)); } | ||
| cJSON* cJSON_CreateArray() { return (cJSON*)calloc(1, sizeof(cJSON)); } | ||
| cJSON* cJSON_CreateString(const char* str) { | ||
| cJSON* item = (cJSON*)calloc(1, sizeof(cJSON)); | ||
| if (str) item->valuestring = strdup(str); |
There was a problem hiding this comment.
FakePlayerJsonObject.cpp declares struct cJSON; as an incomplete type, but then uses sizeof(cJSON) and accesses members like item->valuestring, object->child, etc. This will not compile. Use the real cJSON header (preferred) or provide a minimal compatible struct cJSON + typedefs (e.g., cJSON_bool) that match the fields you access, and ensure the stubs don’t rely on an incomplete type.
| #include <gmock/gmock.h> | ||
| #include "MockDrmCallbacks.h" |
There was a problem hiding this comment.
This header includes itself (#include "MockDrmCallbacks.h"), which will recurse until include guards short-circuit, leaving DrmCallbacks/DrmHelperPtr potentially undefined here. Replace the self-include with the correct dependency header (likely DrmCallbacks.h) and remove the duplicate <gmock/gmock.h> include.
| #include <gmock/gmock.h> | |
| #include "MockDrmCallbacks.h" | |
| #include "DrmCallbacks.h" |
| GType gst_verimatrixdecryptor_get_type(void) { return G_TYPE_OBJECT; } | ||
| GType gst_subtecbin_get_type(void) { return G_TYPE_OBJECT; } | ||
| GType gst_subtecmp4transform_get_type(void) { return G_TYPE_OBJECT; } | ||
| GType gst_subtecsink_get_type(void) { return G_TYPE_OBJECT; } | ||
| GType gst_vipertransform_get_type(void) { return G_TYPE_OBJECT; } | ||
|
|
There was a problem hiding this comment.
GstPluginsTests and PluginsTests unit tests reference several DRM plugin *_get_type() symbols (e.g., Widevine/PlayReady/ClearKey/CDMI). FakeGStreamer.cpp currently only provides stubs for Verimatrix/Subtec types, so these tests will likely fail to link unless the real plugin sources are built/linked into the test binaries. Add stubs for the missing *_get_type() functions here (or restore linking the corresponding plugin source files into the test executables).
| set(TEST_SOURCES GstPlugingRun.cpp | ||
| GstcdmidecryptorFun.cpp | ||
| GstclearkeydecryptorFun.cpp | ||
| GstplayreadydecryptorFun.cpp | ||
| GstverimatrixdecryptorFun.cpp | ||
| GstwidevinedecryptorFun.cpp) | ||
|
|
||
| set(FAKE_SOURCES ${UTESTS_ROOT}/fakes/FakeSocUtils.cpp | ||
| ${UTESTS_ROOT}/fakes/FakeGstPlayerTaskPool.cpp | ||
| ${UTESTS_ROOT}/fakes/FakeGStreamer.cpp) | ||
|
|
||
| set(PLAYER_SOURCES ${PLAYER_ROOT}/playerLogManager/PlayerLogManager.cpp | ||
| ${PLAYER_ROOT}/PlayerScheduler.cpp | ||
| ${PLAYER_ROOT}/PlayerUtils.cpp | ||
| ${PLAYER_ROOT}/baseConversion/_base64.cpp) | ||
|
|
There was a problem hiding this comment.
The tests in this target include headers like gstwidevinedecryptor.h / gstplayreadydecryptor.h and call gst_*_get_type() APIs. This CMakeLists doesn’t compile/link the corresponding plugin sources (e.g. gst-plugins/drm/gst/gstwidevinedecryptor.cpp, etc.) nor does it link against a library that provides these symbols, so the test binary is likely to fail at link time unless stubs are provided elsewhere. Either add the needed plugin sources to PLAYER_SOURCES / link to the plugin library target, or ensure FakeGStreamer.cpp provides all required gst_*_get_type() stubs for this executable.
| #include "PacketSender.hpp" | ||
| #include <gmock/gmock.h> | ||
|
|
||
| class MockPacketSender : public PacketSender { | ||
| public: | ||
| static PacketSender* Instance() { | ||
| static MockPacketSender instance; | ||
| return &instance; | ||
| } | ||
|
|
||
| bool initSocket(const char* socket_path) override { | ||
| std::cout << "[Mocked] initSocket called for " << socket_path << std::endl; | ||
| return true; // Pretend success | ||
| } |
There was a problem hiding this comment.
MockPacketSender uses std::cout but doesn’t include <iostream>, and PacketSender.hpp also doesn’t include it. This can break compilation depending on include order. Add <iostream> to this header (or avoid iostream usage in headers).
| #include <gtest/gtest.h> | ||
|
|
||
| int main(int argc, char** argv) | ||
| { | ||
| testing::InitGoogleTest(&argc, argv); | ||
| return RUN_ALL_TESTS(); | ||
| } No newline at end of file |
There was a problem hiding this comment.
This file is named PlayerCCManagerTests.cpp but it only defines main(). In the rest of the utests tree the convention appears to be *Run.cpp (e.g., PlayerVendorRun.cpp, SubtitleRun.cpp) for the test runner entry point. Consider renaming this to PlayerCCManagerRun.cpp (and keep actual test cases in *Tests.cpp) to avoid confusion and keep naming consistent.
| bool isVideoMaster = true; | ||
| if (socInterface) | ||
| { | ||
| isVideoMaster = socInterface->IsVideoMaster(gstPrivateContext->video_sink); | ||
| } | ||
| else | ||
| { | ||
| MW_LOG_WARN("socInterface is null, assuming video master"); | ||
| } | ||
|
|
||
| // set applied_rate to trickplay rate if video sink doesn't use vmaster | ||
| // so that it can correctly handle there being no audio | ||
| if (!isVideoMaster) | ||
| { |
There was a problem hiding this comment.
PR description focuses on moving/integrating L1 test cases, but this change modifies production playback behavior (handling of socInterface == nullptr and applied_rate logic in SendNewSegmentEvent). Please confirm this functional change is intended/in-scope for RDKEMW-14462 and that downstream consumers have been validated (or move the production fix into a separate PR).
RDKEMW-14462 : Move New L1 Test Cases to feature/dev_sprint_pli Branch
Reason for change: Integerate L1 test cases
Test Procedure: Refer Ticket
Risks: Medium
Priority: P1
Signed-off-by: Rekha Kandhavelan [rekha_kandhavelan@comcast.com]