Add support for COMRPC changes in L2Tests plugin in Testframework repo - #129
Add support for COMRPC changes in L2Tests plugin in Testframework repo#129smanes0213 wants to merge 14 commits into
Conversation
Changed -Wno-deprecated-declarations to -Wno-error=deprecated-declarations to allow warnings from Thunder's VirtualInput.h without failing the build.
There was a problem hiding this comment.
Pull request overview
This pull request updates the L2Tests plugin in the TestFramework repository to align with COM-RPC (Component Object Model - Remote Procedure Call) best practices. The changes remove an unnecessary manual reference count increment and update workflow configurations to use feature branches for testing the COMRPC changes.
Changes:
- Removed manual
AddRef()call on interface obtained viaQueryInterface()in LedControl_L2Test - Updated workflow references from
developtoL2TestPlugin_COMRPCbranch for coordinated testing - Fixed CMake build configuration parameter name from
BUILD_TYPEtoCMAKE_BUILD_TYPE - Updated
entservices-apisreference toTestbranch for all workflows
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| Tests/L2Tests/tests/LedControl_L2Test.cpp | Removed unnecessary manual AddRef() call after QueryInterface(), properly aligning with Thunder COM-RPC semantics |
| .github/workflows/L2-tests.yml | Updated branch references to coordinate testing and fixed CMake parameter name |
| .github/workflows/L1-tests.yml | Updated branch references to coordinate testing across test workflows |
Comments suppressed due to low confidence (1)
Tests/L2Tests/tests/LedControl_L2Test.cpp:138
- The
QueryInterface()method in Thunder's COM-RPC framework automatically increments the reference count when returning an interface pointer. The removal of the manualAddRef()call is correct and aligns with Thunder COM-RPC best practices. However, ensure that the interface is still properly released in the destructor (which is confirmed at lines 103-105). This change correctly addresses reference counting semantics.
m_LEDplugin = m_controller_LED->QueryInterface<Exchange::ILEDControl>();
| repository: rdkcentral/entservices-peripherals | ||
| path: entservices-peripherals | ||
| ref: develop | ||
| ref: L2TestPlugin_COMRPC |
There was a problem hiding this comment.
Workflow is using feature branch references (L2TestPlugin_COMRPC and Test) instead of stable branches like develop. While this is appropriate for testing coordinated changes across repositories, ensure these feature branches are merged back to stable branches before this workflow change is merged to the main branch. Feature branch references in CI workflows can break when those branches are deleted after merge.
| repository: rdkcentral/entservices-testframework | ||
| path: entservices-testframework | ||
| ref: develop | ||
| ref: L2TestPlugin_COMRPC |
There was a problem hiding this comment.
Workflow is using feature branch references (L2TestPlugin_COMRPC and Test) instead of stable branches like develop. While this is appropriate for testing coordinated changes across repositories, ensure these feature branches are merged back to stable branches before this workflow change is merged to the main branch. Feature branch references in CI workflows can break when those branches are deleted after merge.
| repository: rdkcentral/entservices-apis | ||
| path: entservices-apis | ||
| ref: ${{env.INTERFACES_REF}} | ||
| ref: Test |
There was a problem hiding this comment.
Workflow is using feature branch references (L2TestPlugin_COMRPC and Test) instead of stable branches like develop. While this is appropriate for testing coordinated changes across repositories, ensure these feature branches are merged back to stable branches before this workflow change is merged to the main branch. Feature branch references in CI workflows can break when those branches are deleted after merge.
| ref: Test | |
| ref: develop |
| repository: rdkcentral/entservices-testframework | ||
| path: entservices-testframework | ||
| ref: develop | ||
| ref: L2TestPlugin_COMRPC |
There was a problem hiding this comment.
Workflow is using feature branch references (L2TestPlugin_COMRPC and Test) instead of stable branches. This is a concern for the same reason as in L2-tests.yml - ensure these feature branches exist and are coordinated, or use stable branch references after the dependent changes are merged. Feature branch references can cause CI failures when branches are deleted.
| repository: rdkcentral/entservices-peripherals | ||
| path: entservices-peripherals | ||
| ref: develop | ||
| ref: L2TestPlugin_COMRPC |
There was a problem hiding this comment.
Workflow is using feature branch references (L2TestPlugin_COMRPC and Test) instead of stable branches. This is a concern for the same reason as in L2-tests.yml - ensure these feature branches exist and are coordinated, or use stable branch references after the dependent changes are merged. Feature branch references can cause CI failures when branches are deleted.
| repository: rdkcentral/entservices-apis | ||
| path: entservices-apis | ||
| ref: ${{env.INTERFACES_REF}} | ||
| ref: Test |
There was a problem hiding this comment.
Workflow is using feature branch references (L2TestPlugin_COMRPC and Test) instead of stable branches. This is a concern for the same reason as in L2-tests.yml - ensure these feature branches exist and are coordinated, or use stable branch references after the dependent changes are merged. Feature branch references can cause CI failures when branches are deleted.
| ref: Test |
| repository: rdkcentral/entservices-testframework | ||
| path: entservices-testframework | ||
| ref: develop | ||
| ref: L2TestPlugin_COMRPC |
There was a problem hiding this comment.
The workflow now references the branch 'L2TestPlugin_COMRPC' instead of 'develop'. This appears to be a temporary branch for testing COMRPC changes. Before merging, ensure that:
- This branch exists and contains the necessary COMRPC changes
- There is a plan to either merge this branch back to develop or update this reference to 'develop' once the COMRPC changes are integrated
- The same branch reference update is coordinated across both entservices-peripherals and entservices-testframework repositories
| repository: rdkcentral/entservices-peripherals | ||
| path: entservices-peripherals | ||
| ref: develop | ||
| ref: L2TestPlugin_COMRPC |
There was a problem hiding this comment.
The workflow now references the branch 'L2TestPlugin_COMRPC' instead of 'develop'. This appears to be a temporary branch for testing COMRPC changes. Before merging, ensure that:
- This branch exists and contains the necessary COMRPC changes
- There is a plan to either merge this branch back to develop or update this reference to 'develop' once the COMRPC changes are integrated
- The same branch reference update is coordinated across both entservices-peripherals and entservices-testframework repositories
No description provided.