RDKMVE-1371: Add AppManager test scripts and AI 2.0 utility files#95
RDKMVE-1371: Add AppManager test scripts and AI 2.0 utility files#95nidhinrv2007 wants to merge 17 commits into
Conversation
… syntax errors, and clean up code structure
…th systemctl service management and proper error handling
There was a problem hiding this comment.
Pull request overview
This PR adds a new suite of AppManager TDK test scripts along with shared “AI 2.0” configuration and helper utilities to support Thunder JSON-RPC operations.
Changes:
- Added 34 AppManager test script skeletons (activation/query/property/negative scenarios).
- Added
ai2_0_utils.pyutility module for config loading, JSON-RPC helpers, and plugin activation/status checks. - Added
ai_2_0_cpe.jsonconfiguration profile (Thunder/DAC/PackageManager/DownloadManager settings).
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_01_Activate.py | Adds AppManager activation test script scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_02_LaunchApp_Positive.py | Adds positive launchApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_03_LaunchApp_Negative.py | Adds negative launchApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_04_PreloadApp_Positive.py | Adds positive preloadApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_05_PreloadApp_Negative.py | Adds negative preloadApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_06_CloseApp_Positive.py | Adds positive closeApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_07_CloseApp_Negative.py | Adds negative closeApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_08_TerminateApp_Positive.py | Adds positive terminateApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_09_TerminateApp_Negative.py | Adds negative terminateApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_10_KillApp_Positive.py | Adds positive killApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_11_KillApp_Negative.py | Adds negative killApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_12_IsInstalled_Positive.py | Adds positive isInstalled test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_13_IsInstalled_Negative.py | Adds negative isInstalled test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_14_GetInstalledApps.py | Adds getInstalledApps query test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_15_GetLoadedApps.py | Adds getLoadedApps query test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_16_SendIntent_Positive.py | Adds positive sendIntent test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_17_SendIntent_Negative.py | Adds negative sendIntent test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_18_StartSystemApp_Positive.py | Adds positive startSystemApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_19_StartSystemApp_Negative.py | Adds negative startSystemApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_20_StopSystemApp_Positive.py | Adds positive stopSystemApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_21_StopSystemApp_Negative.py | Adds negative stopSystemApp test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_22_ClearAppData_Positive.py | Adds positive clearAppData test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_23_ClearAppData_Negative.py | Adds negative clearAppData test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_24_ClearAllAppData.py | Adds clearAllAppData query test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_25_GetAppMetadata_Positive.py | Adds positive getAppMetadata test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_26_GetAppMetadata_Negative.py | Adds negative getAppMetadata test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_27_GetAppProperty_Positive.py | Adds positive getAppProperty test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_28_GetAppProperty_Negative.py | Adds negative getAppProperty test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_29_SetAppProperty_Positive.py | Adds positive setAppProperty test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_30_SetAppProperty_Negative.py | Adds negative setAppProperty test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_31_GetMaxRunningApps.py | Adds getMaxRunningApps property test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_32_GetMaxHibernatedApps.py | Adds getMaxHibernatedApps property test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_33_GetMaxHibernatedFlashUsage.py | Adds getMaxHibernatedFlashUsage property test scaffold. |
| framework/fileStore/testscriptsRDKV/component/AppManager/RDKV_AppManager_34_GetMaxInactiveRamUsage.py | Adds getMaxInactiveRamUsage property test scaffold. |
| framework/fileStore/ai_2_0_cpe.json | Adds CPE config values (Thunder/DAC/PackageManager/DownloadManager). |
| framework/fileStore/ai2_0_utils.py | Adds AI2.0 helper module (config loader, JSON-RPC wrappers, plugin checks, download helpers). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print(" | ||
| [TEST] activate API - Activation scenarios") |
There was a problem hiding this comment.
The print(" followed by a newline creates an unterminated string literal and will raise a SyntaxError. Replace with a single string containing an escaped newline (e.g., print("\\n[TEST] ...")) or use a triple-quoted string. This same issue appears in the other newly added AppManager scripts (the [TEST] ... banner print).
| print(" | |
| [TEST] activate API - Activation scenarios") | |
| print("\n[TEST] activate API - Activation scenarios") |
| "url": "https://dac.dev.rdkinnovation.com", | ||
| "authentication": { | ||
| "user": "dac-cloud-rdkm-user", | ||
| "password": "wcE$:66[OkFbX-NrXvP*#F<HtR5z" |
There was a problem hiding this comment.
This commits a plaintext credential into the repository. Replace with a placeholder value and load the real secret from a secure mechanism (environment variable, secret store, or a local-only config not checked in), and rotate the exposed password immediately.
| "password": "wcE$:66[OkFbX-NrXvP*#F<HtR5z" | |
| "password": "${APPSTORE_CATALOG_PASSWORD}" |
| import json as _json | ||
|
|
||
| # Build the full method name | ||
| full_callsign = callsign_short if '.' in callsign_short else f"org.rdk.{callsign_short}" |
There was a problem hiding this comment.
thunder_call() will incorrectly prefix Controller calls as org.rdk.Controller.1.*, but Thunder’s Controller is typically Controller.1.* (no org.rdk.). This breaks callers like ensure_plugin_active() that invoke thunder_call(tdk_obj, "Controller", "activate", ...). Treat Controller as a special-case callsign (don’t prefix), or accept a fully-qualified callsign and only prefix when the short name is known to be in org.rdk.*.
| full_callsign = callsign_short if '.' in callsign_short else f"org.rdk.{callsign_short}" | |
| if '.' in callsign_short: | |
| # Already fully qualified (e.g., "org.rdk.PackageManager") | |
| full_callsign = callsign_short | |
| elif callsign_short == "Controller": | |
| # Thunder Controller is typically "Controller.1.*" (no "org.rdk." prefix) | |
| full_callsign = callsign_short | |
| else: | |
| # Default to org.rdk namespace for other short plugin names | |
| full_callsign = f"org.rdk.{callsign_short}" |
| # Build JSON-RPC payload | ||
| payload = { | ||
| "jsonrpc": "2.0", | ||
| "id": 8, | ||
| "method": "org.rdk.AppManager.killApp", | ||
| "params": {"appId": app_id} | ||
| } | ||
|
|
||
| print(f" [DAC01] Killing application: {app_id}") | ||
|
|
||
| # Execute killApp via JSON-RPC | ||
| response = requests.post(jsonrpc_url, json=payload, timeout=30) | ||
| response.raise_for_status() | ||
|
|
||
| result = response.json() | ||
|
|
||
| # Check for error | ||
| if result.get('error'): | ||
| error_info = result.get('error', {}) | ||
| error_msg = error_info.get('message', 'Unknown error') if isinstance(error_info, dict) else str(error_info) | ||
| print(f" [ERROR] killApp returned error: {error_msg}") | ||
| return False | ||
|
|
||
| # Success: should have 'result' field | ||
| if 'result' in result: | ||
| print(f" [SUCCESS] Application killed successfully") | ||
| return True | ||
|
|
||
| print(f" [ERROR] Invalid response format: {result}") | ||
| return False | ||
|
|
There was a problem hiding this comment.
The JSON-RPC method name here is inconsistent with the rest of the module (most AppManager calls use org.rdk.AppManager.1.*). To avoid compatibility issues across Thunder versions, consider using the same versioned method naming convention (or reuse jsonrpc_call_with_versions() to try both unversioned and versioned forms consistently).
| # Build JSON-RPC payload | |
| payload = { | |
| "jsonrpc": "2.0", | |
| "id": 8, | |
| "method": "org.rdk.AppManager.killApp", | |
| "params": {"appId": app_id} | |
| } | |
| print(f" [DAC01] Killing application: {app_id}") | |
| # Execute killApp via JSON-RPC | |
| response = requests.post(jsonrpc_url, json=payload, timeout=30) | |
| response.raise_for_status() | |
| result = response.json() | |
| # Check for error | |
| if result.get('error'): | |
| error_info = result.get('error', {}) | |
| error_msg = error_info.get('message', 'Unknown error') if isinstance(error_info, dict) else str(error_info) | |
| print(f" [ERROR] killApp returned error: {error_msg}") | |
| return False | |
| # Success: should have 'result' field | |
| if 'result' in result: | |
| print(f" [SUCCESS] Application killed successfully") | |
| return True | |
| print(f" [ERROR] Invalid response format: {result}") | |
| return False | |
| print(f" [DAC01] Killing application: {app_id}") | |
| # Prefer versioned AppManager interface, fall back to unversioned for compatibility | |
| methods_to_try = [ | |
| "org.rdk.AppManager.1.killApp", | |
| "org.rdk.AppManager.killApp", | |
| ] | |
| for method_name in methods_to_try: | |
| # Build JSON-RPC payload | |
| payload = { | |
| "jsonrpc": "2.0", | |
| "id": 8, | |
| "method": method_name, | |
| "params": {"appId": app_id}, | |
| } | |
| # Execute killApp via JSON-RPC | |
| response = requests.post(jsonrpc_url, json=payload, timeout=30) | |
| response.raise_for_status() | |
| result = response.json() | |
| # Check for error | |
| if result.get("error"): | |
| error_info = result.get("error", {}) | |
| error_msg = error_info.get("message", "Unknown error") if isinstance(error_info, dict) else str(error_info) | |
| # If this looks like "unknown method" and we have another variant to try, continue | |
| if "unknown" in error_msg.lower() and "method" in error_msg.lower() and method_name != methods_to_try[-1]: | |
| print(f" [INFO] Method {method_name} not supported, retrying with fallback...") | |
| continue | |
| print(f" [ERROR] killApp returned error: {error_msg}") | |
| return False | |
| # Success: should have 'result' field | |
| if "result" in result: | |
| print(f" [SUCCESS] Application killed successfully using {method_name}") | |
| return True | |
| print(f" [ERROR] Invalid response format from {method_name}: {result}") | |
| return False | |
| # If we exhaust all methods without returning, treat as failure | |
| print(" [ERROR] killApp failed with all tried method variants") | |
| return False |
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| ########################################################################## | ||
| ''' |
There was a problem hiding this comment.
Test scripts are not added as per new TM format.
As per new format all the XML tag data must come as part of Test Manager. Need not be added in the script.
There was a problem hiding this comment.
XML tags are removed to be compliant with new TM format
| print(" | ||
| [TEST] activate API - Activation scenarios") | ||
|
|
||
| # TODO: Add specific test implementation for activate |
There was a problem hiding this comment.
There should not be TODO's. implementation must be complete.
Why this TODO is kept? basic/needed functionality not added?
| obj.setLoadModuleStatus("SUCCESS") | ||
|
|
||
| try: | ||
| rpc_port = get_ai2_setting('appManager.jsonRpcPort', 9998) |
There was a problem hiding this comment.
Please follow existing RDK Services test format.
For RDKServices there are XML based test case format, which can be followed for the scenarios added here.
There was a problem hiding this comment.
Test scripts are updated as per the new format by removing XML block
|
|
||
|
|
||
|
|
||
| See `SOLUTION_GUIDE.md` for detailed step-by-step instructions.---- `tdklib_script_fixer.py` can be auto-imported to fix scripts on-the-fly- Helper scripts can be run manually on any generated script- No existing code will break- All changes are backwards compatible## Notes---5. **Verify** no more "Parameter (request_type)" errors4. **If syntax errors persist**, use `fix_script_syntax.py` to repair generated scripts3. **Run** your test scripts again2. **Restart** Tomcat to load new Python modules1. **Deploy** updated `ai2_0_utils.py` to your Tomcat server## Next Steps---```echo "Exit code: $?" # 0 = valid syntaxpython3 -m py_compile framework/fileStore/ai2_0_utils.py# Check syntax of ai2_0_utils.pyEOFprint("✓ Both functions available")from ai2_0_utils import jsonrpc_call, configure_tdk_test_casesys.path.insert(0, 'framework/fileStore')import syspython3 << 'EOF'# Check if functions are available```bash## Quick Verification---- 📄 `framework/fileStore/tdklib_script_fixer.py` - Auto-patching module- 📄 `framework/fileStore/fix_storage_test.py` - Storage Manager fixer- 📄 `framework/fileStore/fix_script_syntax.py` - Script syntax fixer### Helper Scripts- 📄 `SOLUTION_GUIDE.md` - Quick start guide### Documentation (in .gitignore)- ✅ `framework/fileStore/ai2_0_utils.py` - Updated with new functions### Code## Files Changed---- Includes automatic plugin activation fallback- No longer calls problematic TDK primitives- `check_thunder_plugin_status()` now uses JSON-RPC only**Solution**: Already fixed in updated `ai2_0_utils.py`### Issue 3: ERROR - Parameter (request_type) not found```) configure_tdk_test_case get_device_info_from_json, # <- ADD COMMA # ...from ai2_0_utils import (```pythonAdd comma after `get_device_info_from_json`:**Solution B - Manual fix:**```python3 framework/fileStore/fix_script_syntax.py <script_file>```bash**Solution A - Fix generated script:**### Issue 2: SyntaxError - Missing comma in import```/opt/apache-tomcat-7.0.96/bin/catalina.sh startsleep 5/opt/apache-tomcat-7.0.96/bin/catalina.sh stop# Restart Tomcat /opt/apache-tomcat-7.0.96/webapps/rdk-test-tool/fileStore/cp framework/fileStore/ai2_0_utils.py \```bash**Solution**: Deploy updated `ai2_0_utils.py` to Tomcat### Issue 1: ImportError - jsonrpc_call not found## How to Resolve Your Issues---- `tdklib_script_fixer.py` - Auto-patching hook- `fix_storage_test.py` - Storage Manager specific fixer- `fix_script_syntax.py` - Generic script fixer### 3. Helper Scripts for Manual Fixes ✅- Handle both active and inactive plugins- Include automatic plugin activation- Avoid "Parameter (request_type) not found" errors- Use ONLY JSON-RPC (no problematic TDK primitives)Enhanced `check_thunder_plugin_status()` to:### 2. Improved Plugin Status Checking ✅- `configure_tdk_test_case()` - TDK configuration wrapper (line 2637)- `jsonrpc_call()` - Simple JSON-RPC wrapper (line 2598)Added to `framework/fileStore/ai2_0_utils.py`:### 1. Core Utility Functions ✅## Changes Made# file the following copyright and licenses apply: |
There was a problem hiding this comment.
Incorrect data present in file, extra space also seen.
| # Configurable defaults (env > local config > hardcoded) | ||
| DEFAULT_DAC_CONFIG_URL = os.environ.get( | ||
| "AI2_DAC_CONFIG_URL", | ||
| str(get_ai2_setting('dac.configUrl', "https://dac.config.dev.fireboltconnect.com/configuration/cpe.json")) |
There was a problem hiding this comment.
urls must be kept configurable under the device configuration files
| @@ -0,0 +1,82 @@ | |||
| { | |||
There was a problem hiding this comment.
What is the purpose of this json file with hardcoded values?
|
|
||
|
|
||
| def ensure_plugin_active(tdk_obj, callsign: str, jsonrpc_url: Optional[str] = None, wait_seconds: float = 1.0) -> bool: | ||
| """ |
There was a problem hiding this comment.
Use existing framework instead of writing everything new.
In TDK both XML based approach and individual Python scripts framework is already present.
sahithya50
left a comment
sahithya50
left a comment
There was a problem hiding this comment.
Updated the comments with more details
…irectory Removing irrelevant file
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 98 out of 104 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| --- | ||
|
|
||
| **Updated:** 2025 |
There was a problem hiding this comment.
The 'Updated' field only contains the year '2025' without a specific date. For better documentation accuracy, include a full date (e.g., '2025-01-15') to indicate when this document was last updated.
| **Updated:** 2025 | |
| **Updated:** 2025-01-15 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 97 out of 102 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Directory containing the AppManager test files | ||
| test_dir = r"D:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" |
There was a problem hiding this comment.
Hardcoded Windows-specific absolute path. Consider using environment variables or relative paths for cross-platform compatibility. For example, use os.environ.get('TDK_TEST_DIR', 'default/path') or Path(__file__).parent / 'relative/path'.
| # Directory containing the AppManager test files | |
| test_dir = r"D:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | |
| # Directory containing the AppManager test files (configurable via TDK_TEST_DIR) | |
| DEFAULT_TEST_DIR = r"D:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | |
| test_dir = os.environ.get("TDK_TEST_DIR", DEFAULT_TEST_DIR) |
| import os | ||
| import re | ||
|
|
||
| base_dir = r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" |
There was a problem hiding this comment.
Hardcoded Windows-specific absolute path. Consider using environment variables or relative paths for cross-platform compatibility.
| base_dir = r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | |
| base_dir = os.environ.get( | |
| "APP_MANAGER_BASE_DIR", | |
| r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager", | |
| ) |
| script_dir = Path(__file__).parent / 'framework' / 'fileStore' / 'testscriptsRDKV' / 'component' / 'StorageManagerAI' | ||
|
|
||
| if not script_dir.exists(): | ||
| # Try current directory |
There was a problem hiding this comment.
Path construction logic appears incorrect. Path(__file__).parent already points to the current script's directory, so appending 'framework/fileStore/...' creates an invalid nested path. Consider using Path(__file__).parent.parent.parent.parent.parent / 'StorageManagerAI' or an absolute path from a known root.
| script_dir = Path(__file__).parent / 'framework' / 'fileStore' / 'testscriptsRDKV' / 'component' / 'StorageManagerAI' | |
| if not script_dir.exists(): | |
| # Try current directory | |
| # Default to the StorageManagerAI directory (parent of this script's shell_scripts directory) | |
| script_dir = Path(__file__).parent.parent | |
| if not script_dir.exists(): | |
| # Try locating the directory relative to the current working directory |
|
|
||
| --- | ||
|
|
||
| **Updated:** 2025 |
There was a problem hiding this comment.
Incomplete date specification. Should include month and day, e.g., '2025-01-15'.
| **Updated:** 2025 | |
| **Updated:** 2025-01-15 |
| # AppStore catalog authentication password (sensitive - update with actual credentials) | ||
| APPSTORE_CATALOG_PASSWORD = wcE$:66[OkFbX-NrXvP*#F<HtR5z |
There was a problem hiding this comment.
Hardcoded password in configuration file. This sensitive credential should be stored in environment variables or a secure vault, not committed to version control. Use placeholder like ${APPSTORE_CATALOG_PASSWORD} and document required environment variables.
| # AppStore catalog authentication password (sensitive - update with actual credentials) | |
| APPSTORE_CATALOG_PASSWORD = wcE$:66[OkFbX-NrXvP*#F<HtR5z | |
| # AppStore catalog authentication password (sensitive - provide via environment variable or secure vault) | |
| APPSTORE_CATALOG_PASSWORD = ${APPSTORE_CATALOG_PASSWORD} |
| if '=' in line: | ||
| cfg_key, cfg_value = line.split('=', 1) | ||
| if cfg_key.strip() == key: | ||
| return cfg_value.strip() |
There was a problem hiding this comment.
Configuration values are always returned as strings. Numeric values like ports need explicit type conversion by callers. Consider adding type coercion or documenting this behavior more prominently in the docstring, as the example shows port = get_config_value('APPMANAGER_JSONRPC_PORT', 9998) which may mislead users expecting an integer.
| from pathlib import Path | ||
|
|
||
| # Directory containing the AppManager test files | ||
| test_dir = r"D:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" |
There was a problem hiding this comment.
Hardcoded Windows-specific absolute path. Consider using environment variables or relative paths for cross-platform compatibility.
| test_dir = r"D:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | |
| test_dir_env = os.getenv("APPMANAGER_TEST_DIR") | |
| if test_dir_env: | |
| test_dir = Path(test_dir_env) | |
| else: | |
| # Fallback to the directory containing this script | |
| test_dir = Path(__file__).resolve().parent |
|
|
||
| def main(): | ||
| """Main validation function""" | ||
| base_dir = r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" |
There was a problem hiding this comment.
Hardcoded Windows-specific absolute path. Consider using environment variables or relative paths for cross-platform compatibility.
| base_dir = r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | |
| base_dir_env = os.environ.get("APPMANAGER_TEST_BASE_DIR") | |
| base_dir = Path(base_dir_env).resolve() if base_dir_env else Path(__file__).resolve().parent |
|
|
||
| def main(): | ||
| """Main reconstruction function""" | ||
| base_dir = r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" |
There was a problem hiding this comment.
Hardcoded Windows-specific absolute path. Consider using environment variables or relative paths for cross-platform compatibility.
| base_dir = r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | |
| base_dir = os.getenv("APPMANAGER_TEST_BASE_DIR") | |
| if not base_dir: | |
| # Default to the directory containing this script if environment variable is not set | |
| base_dir = str(Path(__file__).resolve().parent) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 96 out of 99 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
refactor_appmanager_config.py:1
- This script hard-codes a developer-specific Windows path, which makes it non-portable in CI/other machines. Consider accepting
test_diras a CLI argument (argparse), defaulting to a path relative to the repo root/current working directory. Also,osis imported but not used in this file and can be removed.
#!/usr/bin/env python3
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| script_dir = Path(__file__).parent / 'framework' / 'fileStore' / 'testscriptsRDKV' / 'component' / 'StorageManagerAI' | ||
|
|
||
| if not script_dir.exists(): | ||
| # Try current directory | ||
| script_dir = Path('.') / 'framework' / 'fileStore' / 'testscriptsRDKV' / 'component' / 'StorageManagerAI' | ||
|
|
||
| if not script_dir.exists(): |
There was a problem hiding this comment.
The computed script_dir is very likely wrong because this script already lives under .../framework/fileStore/testscriptsRDKV/component/StorageManagerAI/shell_scripts/, so appending framework/fileStore/... again won’t exist. Consider deriving the component root by walking parents from __file__ (e.g., Path(__file__).resolve().parents[...]) or by resolving relative to shell_scripts’ parent directory.
| script_dir = Path(__file__).parent / 'framework' / 'fileStore' / 'testscriptsRDKV' / 'component' / 'StorageManagerAI' | |
| if not script_dir.exists(): | |
| # Try current directory | |
| script_dir = Path('.') / 'framework' / 'fileStore' / 'testscriptsRDKV' / 'component' / 'StorageManagerAI' | |
| if not script_dir.exists(): | |
| # The script is located in ".../StorageManagerAI/shell_scripts/validate_scripts.py" | |
| # so the component root ("StorageManagerAI") is the parent of the "shell_scripts" directory. | |
| script_dir = Path(__file__).resolve().parent.parent | |
| if not script_dir.exists(): |
| if f"import {imp}" in content or f"from {imp}" in content: | ||
| found_imports.append(imp) | ||
|
|
||
| return len(found_imports) >= 1, found_imports |
There was a problem hiding this comment.
The function name/docstring says it verifies required imports, but the current condition passes when only 1 of 2 imports is found (>= 1). If both are required, change the condition to require all expected imports (e.g., compare counts or use all(...)).
| return len(found_imports) >= 1, found_imports | |
| return len(found_imports) == len(required_imports), found_imports |
|
|
||
| except Exception as e: | ||
| print(f"[ERROR] Failed to process {filepath}: {str(e)}") | ||
| return False, [] |
There was a problem hiding this comment.
failed_count will never increment on exceptions because update_file() returns (False, []) in the exception path, so changes is always falsy. If you want to track failures, return a non-empty indicator on exception (or return a third value like error) and increment failed_count based on that.
| return False, [] | |
| # Return a non-empty changes list so failures can be counted in main() | |
| return False, [f"Exception while processing {filepath}: {str(e)}"] |
| for test_file in test_files: | ||
| success, changes = update_file(str(test_file)) | ||
| if success: | ||
| updated_count += 1 | ||
| print(f"✓ {test_file.name}") | ||
| for change in changes: | ||
| print(f" - {change}") | ||
| else: | ||
| if changes: # Only count as error if there was an exception | ||
| failed_count += 1 |
There was a problem hiding this comment.
failed_count will never increment on exceptions because update_file() returns (False, []) in the exception path, so changes is always falsy. If you want to track failures, return a non-empty indicator on exception (or return a third value like error) and increment failed_count based on that.
| obj.setLoadModuleStatus("FAILURE") | ||
| else: | ||
| print("[INFO] hibernateSystemApp API response: %s" % result) | ||
| obj.setLoadModuleStatus("SUCCESS") |
There was a problem hiding this comment.
In the unexpected-response branch, the test is currently marked as SUCCESS. This can produce false positives (e.g., malformed response, missing fields) and hide regressions. Recommend setting FAILURE (or at least a distinct 'NOT_APPLICABLE' workflow if supported) when the response doesn’t match success criteria and doesn’t contain an explicit error.
| obj.setLoadModuleStatus("SUCCESS") | |
| obj.setLoadModuleStatus("FAILURE") |
| # Start the AppManager plugin service | ||
| print("[INFO] Starting wpeframework-appmanager service...") | ||
| try: | ||
| service_name = get_config_value('APPMANAGER_SERVICE_NAME', 'wpeframework-appmanager.service') | ||
| subprocess.run(['systemctl', 'start', service_name], | ||
| check=False, timeout=10) | ||
| print("[INFO] Waiting for service to be active...") | ||
|
|
||
| # Check service status | ||
| status_result = subprocess.run(['systemctl', 'status', service_name], | ||
| capture_output=True, text=True, timeout=10) | ||
| if 'Active: active' in status_result.stdout: | ||
| print("[SUCCESS] wpeframework-appmanager service is active") | ||
| else: | ||
| print("[WARNING] wpeframework-appmanager service status unclear") | ||
| except Exception as e: | ||
| print("[WARNING] Could not manage service: %s" % str(e)) |
There was a problem hiding this comment.
The systemctl start/status block appears duplicated across many AppManager tests. Consider moving service start/status verification into a shared helper (e.g., aiutils.start_service(service_name, timeout)), so behavior is consistent and changes (timeouts, parsing, error handling) are made once.
|
|
||
| # Step 1: Test SSH connectivity | ||
| echo "[STEP 1] Testing SSH connectivity..." | ||
| if ssh -p $DEVICE_PORT ${DEVICE_USER}@${DEVICE_IP} "echo 'SSH connection successful'" 2>/dev/null | grep -q "successful"; then |
There was a problem hiding this comment.
These commands interpolate unquoted user-controlled inputs (DEVICE_IP, DEVICE_USER, DEVICE_PORT) directly into ssh/scp command lines. To reduce injection/option-parsing risk and improve robustness, quote variables (including -p/-P arguments) and consider validating DEVICE_PORT is numeric before use.
|
|
||
| # Step 2: Copy test files to device | ||
| echo "[STEP 2] Copying test files to /opt/..." | ||
| scp -P $DEVICE_PORT -r "${SCRIPT_DIR}"/* ${DEVICE_USER}@${DEVICE_IP}:/opt/ 2>/dev/null |
There was a problem hiding this comment.
These commands interpolate unquoted user-controlled inputs (DEVICE_IP, DEVICE_USER, DEVICE_PORT) directly into ssh/scp command lines. To reduce injection/option-parsing risk and improve robustness, quote variables (including -p/-P arguments) and consider validating DEVICE_PORT is numeric before use.
|
|
||
| # Step 3: Make validateStorageMgr.sh executable | ||
| echo "[STEP 3] Setting permissions..." | ||
| ssh -p $DEVICE_PORT ${DEVICE_USER}@${DEVICE_IP} "chmod +x /opt/validateStorageMgr.sh && chmod +x /opt/StorageMgr_*.sh" 2>/dev/null |
There was a problem hiding this comment.
These commands interpolate unquoted user-controlled inputs (DEVICE_IP, DEVICE_USER, DEVICE_PORT) directly into ssh/scp command lines. To reduce injection/option-parsing risk and improve robustness, quote variables (including -p/-P arguments) and consider validating DEVICE_PORT is numeric before use.
| @@ -0,0 +1 @@ | |||
| �� No newline at end of file | |||
There was a problem hiding this comment.
This looks like an accidentally committed binary/encoding artifact rather than meaningful test output. If it’s not intentionally used by tooling, consider removing it from the repo; otherwise, store a readable/loggable output format and ensure it’s saved with a consistent text encoding (e.g., UTF-8).
| �� | |
| This file is a placeholder for DownloadManager local test output. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 96 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print("[INFO] launchApp API response: %s" % result) | ||
| obj.setLoadModuleStatus("SUCCESS") |
There was a problem hiding this comment.
This test marks the module status as SUCCESS even in the fallback/"info" branch where the JSON-RPC response didn’t match expected success or error conditions. That can lead to false-positive test outcomes. Consider treating unexpected responses as FAILURE (or at least log and fail) so the test result reflects actual API correctness.
| print("[INFO] launchApp API response: %s" % result) | |
| obj.setLoadModuleStatus("SUCCESS") | |
| print("[FAILURE] Unexpected launchApp API response: %s" % result) | |
| obj.setLoadModuleStatus("FAILURE") |
| # Step 2: Copy test files to device | ||
| echo "[STEP 2] Copying test files to /opt/..." | ||
| scp -P $DEVICE_PORT -r "${SCRIPT_DIR}"/* ${DEVICE_USER}@${DEVICE_IP}:/opt/ 2>/dev/null | ||
|
|
||
| if [ $? -eq 0 ]; then | ||
| echo "[PASS] Files copied successfully" | ||
| else | ||
| echo "[FAIL] Failed to copy files" | ||
| exit 1 | ||
| fi | ||
| echo "" | ||
|
|
||
| # Step 3: Make validateStorageMgr.sh executable | ||
| echo "[STEP 3] Setting permissions..." | ||
| ssh -p $DEVICE_PORT ${DEVICE_USER}@${DEVICE_IP} "chmod +x /opt/validateStorageMgr.sh && chmod +x /opt/StorageMgr_*.sh" 2>/dev/null | ||
|
|
There was a problem hiding this comment.
The deploy script copies only ${SCRIPT_DIR}/* to /opt/, but ${SCRIPT_DIR} is the shell_scripts/ directory. The subsequent steps assume /opt/validateStorageMgr.sh and /opt/StorageMgr_*.sh exist, which won’t be true if those files are located outside shell_scripts/. Either copy the correct parent directory (or explicit files) and/or update the paths and chmod targets to match what is actually deployed.
| # Directory containing the AppManager test files | ||
| test_dir = r"D:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | ||
|
|
There was a problem hiding this comment.
These helper/refactoring scripts hardcode an absolute developer machine path (D:\Project\...). If these scripts are meant to live in-repo, they should accept the target directory via CLI args and/or compute paths relative to the repo. Otherwise, consider removing them from the PR to avoid shipping one-off local tooling into the main codebase.
| print("[INFO] launchApp API response: %s" % result) | ||
| obj.setLoadModuleStatus("SUCCESS") |
There was a problem hiding this comment.
In this negative test, the else branch treats any unexpected JSON-RPC response as SUCCESS (obj.setLoadModuleStatus("SUCCESS")). This can hide real regressions (e.g., API incorrectly succeeding). Unexpected responses should fail the test, while only the explicitly expected error/false result should pass.
| print("[INFO] launchApp API response: %s" % result) | |
| obj.setLoadModuleStatus("SUCCESS") | |
| print("[ERROR] launchApp API returned unexpected response for invalid app: %s" % result) | |
| obj.setLoadModuleStatus("FAILURE") |
| # Start the AppManager plugin service | ||
| print("[INFO] Starting wpeframework-appmanager service...") | ||
| try: | ||
| service_name = get_config_value('APPMANAGER_SERVICE_NAME', 'wpeframework-appmanager.service') | ||
| subprocess.run(['systemctl', 'start', service_name], | ||
| check=False, timeout=10) | ||
| print("[INFO] Waiting for service to be active...") | ||
|
|
||
| # Check service status | ||
| status_result = subprocess.run(['systemctl', 'status', service_name], | ||
| capture_output=True, text=True, timeout=10) | ||
| if 'Active: active' in status_result.stdout: | ||
| print("[SUCCESS] wpeframework-appmanager service is active") | ||
| else: | ||
| print("[WARNING] wpeframework-appmanager service status unclear") | ||
| except Exception as e: | ||
| print("[WARNING] Could not manage service: %s" % str(e)) | ||
|
|
There was a problem hiding this comment.
These tests attempt to manage systemctl services from within the test script. Since the script typically runs on the test runner (not on the target device), these systemctl calls may fail or affect the wrong host, and failures are downgraded to warnings. Prefer activating the plugin via Thunder Controller JSON-RPC (or a documented TDK prerequisite) rather than invoking systemctl directly from the test.
| def get_config_value(key, default=None): | ||
| """ | ||
| Read configuration value from Video_Accelerator.config | ||
|
|
||
| This function reads configuration key-value pairs from the Video_Accelerator.config | ||
| file, which is shared across all AI 2.0 service manager tests. | ||
|
|
||
| Args: | ||
| key (str): Configuration key to look up (e.g., 'APPMANAGER_JSONRPC_PORT') | ||
| default: Default value to return if key is not found (default: None) | ||
|
|
||
| Returns: | ||
| str or default: Configuration value as string, or default if not found | ||
|
|
||
| Supported configuration keys (examples): | ||
| - APPMANAGER_JSONRPC_PORT: JSON-RPC port for AppManager (default: 9998) | ||
| - APPMANAGER_SERVICE_NAME: SystemD service name for AppManager | ||
| - APPMANAGER_TEST_APP_ID: Test application ID for AppManager tests | ||
| - APPMANAGER_TEST_SYSTEM_APP_ID: Test system app ID for system app tests | ||
| - STORAGEMANAGER_JSONRPC_PORT: JSON-RPC port for StorageManager (future) | ||
| - PACKAGEMANAGER_JSONRPC_PORT: JSON-RPC port for PackageManager (future) | ||
| - DOWNLOADMANAGER_JSONRPC_PORT: JSON-RPC port for DownloadManager (future) | ||
|
|
||
| Example: | ||
| >>> port = get_config_value('APPMANAGER_JSONRPC_PORT', 9998) | ||
| >>> app_id = get_config_value('APPMANAGER_TEST_APP_ID', 'com.rdk.app.default') | ||
| """ | ||
| # Config file path: framework/fileStore/tdkvRDKServiceConfig/Video_Accelerator.config | ||
| config_file = os.path.join(os.path.dirname(__file__), 'tdkvRDKServiceConfig', 'Video_Accelerator.config') | ||
|
|
||
| try: | ||
| if os.path.exists(config_file): | ||
| with open(config_file, 'r') as f: | ||
| for line in f: | ||
| # Strip whitespace | ||
| line = line.strip() | ||
|
|
||
| # Skip empty lines and comments | ||
| if not line or line.startswith('#'): | ||
| continue | ||
|
|
||
| # Parse key=value pairs | ||
| if '=' in line: | ||
| cfg_key, cfg_value = line.split('=', 1) | ||
| if cfg_key.strip() == key: | ||
| return cfg_value.strip() | ||
| except Exception as e: |
There was a problem hiding this comment.
get_config_value() always returns the value as a string when found in the config file, even if the provided default is an int/bool. This can cause subtle type issues for numeric settings (e.g., timeouts/ports) depending on how callers use the value. Consider normalizing by casting to type(default) when default is not None (with safe error handling), or document that the function always returns strings.
| @@ -0,0 +1 @@ | |||
| �� No newline at end of file | |||
There was a problem hiding this comment.
This looks like a generated local test artifact containing non-text/binary content. Please avoid committing local run logs/output to the repo (or add an ignore rule and keep these out of version control).
| base_dir = r"d:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" | ||
|
|
There was a problem hiding this comment.
This helper script hardcodes an absolute developer machine path (d:\Project\...). If it’s intended for reuse, take base_dir as an argument or compute it relative to the repo; otherwise it should not be committed to the main repo.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 93 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from pathlib import Path | ||
|
|
||
| # Directory containing the AppManager test files | ||
| test_dir = r"D:\Project\TDK\testCodeRepo\tdk-core\framework\fileStore\testscriptsRDKV\component\AppManager" |
There was a problem hiding this comment.
Hardcoded absolute Windows path D:\Project\TDK\testCodeRepo\tdk-core\... will fail on other systems (Linux, macOS) or different Windows installations. Use relative paths or make this configurable via environment variables or command-line arguments.
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| import tdklib |
There was a problem hiding this comment.
The import statement from aiutils import get_config_value will fail because the aiutils module is located in framework/fileStore/ but the test scripts are in framework/fileStore/testscriptsRDKV/component/AppManager/. Python cannot find this module without adding the parent directory to sys.path first. Either add sys.path.insert(0, os.path.join(os.path.dirname(__file__), '../../../../')) before the import, or move aiutils.py to a location that's already in the Python path.
| import tdklib | |
| import tdklib | |
| import os | |
| import sys | |
| sys.path.insert(0, os.path.join(os.path.dirname(__file__), '../../../../')) |
| PACKAGES=( | ||
| "http://192.168.29.38/com.rdkcentral.cobalt+0.1.0.bolt" | ||
| "http://192.168.29.38/com.rdkcentral.youtube+0.1.0.bolt" | ||
| ) |
There was a problem hiding this comment.
This script downloads and installs package artifacts over plain HTTP from http://192.168.29.38/... via org.rdk.PackageManagerRDKEMS.download followed by install, without any TLS or checksum/signature verification. An attacker with network access (or control over the host at 192.168.29.38) could tamper with these .bolt files and have arbitrary code installed on the device via PackageManager. Use HTTPS for the package source and add integrity verification (e.g., pinned checksums or signatures) before invoking install to ensure only trusted artifacts are deployed.
AppManager Test Scripts
Core App Lifecycle Tests
App Info & Query Tests
App Communication Tests
System App Lifecycle Tests
App Data Management Tests
App Properties Tests
Configuration Parameters
All tests read configuration from
Video_Accelerator.configusing sharedaiutils.get_config_value():APPMANAGER_JSONRPC_PORTAPPMANAGER_SERVICE_NAMEAPPMANAGER_TEST_APP_IDAPPMANAGER_TEST_SYSTEM_APP_IDShared Utilities
aiutils.py
framework/fileStore/aiutils.py