Skip to content

Answer both requests the command sends - #5681

Merged
bgoncal merged 1 commit into
mainfrom
cover-turn-on-off-card
Sep 7, 2026
Merged

Answer both requests the command sends#5681
bgoncal merged 1 commit into
mainfrom
cover-turn-on-off-card

Conversation

@bgoncal

@bgoncal bgoncal commented Sep 7, 2026

Copy link
Copy Markdown
Member

AI Policy

Select exactly one option that describes AI usage in this contribution:

  • I have not used AI for this contribution.
  • AI assistance was used for this contribution.
  • AI fully generated the code for this contribution, but I've reviewed and understood it before submitting and will respond without AI during review.

Summary

Follows #5677, which landed at 84.61% patch coverage. The two uncovered lines were the ones that build the result card.

The command sends two requests — the service, then the state it reads back to describe — and the test answered only the first. The state read never completed, so the card was never built and the lines that build it never ran, while the assertion on the service still passed. The test now answers both.

Screenshots

Tests only.

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

No behaviour change.

It calls the service and then reads the state back to describe. Leaving the
second unanswered meant the card was never built, so the lines that build it
went unexercised while the test still passed.
Copilot AI lite review requested due to automatic review settings September 7, 2026 08:50

Copilot AI 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.

🟡 Changes recommended

The test helper currently swallows intent.perform() errors with try?, which can allow false-positive passes even if the intent failed and the intended coverage path still didn’t execute.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the TurnOnOffEntityAppIntent unit tests to properly respond to both network requests triggered by perform() (the service call and the subsequent state readback), ensuring the snippet/card-building code path is exercised and patch coverage reflects the real execution flow.

Changes:

  • Adds a decodable mock HAData entity-state response to support the readback decode used for building the result card.
  • Updates the helper that drains HAMockConnection.pendingRequests to answer two requests in order and still capture the service name for assertions.
  • Passes the entity id into the helper so the mocked readback aligns with the intent’s target entity.
File summaries
File Description
Tests/App/Utilities/TurnOnOffEntityAppIntentTests.swift Updates intent test plumbing to answer both the service request and the follow-up entity state readback so the snippet/card path executes.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 72 to +73
_ = try? await task.value
return request?.data["service"] as? String
return first?.data["service"] as? String
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ Unused L10n strings detected

Found 6 unused localization strings in the codebase.

Click to see details
Parsing Strings.swift...
Found 2809 L10n strings

Reading all Swift source code...
Read 11310055 characters of Swift code

Checking for unused strings...
Checked 100/2809 strings...
Checked 200/2809 strings...
Checked 300/2809 strings...
Checked 400/2809 strings...
Checked 500/2809 strings...
Checked 600/2809 strings...
Checked 700/2809 strings...
Checked 800/2809 strings...
Checked 900/2809 strings...
Checked 1000/2809 strings...
Checked 1100/2809 strings...
Checked 1200/2809 strings...
Checked 1300/2809 strings...
Checked 1400/2809 strings...
Checked 1500/2809 strings...
Checked 1600/2809 strings...
Checked 1700/2809 strings...
Checked 1800/2809 strings...
Checked 1900/2809 strings...
Checked 2000/2809 strings...
Checked 2100/2809 strings...
Checked 2200/2809 strings...
Checked 2300/2809 strings...
Checked 2400/2809 strings...
Checked 2500/2809 strings...
Checked 2600/2809 strings...
Checked 2700/2809 strings...
Checked 2800/2809 strings...

================================================================================
UNUSED STRINGS REPORT
================================================================================

Found 6 unused strings:


APPINTENTS:
  - L10n.AppIntents.ActiveEntities.Filter.climates
    Key: app_intents.active_entities.filter.climates
    Line: 297
  - L10n.AppIntents.ActiveEntities.Filter.covers
    Key: app_intents.active_entities.filter.covers
    Line: 299
  - L10n.AppIntents.ActiveEntities.Filter.fans
    Key: app_intents.active_entities.filter.fans
    Line: 301
  - L10n.AppIntents.ActiveEntities.Filter.locks
    Key: app_intents.active_entities.filter.locks
    Line: 305
  - L10n.AppIntents.ActiveEntities.Filter.mediaPlayers
    Key: app_intents.active_entities.filter.media_players
    Line: 307
  - L10n.AppIntents.ActiveEntities.Filter.switches
    Key: app_intents.active_entities.filter.switches
    Line: 311

================================================================================
Total unused: 6
================================================================================

================================================================================
Copy-paste these keys into the "Lokalise: Delete Keys" workflow (keys input):
================================================================================
app_intents.active_entities.filter.climates,app_intents.active_entities.filter.covers,app_intents.active_entities.filter.fans,app_intents.active_entities.filter.locks,app_intents.active_entities.filter.media_players,app_intents.active_entities.filter.switches

To remove them, run the
Lokalise: Delete Keys
workflow — it deletes the keys from Lokalise and opens a PR removing them from
Localizable.strings and regenerating Strings.swift. Copy-paste these keys into the keys input:

app_intents.active_entities.filter.climates,app_intents.active_entities.filter.covers,app_intents.active_entities.filter.fans,app_intents.active_entities.filter.locks,app_intents.active_entities.filter.media_players,app_intents.active_entities.filter.switches

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.69%. Comparing base (2536e2a) to head (6a7c6db).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5681      +/-   ##
==========================================
- Coverage   38.86%   38.69%   -0.18%     
==========================================
  Files        1069     1069              
  Lines       75564    75278     -286     
==========================================
- Hits        29371    29130     -241     
+ Misses      46193    46148      -45     

see 43 files with indirect coverage changes

☔ 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.

@bgoncal
bgoncal merged commit f5d22c6 into main Sep 7, 2026
20 of 21 checks passed
@bgoncal
bgoncal deleted the cover-turn-on-off-card branch September 7, 2026 10:18
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.

2 participants