Skip to content

TC_SU_2_4 - #74064

Open
r-recchia wants to merge 3 commits into
project-chip:masterfrom
r-recchia:TC_SU_2_4
Open

TC_SU_2_4#74064
r-recchia wants to merge 3 commits into
project-chip:masterfrom
r-recchia:TC_SU_2_4

Conversation

@r-recchia

@r-recchia r-recchia commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

This test fixes project-chip/matter-test-scripts#653

Testing

./out/darwin-arm64-ota-requestor/chip-ota-requestor-app --discriminator 123 --passcode 2123 --autoApplyImage
python3 src/python_testing/TC_SU_2_4.py --commissioning-method on-network --discriminator 123 --passcode 2123 --endpoint 0 --string-arg provider_app_path:./out/debug/chip-ota-provider-app --string-arg provider_app_pipe:/tmp/provider_2_4_fifo --string-arg provider_app_pipe_out:/tmp/provider_2_4_fifo_out --string-arg ota_image:firmware_v2.ota --int-arg ota_image_download_timeout:360 --timeout 2100 --PICS src/app/tests/suites/certification/ci-pics-values

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7f80e8ef-9818-4b6b-b54a-f9ee8b0ec561

📥 Commits

Reviewing files that changed from the base of the PR and between 270b78e and 6016a96.

📒 Files selected for processing (1)
  • src/python_testing/TC_SU_2_4.py

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The OTA provider now records update tokens and the requested software version. Its snapshots expose these values to tests. A new Python TC-SU-2.4 test validates OTA state transitions and compares ApplyUpdateRequest values with the preceding QueryImageResponse. The legacy YAML test definition is removed. The Python test is added to nightly metadata and the nightly workflow filter.

Sequence Diagram(s)

sequenceDiagram
  participant Test as TC_SU_2_4
  participant Provider as OTA provider
  participant DUT as OTA requestor
  Test->>Provider: Start and commission provider
  Test->>DUT: Announce OTA provider
  DUT->>Provider: QueryImage
  Provider-->>DUT: QueryImageResponse with UpdateToken
  DUT->>Provider: ApplyUpdateRequest with UpdateToken and NewVersion
  Test->>Provider: Read snapshots
  Test-->>Test: Verify token and version values
Loading

Suggested reviewers: jtrejoespinoza-grid

Merge Risk: ⚪ Minimal · up to 6016a

The change adds OTA request observability and nightly TC-SU-2.4 validation without identified merge-blocking impact.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The pull request adds a dedicated TC-SU-2.4 test instead of showing whether the verification can be covered by an existing step in TC-SU-2.2 or TC-SU-2.3, as requested by issue #653. Provide evidence that TC-SU-2.2 and TC-SU-2.3 cannot cover this verification, or move the verification into the applicable existing test and avoid the additional dedicated test.
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The changes remain focused on TC-SU-2.4 verification, including provider instrumentation, test registration, test migration, and nightly execution. No unrelated code changes are evident.
Title check ✅ Passed The title identifies the specific TC_SU_2_4 test affected by the changes. It is concise and directly related to the primary change.
Description check ✅ Passed The description explains that the pull request fixes TC-SU-2.4, references the related issue, and provides testing commands for the new test implementation.
  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.91%. Comparing base (fc18ade) to head (6016a96).
⚠️ Report is 24 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #74064      +/-   ##
==========================================
- Coverage   57.42%   56.91%   -0.51%     
==========================================
  Files        1940     1985      +45     
  Lines      127630   129597    +1967     
  Branches    14495    15060     +565     
==========================================
+ Hits        73287    73758     +471     
- Misses      54343    55839    +1496     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jtrejoespinoza-grid jtrejoespinoza-grid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small subscription that is not used can be removed.

Comment thread src/python_testing/TC_SU_2_4.py Outdated
Comment thread src/python_testing/TC_SU_2_4.py Outdated
Comment thread src/python_testing/TC_SU_2_4.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TC-SU-2.4: Can this be verified in another test?

2 participants