Skip to content

Pin the Health Connect delete-gate test to Health Connect (fails on macOS) - #685

Merged
abdulsaheel merged 3 commits into
OpenStrap:mainfrom
kurexw:fix/health-export-test-macos
Nov 13, 2026
Merged

abdulsaheel merged 3 commits into
OpenStrap:mainfrom
kurexw:fix/health-export-test-macos

Conversation

@kurexw

@kurexw kurexw commented Nov 10, 2026 •

Copy link
Copy Markdown

Why

health_workout_export_delete_gate_test ("a failed delete does not write a duplicate on top of it") fails on every Mac and passes in CI.

The exportWorkout delete-then-write group tests the Health Connect reading of a false delete, and its comment assumed "the host VM is neither iOS nor macOS". But HealthExporter.isApple is Platform.isIOS || Platform.isMacOS, and flutter test on a Mac runs on a macOS host VM. There isApple is true, the exporter takes the HealthKit branch (where a false delete is the documented empty-range answer and the write goes ahead, by design), and the test's Health Connect expectation fails. CI runs on ubuntu-latest, so it never sees it; a contributor on a Mac sees a red test unrelated to their change.

The app is fine on both platforms. Only the test was host-dependent.

What changes

  • lib/health/health_export.dart: a @visibleForTesting static bool? isAppleOverride, read first by isApple. null (the default, and always in the app) keeps the real platform check, so production behaviour is unchanged. Same pattern as ScreenWake.platformOverride in lib/gps/screen_wake.dart.
  • test/health_workout_export_delete_gate_test.dart: the group pins isAppleOverride = false in setUp and resets it in tearDown, so it tests Health Connect on every host.

Tests

  • test/health_workout_export_delete_gate_test.dart: 11/11 pass on macOS (was 10/11).
  • flutter analyze: no issues.
  • flutter test --concurrency=1: 7010 passed, 28 skipped, 8 failed. The 8 are the ones fix main's test run: missing translations and gallery case #683 fixes (ui2_tokens gallery, russian_localization, russian_review_findings, settings_report_l10n es/fr/hi/zh, thai_l10n); this test is no longer among them.

Not related to #683, which fixes the other red tests on main (translations, gallery); this one is a macOS-only failure #683 doesn't cover.

🤖 Generated with Claude Code

Summary by Sourcery

Make Health Connect delete-gate tests platform-independent without changing production behavior.

Bug Fixes:

  • Make Health Connect delete-gate tests deterministic across host platforms by allowing tests to select the platform behavior they exercise.

Enhancements:

  • Route health export decisions through the selected platform behavior while preserving the real platform check by default.

Tests:

  • Pin the delete-gate test group to Health Connect behavior so it no longer fails on macOS.

Summary by CodeRabbit

  • Chores
    • No user-facing changes in this release. Apple-platform behavior remains unchanged in normal use.

The `exportWorkout delete-then-write` group tests the Health Connect
reading of a false delete, and assumed a non-Apple host. flutter test
on a Mac runs on a macOS host VM, where HealthExporter.isApple is true:
the exporter takes the HealthKit branch (a false delete is the
documented empty-range answer and the write proceeds, by design) and
"a failed delete does not write a duplicate" fails. CI runs on Linux,
so only contributors on a Mac saw it.

Add a @VisibleForTesting HealthExporter.isAppleOverride (null = the
real platform, so the app is unchanged), the same pattern as
ScreenWake.platformOverride, and pin it to false in that group.
@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

HealthExporter accepts an optional Apple-platform test override. When set, export logic uses the selected platform for platform-specific behavior. Otherwise, it uses the actual platform.

Changes: Apple platform detection and export behavior

Layer / File(s) Summary
Apple platform override
lib/health/health_export.dart
Adds an optional nullable test override. The internal platform decision uses it when set and otherwise checks the actual platform. The public storeName remains based on the actual platform.
Platform-specific export behavior
lib/health/health_export.dart
Uses the selected platform for sleep-envelope filtering and writing, deletion semantics, HRV type and value, workout energy, and activity mapping.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other · Severity of issue fixed: Low

Possibly related PRs

  • OpenStrap/edge#653: Reports a macOS Health export test failure caused by an assumption about the host platform’s isApple value.

Suggested reviewers: abdulsaheel

Merge Risk: Minimal · up to 72592

This adds a test-only platform override to the health exporter. Production behavior is unchanged when the override is not set, and no merge-blocking risk was found.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the test being pinned and the platform it fixes, which matches the main change (a test-only platform override plus pinning the delete-gate test group to Health Connect). It is specific…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the test being pinned and the platform it fixes, which matches the main change (a test-only platform override plus pinning the delete-gate test group to Health Connect). It is specific, but it is long (74 characters) and the parenthetical reads like a failure report, so it is not a perfect summary of the change. The title is clear enough for a teammate scanning history to see the primary change.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: OpenStrap/edge
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: 3fd7f37
  • Layers: Repository YAML
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 84fe9d32-c55a-4299-b275-5341eb6477e3
Commits

Reviewing files that changed from the base of the PR and between 7382f7e and 72592d7.

Files ignored due to path filters: 1

  • test/health_workout_export_delete_gate_test.dart is excluded by !test/**
Files selected for processing: 1

  • lib/health/health_export.dart

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

@sourcery-ai

sourcery-ai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

API

Add a test-only constructor override so callers can pin the exporter’s platform selection without changing the production default.

  • lib/health/health_export.dart L541-548

Implementation

Resolve the effective Apple-platform flag from the override or the real platform, and use it consistently for platform-specific export behavior.

  • lib/health/health_export.dart L635-639 (+10 small edits)

Tests

Pin the delete-then-write tests to Health Connect so their expectations do not depend on the host operating system.

  • test/health_workout_export_delete_gate_test.dart L88-189 (8 small edits)

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@kurexw
kurexw marked this pull request as ready for review November 10, 2026 15:52

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@kurexw

kurexw commented Nov 11, 2026

Copy link
Copy Markdown
Author

Closing this myself: it only matters when running the suite on macOS, which CI doesn't do. Glad to reopen if it's wanted.

@abdulsaheel
abdulsaheel merged commit 652bfe2 into OpenStrap:main Nov 13, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants