RDKEMW-22169:Updated version to 4.4.6 - #178
Open
RajaLucy wants to merge 9 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the pinned Thunder/ThunderTools release references used by local dependency build scripting and the CI L1 test workflow to align with the R4.4.6 release stream.
Changes:
- Bump ThunderTools and Thunder clone branches in
build_dependencies.shtoR4.4.6. - Bump Thunder reference (
THUNDER_REF) and ThunderTools checkoutrefin the L1 GitHub Actions workflow toR4.4.6.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| build_dependencies.sh | Updates ThunderTools/Thunder clone branches to R4.4.6 for local/CI dependency setup. |
| .github/workflows/L1-tests.yml | Updates Thunder/ThunderTools refs for L1 CI builds (plus workflow env/ref consistency). |
Comments suppressed due to low confidence (1)
.github/workflows/L1-tests.yml:117
- ThunderTools checkout
refis hardcoded while Thunder usesenv.THUNDER_REF. This can silently drift (and also mismatches the cache key which is based onenv.THUNDER_REF). Consider reusingenv.THUNDER_REFhere for consistency.
ref: R4.4.6
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
.github/workflows/L1-tests.yml:17
- GitHub Actions expression for
AUTOMATICS_PASSCODEhas an extra space aftersecrets.("secrets. AUTOMATICS_PASSCODE"), which will fail expression parsing and leave the env var unset.
THUNDER_REF: "R4.4.6"
INTERFACES_REF: "develop"
AUTOMATICS_UNAME: ${{ secrets.AUTOMATICS_UNAME}}
AUTOMATICS_PASSCODE: ${{ secrets. AUTOMATICS_PASSCODE}}
build_dependencies.sh:64
- Typo in log message: "buliding" -> "building".
echo "======================================================================================"
echo "buliding thunder"
cd Thunder
patch -p1 < $GITHUB_WORKSPACE/entservices-testframework/patches/error_code_R4_4_6.patch
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
.github/workflows/L1-tests.yml:18
- The secret expression for AUTOMATICS_PASSCODE has a space after
secrets.(secrets. AUTOMATICS_PASSCODE), which is not valid GitHub Actions expression syntax and will fail workflow evaluation.
INTERFACES_REF: "develop"
AUTOMATICS_UNAME: ${{ secrets.AUTOMATICS_UNAME}}
AUTOMATICS_PASSCODE: ${{ secrets. AUTOMATICS_PASSCODE}}
ENABLE_CACHE: "false"
build_dependencies.sh:45
echo "buliding thunderTools"contains a spelling error in a user-facing log message.
echo "======================================================================================"
echo "buliding thunderTools"
cd ThunderTools
patch -p1 < $GITHUB_WORKSPACE/entservices-testframework/patches/00010-R4.4.6-Add-support-for-project-dir.patch
build_dependencies.sh:63
echo "buliding thunder"contains a spelling error in a user-facing log message.
# Build Thunder
echo "======================================================================================"
echo "buliding thunder"
cd Thunder
.github/workflows/L1-tests.yml:125
- The workflow now checks out
rdkcentral/entservices-testframeworkfrom a feature branch. Per Tests/README.md this is typically a temporary setup for validating testframework changes; leaving a feature branch ref in this repo can make CI non-reproducible once that branch is rebased/deleted. Consider pinning to a tag/SHA or switching back todeveloponce the testframework changes are merged, and align the PR description with this dependency change.
- name: Checkout entservices-testframework
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-testframework
path: entservices-testframework
ref: feature/RDKEMW-22169
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
Tests/L1Tests/tests/test_XCast.cpp:92
- MockIDeviceInfo::AddRef/Release are expected to behave like COM-style reference counting (returning the updated ref count). Returning 0 / Core::ERROR_NONE while also having an unused _refCount can lead to incorrect behavior in code under test and makes the mock inconsistent with the interface contract.
uint32_t AddRef() const override {
// Mock implementation - do nothing in tests
return 0;
}
uint32_t Release() const override {
// Mock implementation - return reference count
return Core::ERROR_NONE;
}
.github/workflows/L1-tests.yml:17
- The secrets expression for AUTOMATICS_PASSCODE contains an extra space ("secrets. AUTOMATICS_PASSCODE"), which makes the expression invalid/undefined in GitHub Actions. This will result in an empty value or workflow expression error at runtime.
THUNDER_REF: "R4.4.6"
INTERFACES_REF: "develop"
AUTOMATICS_UNAME: ${{ secrets.AUTOMATICS_UNAME}}
AUTOMATICS_PASSCODE: ${{ secrets. AUTOMATICS_PASSCODE}}
.github/workflows/L1-tests.yml:125
- This PR changes entservices-testframework checkout from a stable branch (develop) to a feature branch (feature/RDKEMW-22169). That’s a significant workflow behavior change not mentioned in the PR description and can reduce build reproducibility if the feature branch is force-pushed or deleted. Consider pinning to a tag or commit SHA, or moving required patches to a stable branch.
- name: Checkout entservices-testframework
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-testframework
path: entservices-testframework
ref: feature/RDKEMW-22169
build_dependencies.sh:37
- build_dependencies.sh now clones entservices-testframework from a feature branch (feature/RDKEMW-22169). This can make dependency builds non-reproducible if the branch changes or is removed; prefer pinning to a tag or commit SHA (or a stable branch) once the required patches are merged upstream.
git clone --branch develop https://github.com/rdkcentral/entservices-apis.git
git clone --branch feature/RDKEMW-22169 https://github.com/rdkcentral/entservices-testframework.git
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
Tests/L1Tests/tests/test_XCast.cpp:87
- AddRef() currently returns 0 and ignores the existing _refCount. Some code paths (including COM-style helpers) expect AddRef() to return a non-zero reference count, and returning 0 can break ownership assumptions in tests.
uint32_t AddRef() const override {
// Mock implementation - do nothing in tests
return 0;
}
Tests/L1Tests/tests/test_XCast.cpp:92
- Release() returns Core::ERROR_NONE (0) rather than an updated reference count and also ignores _refCount, which makes the mock's lifetime semantics inconsistent with IUnknown expectations.
uint32_t Release() const override {
// Mock implementation - return reference count
return Core::ERROR_NONE;
}
.github/workflows/L1-tests.yml:15
- This PR is described as updating Thunder/ThunderTools to 4.4.6, but INTERFACES_REF is also changed to "develop" (while build_dependencies.sh still pins entservices-apis to 4.0.8). If the interfaces bump is intentional, it should be called out explicitly; otherwise CI and local dependency builds may use different interface versions and produce inconsistent results.
REPO_NAME: "casting"
INTERFACES_REF: "develop"
AUTOMATICS_UNAME: ${{ secrets.AUTOMATICS_UNAME}}
build_dependencies.sh:37
- build_dependencies.sh clones entservices-testframework from a moving feature branch (feature/RDKEMW-22169). This makes builds non-reproducible over time and can break unexpectedly as the branch changes; prefer pinning to a tag/release branch or a specific commit SHA.
git clone --branch 4.0.8 https://github.com/rdkcentral/entservices-apis.git
git clone --branch feature/RDKEMW-22169 https://github.com/rdkcentral/entservices-testframework.git
Comment on lines
12
to
16
| BUILD_TYPE: Debug | ||
| REPO_NAME: "casting" | ||
| THUNDER_REF: "R4.4.1" | ||
| INTERFACES_REF: "4.0.8" | ||
| INTERFACES_REF: "develop" | ||
| AUTOMATICS_UNAME: ${{ secrets.AUTOMATICS_UNAME}} | ||
| AUTOMATICS_PASSCODE: ${{ secrets. AUTOMATICS_PASSCODE}} |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (7)
Tests/L1Tests/tests/test_XCast.cpp:87
AddRef()currently returns 0, which is not a meaningful COM-style refcount and can break code paths that expect the returned refcount to be >= 1. Since the class already has_refCount, increment and return it here.
uint32_t AddRef() const override {
// Mock implementation - do nothing in tests
return 0;
}
Tests/L1Tests/tests/test_XCast.cpp:92
Release()returnsCore::ERROR_NONE(0) instead of a reference count, and_refCountis never initialized. This makes the mock’s lifetime semantics inconsistent and may cause flaky behavior if any code uses the returned refcount.
uint32_t Release() const override {
// Mock implementation - return reference count
return Core::ERROR_NONE;
}
.github/workflows/L1-tests.yml:18
- The secret expression has an extra space (
secrets. AUTOMATICS_PASSCODE) which makes the GitHub Actions expression invalid and can prevent the workflow from parsing/running.
AUTOMATICS_PASSCODE: ${{ secrets. AUTOMATICS_PASSCODE}}
build_dependencies.sh:71
- These
cdcommands are a no-op (cd Thunder; cd -) and don’t affect the subsequent build (which uses-S Thunder). Leaving them in is confusing and makes the script harder to maintain.
echo "buliding thunder"
cd Thunder
cd -
.github/workflows/L1-tests.yml:66
- The cache key does not include
THUNDER_TOOLS_COMMIT_SHA, but the cached paths includebuild/ThunderToolsandinstalloutputs derived from it. This can cause incorrect cache hits when ThunderTools changes.
key: ${{ runner.os }}-${{ env.REPO_NAME }}-${{ env.THUNDER_COMMIT_SHA }}-${{ env.INTERFACES_REF }}-4
.github/workflows/L1-tests.yml:16
INTERFACES_REFwas changed todevelop, which is a moving target and reduces build reproducibility; it also diverges frombuild_dependencies.shwhich still clonesentservices-apisfrom4.0.8. It would be better to pin this to a tag/commit and keep both build paths consistent.
INTERFACES_REF: "develop"
build_dependencies.sh:53
- These
cdcommands are a no-op (cd ThunderTools; cd -) and don’t affect the subsequent build (which uses-S ThunderTools). Leaving them in is confusing and makes the script harder to maintain.
This issue also appears on line 68 of the same file.
echo "buliding thunderTools"
cd ThunderTools
cd -
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Suppressed comments (6)
Tests/L1Tests/tests/test_XCast.cpp:88
- MockIDeviceInfo's IUnknown methods don't maintain the mock's _refCount: AddRef() always returns 0 and Release() always returns Core::ERROR_NONE. Since getSerialNumberFromDeviceInfo() calls Release() on the returned IDeviceInfo pointer, keeping a consistent (non-underflowing) refcount in the mock avoids subtle lifetime issues if test code (or future code) starts relying on COM-style semantics.
uint32_t AddRef() const override {
// Mock implementation - do nothing in tests
return 0;
}
.github/workflows/L1-tests.yml:391
- Same issue as above: the build sets the CMake variable USE_THUNDER_R4, but without also defining the preprocessor macro, code guarded by
#ifdef USE_THUNDER_R4will compile the non-R4 path while CMake links the R4/COM path.
-DCMAKE_CXX_FLAGS="
-fprofile-arcs
-ftest-coverage
-DEXCEPTIONS_ENABLE=ON
-DTHUNDER_VERSION=4
-DTHUNDER_VERSION_MAJOR=4
.github/workflows/L1-tests.yml:461
- Same issue as above: USE_THUNDER_R4 is no longer defined in the compiler flags, but this repo relies on
#ifdef USE_THUNDER_R4in multiple headers/sources. Without a compiler define, the non-R4 code path will be compiled even when linking against R4/COM.
-DCMAKE_CXX_FLAGS="
-fprofile-arcs
-ftest-coverage
-DEXCEPTIONS_ENABLE=ON
-DTHUNDER_VERSION=4
-DTHUNDER_VERSION_MAJOR=4
.github/workflows/L1-tests.yml:126
- The workflow now checks out entservices-testframework from a feature branch (feature/RDKEMW-22169). Using a moving branch can make CI results non-reproducible over time; prefer pinning to a commit SHA (as done for Thunder/ThunderTools) or a stable tag when possible.
- name: Checkout entservices-testframework
uses: actions/checkout@v3
with:
repository: rdkcentral/entservices-testframework
path: entservices-testframework
ref: feature/RDKEMW-22169
.github/workflows/L1-tests.yml:16
- INTERFACES_REF is now set to "develop", which makes CI non-deterministic (the same commit of this repo can start failing when entservices-apis changes). It also diverges from build_dependencies.sh, which still clones entservices-apis at the pinned "4.0.8" tag. Consider pinning this to a released tag/commit SHA for reproducible builds.
THUNDER_TOOLS_COMMIT_SHA: "d5dd83c7c19c49c7f25c558c126500bd2d64f7a4"
THUNDER_COMMIT_SHA: "2c0fcc5529e7da734be558ca6efa05d934dcce31"
INTERFACES_REF: "develop"
build_dependencies.sh:45
- build_dependencies.sh now pins Thunder/ThunderTools to specific commit SHAs, but entservices-testframework is still cloned from a feature branch. For reproducible local builds (and to avoid unexpected breakages when the branch moves), consider pinning entservices-testframework to a commit SHA or stable tag as well.
git clone --branch 4.0.8 https://github.com/rdkcentral/entservices-apis.git
git clone --branch feature/RDKEMW-22169 https://github.com/rdkcentral/entservices-testframework.git
| -DENABLE_SET_WAKEUP_SRC_CONFIG -DENABLE_THERMAL_PROTECTION \ | ||
| -DUSE_DRM_SCREENCAPTURE -DHAS_API_SYSTEM -DHAS_API_POWERSTATE \ | ||
| -DHAS_RBUS -DDISABLE_SECURITY_TOKEN -DENABLE_DEVICE_MANUFACTURER_INFO -DUSE_THUNDER_R4=ON -DTHUNDER_VERSION=4 -DTHUNDER_VERSION_MAJOR=4 -DTHUNDER_VERSION_MINOR=4" \ | ||
| -DHAS_RBUS -DDISABLE_SECURITY_TOKEN -DENABLE_DEVICE_MANUFACTURER_INFO -DTHUNDER_VERSION=4 -DTHUNDER_VERSION_MAJOR=4 -DTHUNDER_VERSION_MINOR=4" \ |
Comment on lines
329
to
333
| -DCMAKE_CXX_FLAGS=" | ||
| -fprofile-arcs | ||
| -ftest-coverage | ||
| -DEXCEPTIONS_ENABLE=ON | ||
| -DUSE_THUNDER_R4=ON | ||
| -DTHUNDER_VERSION=4 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason: Updated Thunder and ThunderTools version to 4.4.6.
Test Procedure: Refer ticket.
Risks: Medium.
Priority: P0.
Version: Patch.