diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml index 141eea71..a12e4523 100644 --- a/.github/workflows/code-coverage.yml +++ b/.github/workflows/code-coverage.yml @@ -19,6 +19,36 @@ jobs: run: | sh run_ut.sh --enable-cov + - name: Parse coverage and upload l1-metrics + shell: bash + run: | + set -euo pipefail + COV_FILE="rfcMgr/gtest/coverage.info" + SUMMARY="$(lcov --summary "$COV_FILE" 2>&1)" + + line_pct="$(echo "$SUMMARY" | sed -n 's/^[[:space:]]*lines\.*:[[:space:]]*\([0-9.]*\)%.*/\1/p' | head -n1)" + line_hit="$(echo "$SUMMARY" | sed -n 's/^[[:space:]]*lines\.*:[[:space:]]*[0-9.]*% (\([0-9]*\) of [0-9]*.*/\1/p' | head -n1)" + line_total="$(echo "$SUMMARY" | sed -n 's/^[[:space:]]*lines\.*:[[:space:]]*[0-9.]*% ([0-9]* of \([0-9]*\).*/\1/p' | head -n1)" + func_pct="$(echo "$SUMMARY" | sed -n 's/^[[:space:]]*functions\.*:[[:space:]]*\([0-9.]*\)%.*/\1/p' | head -n1)" + func_hit="$(echo "$SUMMARY" | sed -n 's/^[[:space:]]*functions\.*:[[:space:]]*[0-9.]*% (\([0-9]*\) of [0-9]*.*/\1/p' | head -n1)" + func_total="$(echo "$SUMMARY" | sed -n 's/^[[:space:]]*functions\.*:[[:space:]]*[0-9.]*% ([0-9]* of \([0-9]*\).*/\1/p' | head -n1)" + + { + echo "LINE_PCT=$line_pct" + echo "LINE_HIT=$line_hit" + echo "LINE_TOTAL=$line_total" + echo "FUNC_PCT=$func_pct" + echo "FUNC_HIT=$func_hit" + echo "FUNC_TOTAL=$func_total" + } > l1_metrics.env + + - name: Upload l1-metrics + uses: actions/upload-artifact@v4 + with: + name: l1-metrics + path: l1_metrics.env + if-no-files-found: error + - name: Caculate the code coverage summary run: | cd ./rfcMgr/gtest diff --git a/.github/workflows/generate-coverage-report.yml b/.github/workflows/generate-coverage-report.yml new file mode 100644 index 00000000..eb2d6c9d --- /dev/null +++ b/.github/workflows/generate-coverage-report.yml @@ -0,0 +1,271 @@ +name: Generate Coverage Report + +on: + workflow_run: + workflows: ["Code Coverage"] + types: [completed] + +permissions: + contents: write + pull-requests: write + +jobs: + l2-coverage: + name: L2 functional coverage metrics + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + ref: ${{ github.event.workflow_run.head_sha }} + + - name: Pull required docker images + run: | + docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/mockxconf:latest + docker pull ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest + + - name: Start mockxconf and native-platform containers + run: | + docker run -d --name mockxconf -p 50050:50050 -p 50051:50051 -p 50052:50052 -p 50054:50054 -v ${{ github.workspace }}:/mnt/L2_CONTAINER_SHARED_VOLUME ghcr.io/rdkcentral/docker-device-mgt-service-test/mockxconf:latest + docker run -d --name native-platform --link mockxconf -v ${{ github.workspace }}:/mnt/L2_CONTAINER_SHARED_VOLUME ghcr.io/rdkcentral/docker-device-mgt-service-test/native-platform:latest + + - name: Build and run L2 tests + run: | + docker exec -i native-platform /bin/bash -c "cd /mnt/L2_CONTAINER_SHARED_VOLUME && sh cov_build.sh && export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/lib/x86_64-linux-gnu:/lib/aarch64-linux-gnu:/usr/local/lib && sh run_l2.sh" + + - name: Copy L2 reports to runner + run: | + docker cp native-platform:/tmp/rfc_test_report /tmp/L2_TEST_RESULTS + ls -l /tmp/L2_TEST_RESULTS + + - name: Extract L2 metrics + shell: bash + run: | + set -euo pipefail + + feature_files=$(find test/functional-tests/features -name '*.feature' | wc -l | tr -d ' ') + feature_scenarios=$(grep -R "^[[:space:]]*Scenario:" test/functional-tests/features/ | wc -l | tr -d ' ') + feature_scenarios_disabled=$(grep -R "^[[:space:]]*#[[:space:]]*Scenario:" test/functional-tests/features/ | wc -l | tr -d ' ' || true) + + test_files=$(find test/functional-tests/tests -maxdepth 1 -name 'test_*.py' | wc -l | tr -d ' ') + test_functions=$(grep -R "^def test_" test/functional-tests/tests/ | wc -l | tr -d ' ') + test_functions_disabled=$(grep -R "^[[:space:]]*#[[:space:]]*def test_" test/functional-tests/tests/ | wc -l | tr -d ' ' || true) + + src_functions=$(grep -R "^[A-Za-z_][A-Za-z0-9_[:space:]\*]*([^;]*)[[:space:]]*{" rfcMgr/ rfcapi/ tr181api/ utils/ --include='*.cpp' --include='*.c' | wc -l | tr -d ' ') + + # Parse L2 JSON results first so pass count can drive coverage formula + python3 - <<'PY' > /tmp/l2_runtime.env + import glob + import json + + totals = {"collected": 0, "passed": 0, "failed": 0, "skipped": 0} + for path in glob.glob("/tmp/L2_TEST_RESULTS/*.json"): + try: + with open(path, "r", encoding="utf-8") as f: + data = json.load(f) + s = data.get("summary", {}) + totals["collected"] += int(s.get("collected", s.get("total", 0) or 0)) + totals["passed"] += int(s.get("passed", 0) or 0) + totals["failed"] += int(s.get("failed", 0) or 0) + totals["skipped"] += int(s.get("skipped", 0) or 0) + except Exception: + continue + + print(f"L2_COLLECTED={totals['collected']}") + print(f"L2_PASSED={totals['passed']}") + print(f"L2_FAILED={totals['failed']}") + print(f"L2_SKIPPED={totals['skipped']}") + PY + + # Source the results so L2_PASSED is available for the coverage formula + # shellcheck source=/dev/null + source /tmp/l2_runtime.env + + # Coverage = tests that actually passed / total feature scenarios + # (not 100% just because every scenario has a test function) + functional_coverage_pct=$(awk -v passed="$L2_PASSED" -v total="$feature_scenarios" 'BEGIN { if (total > 0) printf "%.2f", (passed/total)*100; else print "0.00" }') + gap_to_100_pct=$(awk -v cov="$functional_coverage_pct" 'BEGIN { printf "%.2f", 100-cov }') + + { + echo "FEATURE_FILES=$feature_files" + echo "FEATURE_SCENARIOS=$feature_scenarios" + echo "FEATURE_SCENARIOS_DISABLED=$feature_scenarios_disabled" + echo "TEST_FILES=$test_files" + echo "TEST_FUNCTIONS=$test_functions" + echo "TEST_FUNCTIONS_DISABLED=$test_functions_disabled" + echo "SRC_FUNCTIONS_APPROX=$src_functions" + echo "FUNCTIONAL_COVERAGE_PCT=$functional_coverage_pct" + echo "GAP_TO_100_PCT=$gap_to_100_pct" + cat /tmp/l2_runtime.env + } > l2_metrics.env + + { + echo "## L2 Coverage Summary" + echo "" + echo "| Metric | Value |" + echo "|---|---:|" + echo "| Feature files | $feature_files |" + echo "| Feature scenarios | $feature_scenarios |" + echo "| Test files | $test_files |" + echo "| Test functions | $test_functions |" + echo "| L2 collected | $L2_COLLECTED |" + echo "| L2 passed | $L2_PASSED |" + echo "| L2 failed | $L2_FAILED |" + echo "| L2 skipped | $L2_SKIPPED |" + echo "| Estimated functional coverage | $functional_coverage_pct% |" + echo "| Gap to 100% functional coverage | $gap_to_100_pct% |" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload L2 per-test JSON reports + uses: actions/upload-artifact@v4 + with: + name: l2-test-reports + path: /tmp/L2_TEST_RESULTS + if-no-files-found: warn + + - name: Upload L2 metrics + uses: actions/upload-artifact@v4 + with: + name: l2-metrics + path: l2_metrics.env + + update-docs: + name: Update L1/L2 coverage markdowns + runs-on: ubuntu-latest + needs: [l2-coverage] + # Only update docs when code-coverage.yml succeeded (l1-metrics exists) + if: ${{ github.event.workflow_run.conclusion == 'success' }} + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + ref: ${{ github.event.workflow_run.head_sha }} + fetch-depth: 0 + + - name: Download L1 metrics + uses: actions/download-artifact@v4 + with: + name: l1-metrics + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + path: ci-artifacts/l1 + + - name: Download L2 metrics + uses: actions/download-artifact@v4 + with: + name: l2-metrics + path: ci-artifacts/l2 + + - name: Download L2 per-test JSON reports + uses: actions/download-artifact@v4 + with: + name: l2-test-reports + path: ci-artifacts/l2-reports + + - name: Regenerate L1 and L2 markdown docs + shell: bash + run: | + set -euo pipefail + python3 test/scripts/update_coverage_docs.py \ + --reports-dir ci-artifacts/l2-reports \ + --l1-env ci-artifacts/l1/l1_metrics.env \ + --l2-env ci-artifacts/l2/l2_metrics.env + + - name: Commit docs to PR branch (same-repo PRs only) + id: commit_docs + if: ${{ github.event.workflow_run.head_repository.full_name == github.repository }} + shell: bash + run: | + set -euo pipefail + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + + git add test/docs/L1_Analysis_Report.md test/docs/L2_Analysis_Report.md + if git diff --cached --quiet; then + echo "changed=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + git commit -m "docs(coverage): refresh L1/L2 coverage for PR #${{ github.event.workflow_run.pull_requests[0].number }}" + git push origin HEAD:${{ github.event.workflow_run.head_branch }} + echo "changed=true" >> "$GITHUB_OUTPUT" + + - name: Build PR comment body + shell: bash + run: | + set -euo pipefail + source ci-artifacts/l1/l1_metrics.env + source ci-artifacts/l2/l2_metrics.env + DOC_STATUS="updated in PR branch" + if [ "${{ github.event.workflow_run.head_repository.full_name }}" != "${{ github.repository }}" ]; then + DOC_STATUS="not auto-committed (fork PR)" + elif [ "${{ steps.commit_docs.outputs.changed || 'false' }}" = "false" ]; then + DOC_STATUS="no doc changes detected" + fi + + # Extract the CI-generated component table from the updated doc + COMPONENT_TABLE=$(python3 - <<'PY' + import re, sys + with open("test/docs/L2_Analysis_Report.md", "r") as f: + content = f.read() + m = re.search(r"\n(.*?)\n", content, re.DOTALL) + print(m.group(1) if m else "_Component table not available_") + PY + ) + + { + printf '\n' + printf '## PR Coverage Summary\n\n' + printf '### L1 Code Coverage\n' + printf '| Metric | Value |\n' + printf '|---|---:|\n' + printf '| Line coverage | %s%%%% (%s / %s) |\n' "${LINE_PCT}" "${LINE_HIT}" "${LINE_TOTAL}" + printf '| Function coverage | %s%%%% (%s / %s) |\n' "${FUNC_PCT}" "${FUNC_HIT}" "${FUNC_TOTAL}" + printf '\n### L2 Functional Coverage by Component\n' + printf '%s\n' "${COMPONENT_TABLE}" + printf '\nCoverage docs status: **%s**\n' "${DOC_STATUS}" + printf 'Updated: `test/docs/L1_Analysis_Report.md` . `test/docs/L2_Analysis_Report.md`\n' + } > /tmp/pr_coverage_comment.md + + cat /tmp/pr_coverage_comment.md >> "$GITHUB_STEP_SUMMARY" + + - name: Upsert PR coverage comment + uses: actions/github-script@v7 + env: + COMMENT_PATH: /tmp/pr_coverage_comment.md + PR_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }} + with: + script: | + const fs = require('fs'); + const body = fs.readFileSync(process.env.COMMENT_PATH, 'utf8'); + const marker = ''; + const issue_number = parseInt(process.env.PR_NUMBER); + if (!issue_number) { console.log('No PR number — skipping comment'); return; } + + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number + }); + + const existing = comments.find(c => + c.user && c.user.type === 'Bot' && c.body && c.body.includes(marker) + ); + + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number, + body + }); + } diff --git a/docs/internet-check-analysis.md b/docs/internet-check-analysis.md new file mode 100644 index 00000000..929c9706 --- /dev/null +++ b/docs/internet-check-analysis.md @@ -0,0 +1,333 @@ +# RFC Module — Internet Connectivity Check Analysis + +## Overview + +This document analyzes the internet connectivity checks performed inside the RFC Manager (`rfcMgr`) binary, evaluates whether they are redundant given the platform-level guarantees provided by the caller (Maintenance Manager on video devices, `wan-initialized.target` on RDKB), and provides a recommendation. + +--- + +## 1. RFC Manager's Internal Internet Check + +### 1.1 Entry Point + +In `rfc_main.cpp`, the daemon calls `CheckDeviceIsOnline()` **before** any XConf request: + +```c +// rfc_main.cpp:131 +rfc::DeviceStatus isDeviceOnline = rfcMgr->CheckDeviceIsOnline(); +if (isDeviceOnline == rfc::RFCMGR_DEVICE_ONLINE) { + status = rfcMgr->RFCManagerProcessXconfRequest(); +} +``` + +If the device is offline, the XConf request is **skipped entirely** and on video platforms an `MAINT_RFC_ERROR` event is sent to the Maintenance Manager. + +### 1.2 Platform-Specific Check Logic + +`CheckDeviceIsOnline()` uses compile-time flags to select the connectivity test: + +```c +// rfc_manager.cpp +DeviceStatus RFCManager::CheckDeviceIsOnline() +{ +#ifdef RDKC + // Camera: Poll getifaddrs() for a routable IP (up to 5 min) + if (true == WaitForIpAcquisition()) { + result = RFCMGR_DEVICE_ONLINE; + } +#elif !defined(RDKB_SUPPORT) && !defined(RDKC) + // Video (STB/TV): Check IP route file + DNS resolver + if (true == CheckIProuteConnectivity(GATEWAYIP_FILE)) { + if (true == isDnsResolve(DNS_RESOLV_FILE)) { + result = RFCMGR_DEVICE_ONLINE; + } + } +#else + // RDKB (Broadband): Check eRouter WAN IP via dmcli + if (true == CheckIPConnectivity()) { + result = RFCMGR_DEVICE_ONLINE; + } +#endif + return result; +} +``` + +### 1.3 Check Details by Platform + +| Platform | Compile Flag | Method | What It Checks | Blocking Wait | +|---|---|---|---|---| +| **RDKV (Video)** | `!RDKB_SUPPORT && !RDKC` | `CheckIProuteConnectivity()` + `isDnsResolve()` | Gateway IP route file (`/tmp/.GatewayIP_dfltroute`) + DNS nameserver in `/etc/resolv.dnsmasq` | Polls `/tmp/route_available` every 15 s, up to 5 retries (~75 s max) | +| **RDKB (Broadband)** | `RDKB_SUPPORT` | `CheckIPConnectivity()` | eRouter WAN IPv6/IPv4 via `dmcli` | No blocking wait — single check | +| **RDKC (Camera)** | `RDKC` | `WaitForIpAcquisition()` | `getifaddrs()` for non-loopback, non-link-local IP | Polls every 10 s, up to 30 retries (~5 min max) | + +### 1.4 Commented-Out Code + +In `CheckIProuteConnectivity()` there is a commented-out block that previously called `checkDeviceInternetConnection()` for an actual HTTP connectivity probe: + +```c +/*if (true == checkDeviceInternetConnection(RFC_MGR_INTERNET_CHECK_TIMEOUT)) +{ + ip_status = true; +}*/ +``` + +This was disabled, and `ip_status` is now unconditionally set to `true` after the route file check. This means the video platform check only verifies **local network configuration**, not actual internet reachability. + +--- + +## 2. Platform-Level Internet Checks Before RFC Starts + +### 2.1 RDKV (Video) — Maintenance Manager + +On video devices, RFC is launched as a sub-task of the Maintenance Manager Thunder plugin (`entservices-maintenancemanager`). + +The `task_execution_thread()` in `MaintenanceManager.cpp` performs an internet check **before** starting RFC: + +``` +MaintenanceManager::task_execution_thread() +├── isDeviceOnline() ← Network check with retries +│ └── checkNetwork() ← Queries org.rdk.Network.1 "isConnectedToInternet" +│ └── 4 retries × 30 s = up to 2 min +├── if (!internetConnectStatus) +│ └── exitOnNoNetwork → MAINTENANCE_ERROR ← Exits entire cycle, RFC never invoked +├── (optional WhoAmI / activation checks) +└── system("Start_MaintenanceTasks.sh RFC &") + └── rfcMgr ← RFC binary starts here +``` + +**Key point:** The Maintenance Manager calls `org.rdk.Network.1::isConnectedToInternet` which performs an **actual internet reachability test** (not just local IP/route check). If the device is not connected to the internet, the maintenance cycle exits with `MAINTENANCE_ERROR` and RFC is **never invoked**. + +```mermaid +sequenceDiagram + participant MM as Maintenance Manager + participant NW as org.rdk.Network + participant RFC as rfcMgr + + MM->>NW: isConnectedToInternet() + NW-->>MM: true/false + + alt Device Offline + MM->>MM: MAINTENANCE_ERROR (exit) + Note over RFC: RFC never starts + else Device Online + MM->>RFC: Start_MaintenanceTasks.sh RFC + RFC->>RFC: CheckDeviceIsOnline() + Note over RFC: Checks IP route + DNS
(redundant — already verified) + RFC->>RFC: RFCManagerProcessXconfRequest() + end +``` + +### 2.2 RDKB (Broadband) — systemd Service + +On broadband devices, RFC starts via a systemd service: + +```ini +[Service] +Type=oneshot +ExecStartPre=/bin/sh -c 'sleep 300' +ExecStart=/bin/sh -c '/lib/rdk/rfc.service &' + +[Install] +WantedBy=wan-initialized.target +``` + +**Two layers of network assurance:** + +1. **`wan-initialized.target`** — systemd only starts the RFC service after the WAN interface has been initialized +2. **`sleep 300`** — Additional 5-minute delay to allow DHCP, DNS, and upstream connectivity to stabilize + +Then inside `rfcMgr`, before `CheckDeviceIsOnline()`: + +```c +// rfc_main.cpp (RDKB only) +waitForRfcCompletion(); // Wait for webconfig rfc_blob_processing (up to ~10 min) +``` + +After all that, `CheckDeviceIsOnline()` calls `CheckIPConnectivity()` which queries the eRouter WAN IP via `dmcli`. + +```mermaid +sequenceDiagram + participant SD as systemd + participant WAN as wan-initialized.target + participant RFC as rfcMgr + participant DM as dmcli (eRouter) + + SD->>WAN: Wait for WAN init + WAN-->>SD: WAN ready + SD->>SD: sleep 300 (5 min) + SD->>RFC: Start rfcMgr + RFC->>RFC: waitForRfcCompletion() (~10 min max) + RFC->>DM: Query eRouter WAN IP + DM-->>RFC: IPv6 or IPv4 address + RFC->>RFC: RFCManagerProcessXconfRequest() +``` + +### 2.3 RDKC (Camera) — Standalone + +Camera devices run `rfcMgr` directly. There is **no** external pre-check equivalent to the Maintenance Manager or systemd target. The internal `WaitForIpAcquisition()` is the **only** connectivity gate. + +--- + +## 3. Redundancy Analysis + +### 3.1 Summary Matrix + +| Platform | External Pre-Check | RFC Internal Check | Redundant? | Notes | +|---|---|---|---|---| +| **RDKV (Video)** | Maintenance Manager `isConnectedToInternet()` — actual internet probe with retries | `CheckIProuteConnectivity()` + `isDnsResolve()` — local route/DNS file check | **Yes, largely redundant** | MM already confirmed internet connectivity. RFC's check only verifies local file state, not actual connectivity. | +| **RDKB (Broadband)** | `wan-initialized.target` + `sleep 300` + `waitForRfcCompletion()` | `CheckIPConnectivity()` — eRouter WAN IP via `dmcli` | **Partially redundant** | WAN init + 5 min delay makes IP likely available, but `dmcli` check is a fast sanity validation of WAN state. | +| **RDKC (Camera)** | None | `WaitForIpAcquisition()` — polls for routable IP up to 5 min | **Not redundant** | This is the only connectivity gate. Removal would break RDKC. | + +### 3.2 What RFC's Check Actually Validates + +The RFC internal check does **not** verify internet connectivity on any platform: + +- **RDKV**: Checks for file existence (`/tmp/route_available`, `/tmp/.GatewayIP_dfltroute`, `/etc/resolv.dnsmasq`) and content patterns. The actual `checkDeviceInternetConnection()` HTTP probe is **commented out**. +- **RDKB**: Checks for eRouter WAN IP via `dmcli`. Having a WAN IP does not guarantee internet reachability. +- **RDKC**: Checks for any non-loopback, non-link-local IP via `getifaddrs()`. Again, an IP alone doesn't guarantee reachability. + +In all cases, if the check passes but the internet is unreachable, the `curl` call to XConf will fail anyway with a connection timeout, and the error is handled in `DownloadRuntimeFeatures()` / `ProcessRuntimeFeatureControlReq()`. + +--- + +## 4. Current Execution Flow (All Platforms) + +```mermaid +flowchart TD + A[Platform starts RFC] --> B{Platform?} + + B -->|RDKV| C[MaintenanceMgr: isDeviceOnline] + C -->|Offline| C1[MAINTENANCE_ERROR - RFC never starts] + C -->|Online| C2["Start_MaintenanceTasks.sh RFC"] + C2 --> D[rfcMgr starts] + + B -->|RDKB| E[systemd: wan-initialized + sleep 300] + E --> F["waitForRfcCompletion() ~10 min"] + F --> D + + B -->|RDKC| D + + D --> G["CheckDeviceIsOnline()"] + G -->|RDKV| H["CheckIProuteConnectivity() + isDnsResolve()"] + G -->|RDKB| I["CheckIPConnectivity() via dmcli"] + G -->|RDKC| J["WaitForIpAcquisition() up to 5 min"] + + H -->|Online| K[RFCManagerProcessXconfRequest] + I -->|Online| K + J -->|Online| K + + H -->|Offline| L["Skip XConf + MAINT_RFC_ERROR"] + I -->|Offline| M[Skip XConf - silent] + J -->|Offline| N[Skip XConf - silent] + + K --> O["curl to XConf server"] + O -->|Success| P[Process JSON response] + O -->|Failure| Q["Retry / error handling"] +``` + +--- + +## 5. Risk Assessment of Removing the Internal Check + +### 5.1 If Removed on RDKV + +| Risk | Impact | Mitigation | +|---|---|---| +| Race condition: network drops between MM check and RFC start | Low — MM check and RFC start are seconds apart | `curl` to XConf would fail with connection error, handled by retry logic | +| Maintenance Manager not running (standalone test/debug) | Medium — no pre-check, RFC would attempt XConf on potentially offline device | `curl` timeout handles this; log message would be less clear | +| `SendEventToMaintenanceManager(MAINT_RFC_ERROR)` not sent on offline | Low — MM already detected the device was offline in its own check | MM handles this in its own flow | + +### 5.2 If Removed on RDKB + +| Risk | Impact | Mitigation | +|---|---|---| +| WAN initialized but IP not yet assigned (timing window) | Low — 5 min sleep + webconfig wait make this unlikely | `curl` timeout handles this | +| eRouter loses IP after WAN init | Low | `curl` retry logic handles this | + +### 5.3 If Removed on RDKC + +| Risk | Impact | Mitigation | +|---|---|---| +| No connectivity gate whatsoever | **High** — camera would immediately attempt XConf on boot before any network is ready | None — this is the only check | + +--- + +## 6. Recommendation + +### 6.1 Keep the Internal Check (Recommended) + +**The internal internet check should be kept** for the following reasons: + +1. **Defense in depth**: The internal check provides a safety net even if the external orchestrator changes or is bypassed (debug, standalone testing, future platforms). + +2. **RDKC has no alternative**: Camera devices have no external pre-check. Removing the internal check would break RDKC entirely. + +3. **Low cost**: The checks are fast on RDKV/RDKB (file reads, single `dmcli` call). Only RDKC blocks for up to 5 minutes, which is necessary. + +4. **Different check semantics**: The Maintenance Manager checks actual internet reachability via `org.rdk.Network`. The RFC internal check validates local network configuration (routes, DNS, WAN IP). These are complementary, not identical. + +5. **Error reporting**: The internal check enables RFC-specific logging (`"IP and Route configuration not found"`, `"DNS Nameservers missing"`) and targeted IARM events (`MAINT_RFC_ERROR`) that aid triage. + +### 6.2 Suggested Improvements + +While the check should be kept, the following improvements would reduce redundancy and improve reliability: + +| # | Improvement | Platform | Rationale | +|---|---|---|---| +| 1 | **Uncomment the `checkDeviceInternetConnection()` call** in `CheckIProuteConnectivity()` | RDKV | Currently the check only validates local file state, not actual connectivity. The commented-out HTTP probe would make it a true internet check. | +| 2 | **Add a configurable skip flag** (e.g., TR-181 param or env var) | All | Allow the Maintenance Manager (or other orchestrators) to signal that internet was already verified, letting RFC skip its own check to reduce boot time. | +| 3 | **Reduce `WaitForIpAcquisition()` polling interval** on RDKC from 30 attempts × 10 s to a shorter timeout when triggered by an external orchestrator | RDKC | Future-proofing for when RDKC gains a maintenance manager. | +| 4 | **Add actual DNS resolution test** on RDKB | RDKB | `CheckIPConnectivity()` only verifies WAN IP exists. A DNS test (e.g., resolve the XConf hostname) would catch DNS failures early. | + +### 6.3 Architecture Principle + +``` +┌─────────────────────────────────────────────────────────────┐ +│ Platform Orchestrator │ +│ (MaintenanceMgr / systemd / standalone) │ +│ │ +│ ► Validates: actual internet reachability │ +│ ► Decision: start or skip entire maintenance cycle │ +└──────────────────────────┬──────────────────────────────────┘ + │ RFC is invoked only if online + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ rfcMgr (RFC Module) │ +│ │ +│ ► Validates: local network prerequisites (route, DNS, IP) │ +│ ► Decision: proceed to XConf or report error │ +│ ► Purpose: defense-in-depth, targeted error reporting │ +└──────────────────────────┬──────────────────────────────────┘ + │ Network OK + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ curl → XConf Server │ +│ │ +│ ► Final validation: actual HTTPS connection attempt │ +│ ► Handles: connection timeout, SSL errors, HTTP errors │ +└─────────────────────────────────────────────────────────────┘ +``` + +The three-layer approach (orchestrator → module → curl) provides progressively finer-grained validation and error reporting. + +--- + +## 7. Conclusion + +| Question | Answer | +|---|---| +| Is the RFC internal internet check needed? | **Yes**, it should be kept. | +| Is it redundant on RDKV? | Partially — the Maintenance Manager already validates internet connectivity before invoking RFC. However, the internal check validates local network config (routes, DNS) which is complementary. | +| Is it redundant on RDKB? | Partially — `wan-initialized.target` + sleep 300 provides strong network assurance. The `dmcli` WAN IP check is a fast, low-cost sanity validation. | +| Is it redundant on RDKC? | **No** — it is the **only** connectivity gate on camera devices. | +| Should it be removed? | **No**. It provides defense-in-depth, enables RFC-specific error logging, and is the sole gate on RDKC. Cost is negligible on RDKV/RDKB. | + +--- + +## See Also + +- [RFC Architecture](architecture.md) — System architecture overview +- [Data Processing Flow](data-processing-flow.md) — XConf request/response lifecycle +- [Sequence Diagrams](sequence-diagrams.md) — RFC communication sequences +- [Maintenance Manager Source](https://github.com/rdkcentral/entservices-maintenancemanager) — Platform orchestrator +- [L2 Test: Device Offline](../test/functional-tests/features/rfc_device_offline_status.feature) — Test for DNS file missing scenario diff --git a/test/docs/L2_Analysis_Report.md b/test/docs/L2_Analysis_Report.md index 74376e90..34e5af2c 100644 --- a/test/docs/L2_Analysis_Report.md +++ b/test/docs/L2_Analysis_Report.md @@ -11,25 +11,24 @@ This document analyzes the L2 (integration/functional) test coverage for the RFC --- **Test Coverage Summary** + + +*Not yet generated — run the CI workflow to populate this section with live per-component results.* + + +**Static Source Analysis** *(manually maintained)* ``` Total source functions (approx): ~120 -Functions with direct L2 coverage: ~30 -Functions with indirect L2 coverage: ~15 -Functions with no L2 coverage: ~75 - -Active L2 test functions: 33 -Disabled L2 test functions: 2 -Active feature scenarios: 35 -Proposed new test scenarios: 48 - - High priority: 22 - - Medium priority: 16 - - Low priority: 10 +Functions with direct L2 coverage: ~30 +Functions with indirect L2 coverage: ~15 +Functions with no L2 coverage: ~75 -Test files active: 17 -Test files disabled (commented out): 2 +Proposed new test scenarios: 48 + - High priority: 22 + - Medium priority: 16 + - Low priority: 10 -Estimated current L2 functional coverage: ~35% -Target L2 functional coverage: ~80% +Target L2 functional coverage: ~80% ``` ## 1.Current L2 Test Coverage diff --git a/test/scripts/update_coverage_docs.py b/test/scripts/update_coverage_docs.py new file mode 100644 index 00000000..c6cda07b --- /dev/null +++ b/test/scripts/update_coverage_docs.py @@ -0,0 +1,360 @@ +#!/usr/bin/env python3 +""" +update_coverage_docs.py — RFC CI coverage doc updater. + +Reads per-test pytest JSON reports plus L1/L2 aggregate env files, +then rewrites the CI-generated sections in: + test/docs/L1_Analysis_Report.md + test/docs/L2_Analysis_Report.md + +Usage: + python3 test/scripts/update_coverage_docs.py \ + --reports-dir ci-artifacts/l2-reports \ + --l1-env ci-artifacts/l1/l1_metrics.env \ + --l2-env ci-artifacts/l2/l2_metrics.env +""" + +import argparse +import glob +import json +import os +import re +import sys + + +# --------------------------------------------------------------------------- +# Component map: JSON report filename stem -> (component, [source_fns], disabled) +# Stems match the filenames produced by run_l2.sh and run_l2_reboot_trigger.sh. +# --------------------------------------------------------------------------- +COMPONENT_MAP = { + "rfc_single_instance_run": ("A. Startup & Init", ["main()", "CurrentRunningInst()"], False), + "rfc_init_failure": ("A. Startup & Init", ["GetServURL()"], False), + "rfc_override_rfc_prop": ("A. Startup & Init", ["GetServURL()"], False), + "rfc_device_offline": ("B. Device Connectivity", ["isDnsResolve()"], False), + "rfc_xconf_communication_success": ("C. XConf Communication", ["ProcessRuntimeFeatureControlReq()", "CreateXconfHTTPUrl()"], False), + "rfc_xconf_request_params": ("C. XConf Communication", ["CreateXconfHTTPUrl()"], False), + "rfc_feature_enable": ("C. XConf Communication", ["ProcessRuntimeFeatureControlReq()"], False), + "rfc_setget_param": ("D. RFC Param Mgmt", ["setRFCParameter()", "getRFCParameter()"], False), + "rfc_tr181_setget_local_param": ("D. RFC Param Mgmt", ["setLocalParam()", "getLocalParam()"], False), + "rfc_factory_reset": ("D. RFC Param Mgmt", ["processXconfResponseConfigDataPart()"], False), + "rfc_valid_accountid": ("E. AccountID Lifecycle", ["GetValidAccountId()"], False), + "rfc_trigger_reboot_unknown_accountid": ("E. AccountID Lifecycle", ["isConfigValueChange()"], False), + "rfc_unknown_accountid": ("E. AccountID Lifecycle", ["rfcCheckAccountId()"], False), + "rfc_xconf_reboot": ("F. Reboot & Maintenance", ["SendEventToMaintenanceManager()"], False), + "rfc_configsethash_time": ("G. Config Tracking", ["updateHashAndTimeInDB()"], False), + "rfc_xconf_rfc_data": ("H. Data Persistence", ["processXconfResponseConfigDataPart()"], False), + "rfc_dynamic_static_cert_selector": ("I. mTLS / Certificate", ["getMtlscert()"], True), + "rfc_static_cert_selector": ("I. mTLS / Certificate", ["getMtlscert()"], True), + "rfc_rfc_webpa": ("J. WebPA", ["IARM event handler"], False), +} + +COMPONENT_ORDER = [ + "A. Startup & Init", + "B. Device Connectivity", + "C. XConf Communication", + "D. RFC Param Mgmt", + "E. AccountID Lifecycle", + "F. Reboot & Maintenance", + "G. Config Tracking", + "H. Data Persistence", + "I. mTLS / Certificate", + "J. WebPA", +] + +# Outcome icons used in per-test detail rows +OUTCOME_ICON = { + "passed": "PASS", + "failed": "FAIL", + "error": "ERROR", + "skipped": "SKIP", + "not_run": "NOT RUN", +} + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def load_env_file(path): + """Parse a KEY=VALUE env file and return a dict.""" + env = {} + if not path or not os.path.isfile(path): + return env + with open(path, "r", encoding="utf-8") as fh: + for line in fh: + line = line.strip() + if line and not line.startswith("#") and "=" in line: + key, _, val = line.partition("=") + env[key.strip()] = val.strip() + return env + + +def load_reports(reports_dir): + """Parse per-test results from every *.json in reports_dir. + + Returns dict {stem: {"summary": {...}, "tests": [{"name":..., "outcome":...}]}} + """ + results = {} + if not reports_dir or not os.path.isdir(reports_dir): + print(f"WARNING: reports directory not found: {reports_dir}", file=sys.stderr) + return results + + for path in glob.glob(os.path.join(reports_dir, "*.json")): + stem = os.path.splitext(os.path.basename(path))[0] + try: + with open(path, "r", encoding="utf-8") as fh: + data = json.load(fh) + tests = [] + for t in data.get("tests", []): + nodeid = t.get("nodeid", "") + func_name = nodeid.split("::")[-1] if "::" in nodeid else nodeid + tests.append({"name": func_name, "outcome": t.get("outcome", "unknown")}) + results[stem] = {"summary": data.get("summary", {}), "tests": tests} + except Exception as exc: + print(f"WARNING: Could not parse {path}: {exc}", file=sys.stderr) + return results + + +def compute_component_metrics(results): + """Group test results by component. + + Returns dict {component_label: {"source_fns": set, "rows": [...]}} + where each row is {"name": str, "outcome": str, "disabled": bool}. + """ + comp_data = {c: {"source_fns": set(), "rows": []} for c in COMPONENT_ORDER} + + for stem, (comp, source_fns, disabled) in COMPONENT_MAP.items(): + for fn in source_fns: + comp_data[comp]["source_fns"].add(fn) + + if stem in results: + for t in results[stem]["tests"]: + comp_data[comp]["rows"].append( + {"name": t["name"], "outcome": t["outcome"], "disabled": disabled} + ) + elif not disabled: + # Report expected but missing — test was not run + comp_data[comp]["rows"].append( + {"name": f"[{stem} — report missing]", "outcome": "not_run", "disabled": False} + ) + + return comp_data + + +def build_ci_section(comp_data, generated_date, l2_passed, l2_collected, l2_failed, + feature_scenarios, gap_to_100): + """Return the full markdown string to insert between the CI markers.""" + lines = [] + + # ---- header ----------------------------------------------------------- + lines.append(f"*Generated: {generated_date} | L2 passed: {l2_passed}/{l2_collected}, " + f"failed: {l2_failed} | feature scenarios: {feature_scenarios}*") + lines.append("") + + # ---- per-component table ---------------------------------------------- + lines.append("| Component | Source Functions Exercised | Tests | Passed | Failed | Coverage |") + lines.append("|---|---|---:|---:|---:|---:|") + + grand_active = 0 + grand_passed = 0 + grand_failed = 0 + + for comp_label in COMPONENT_ORDER: + data = comp_data[comp_label] + rows = data["rows"] + src_str = ", ".join(f"`{fn}`" for fn in sorted(data["source_fns"])) or "—" + + # Split active vs disabled + active_rows = [r for r in rows if not r["disabled"]] + disabled_rows = [r for r in rows if r["disabled"]] + + if not active_rows and disabled_rows: + # Whole component is disabled + lines.append( + f"| {comp_label} | {src_str} | {len(disabled_rows)} " + "| DISABLED | DISABLED | *pending open-source* |" + ) + continue + + if not active_rows: + lines.append(f"| {comp_label} | {src_str} | 0 | — | — | — |") + continue + + n_total = len(active_rows) + n_passed = sum(1 for r in active_rows if r["outcome"] == "passed") + n_failed = sum(1 for r in active_rows if r["outcome"] in ("failed", "error")) + coverage = f"{n_passed / n_total * 100:.0f}%" if n_total else "—" + + grand_active += n_total + grand_passed += n_passed + grand_failed += n_failed + + lines.append( + f"| {comp_label} | {src_str} | {n_total} | {n_passed} | {n_failed} | {coverage} |" + ) + + overall = f"{grand_passed / grand_active * 100:.2f}%" if grand_active else "0%" + lines.append( + f"| **TOTAL (active)** | | **{grand_active}** " + f"| **{grand_passed}** | **{grand_failed}** | **{overall}** |" + ) + lines.append("") + lines.append(f"**Gap to 100% functional coverage: {gap_to_100}%**") + lines.append("") + + # ---- per-test detail table ------------------------------------------- + lines.append("
") + lines.append("Per-test function results (click to expand)") + lines.append("") + lines.append("| Component | Test Function | Outcome |") + lines.append("|---|---|:---:|") + + for comp_label in COMPONENT_ORDER: + data = comp_data[comp_label] + for row in data["rows"]: + icon = OUTCOME_ICON.get(row["outcome"], row["outcome"].upper()) + dis = " *(disabled)*" if row["disabled"] else "" + lines.append(f"| {comp_label} | `{row['name']}`{dis} | {icon} |") + + lines.append("") + lines.append("
") + + return "\n".join(lines) + + +# --------------------------------------------------------------------------- +# Document updaters +# --------------------------------------------------------------------------- + +def update_l2_report(doc_path, comp_data, generated_date, l2_env): + """Replace the CI-generated section in L2_Analysis_Report.md.""" + if not os.path.isfile(doc_path): + print(f"ERROR: {doc_path} not found", file=sys.stderr) + return False + + l2_passed = l2_env.get("L2_PASSED", "?") + l2_collected = l2_env.get("L2_COLLECTED", "?") + l2_failed = l2_env.get("L2_FAILED", "?") + feature_scenarios = l2_env.get("FEATURE_SCENARIOS", "?") + gap_to_100 = l2_env.get("GAP_TO_100_PCT", "?") + + ci_block = build_ci_section( + comp_data, generated_date, + l2_passed, l2_collected, l2_failed, + feature_scenarios, gap_to_100, + ) + + with open(doc_path, "r", encoding="utf-8") as fh: + content = fh.read() + + START_MARKER = "" + END_MARKER = "" + + if START_MARKER not in content or END_MARKER not in content: + print( + f"ERROR: CI markers not found in {doc_path}. " + "Add and to the document.", + file=sys.stderr, + ) + return False + + new_content = re.sub( + re.escape(START_MARKER) + r".*?" + re.escape(END_MARKER), + START_MARKER + "\n" + ci_block + "\n" + END_MARKER, + content, + count=1, + flags=re.DOTALL, + ) + + with open(doc_path, "w", encoding="utf-8") as fh: + fh.write(new_content) + print(f"Updated: {doc_path}") + return True + + +def update_l1_report(doc_path, generated_date, l1_env): + """Replace the header section of L1_Analysis_Report.md (before ## Directory Breakdown).""" + if not os.path.isfile(doc_path): + print(f"ERROR: {doc_path} not found", file=sys.stderr) + return False + + line_pct = l1_env.get("LINE_PCT", "?") + line_hit = l1_env.get("LINE_HIT", "?") + line_total = l1_env.get("LINE_TOTAL", "?") + func_pct = l1_env.get("FUNC_PCT", "?") + func_hit = l1_env.get("FUNC_HIT", "?") + func_total = l1_env.get("FUNC_TOTAL", "?") + + new_header = ( + "# L1 Code Coverage Report\n\n" + "**Test File:** coverage.info \n" + f"**Date:** {generated_date} \n\n" + "## Summary\n" + f"- **Line Coverage:** {line_hit} / {line_total} (**{line_pct}%**)\n" + f"- **Function Coverage:** {func_hit} / {func_total} (**{func_pct}%**)\n\n" + ) + + with open(doc_path, "r", encoding="utf-8") as fh: + content = fh.read() + + # Replace everything before ## Directory Breakdown + new_content = re.sub( + r"\A.*?(?=^## Directory Breakdown)", + new_header, + content, + count=1, + flags=re.DOTALL | re.MULTILINE, + ) + if new_content == content: + new_content = re.sub( + r"\A.*?(?=^---)", + new_header, + content, + count=1, + flags=re.DOTALL | re.MULTILINE, + ) + if new_content == content: + new_content = new_header + content + + with open(doc_path, "w", encoding="utf-8") as fh: + fh.write(new_content) + print(f"Updated: {doc_path}") + return True + + +# --------------------------------------------------------------------------- +# Entry point +# --------------------------------------------------------------------------- + +def main(): + parser = argparse.ArgumentParser(description="Update L1/L2 coverage markdown docs.") + parser.add_argument("--reports-dir", default="ci-artifacts/l2-reports", + help="Directory containing pytest JSON report files") + parser.add_argument("--l1-env", default="ci-artifacts/l1/l1_metrics.env", + help="Path to L1 metrics env file") + parser.add_argument("--l2-env", default="ci-artifacts/l2/l2_metrics.env", + help="Path to L2 metrics env file") + parser.add_argument("--l1-doc", default="test/docs/L1_Analysis_Report.md") + parser.add_argument("--l2-doc", default="test/docs/L2_Analysis_Report.md") + parser.add_argument("--date", default=None, + help="Override generated date (YYYY-MM-DD). Defaults to today UTC.") + args = parser.parse_args() + + from datetime import datetime, timezone + generated_date = args.date or datetime.now(timezone.utc).strftime("%Y-%m-%d") + + l1_env = load_env_file(args.l1_env) + l2_env = load_env_file(args.l2_env) + + reports = load_reports(args.reports_dir) + comp_data = compute_component_metrics(reports) + + ok_l1 = update_l1_report(args.l1_doc, generated_date, l1_env) + ok_l2 = update_l2_report(args.l2_doc, comp_data, generated_date, l2_env) + + if not ok_l1 or not ok_l2: + sys.exit(1) + + +if __name__ == "__main__": + main()