Skip to content

Conversation

rfc2822
Copy link
Member

@rfc2822 rfc2822 commented Sep 10, 2025

Purpose

Currently test runs are randomly failing because AndroidSyncFrameworkTest doesn't correctly take optional sync states into account.

Short description

This PR optimizes handling of partial state comparision and adds a "full match required" flag that disables partial state comparision.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@rfc2822 rfc2822 self-assigned this Sep 10, 2025
@rfc2822 rfc2822 added the bug Something isn't working label Sep 10, 2025
@rfc2822 rfc2822 requested a review from Copilot September 10, 2025 09:51
Copilot

This comment was marked as outdated.

@rfc2822 rfc2822 requested a review from Copilot September 10, 2025 09:57
Copilot

This comment was marked as outdated.

@rfc2822 rfc2822 force-pushed the 1681-testverifysyncalwayspending_wrongbehaviour_android14-still-sometimes-fails branch from 75ada59 to 6a6df4c Compare September 10, 2025 09:57
@rfc2822 rfc2822 requested a review from Copilot September 10, 2025 09:58
Copilot

This comment was marked as outdated.

@rfc2822 rfc2822 force-pushed the 1681-testverifysyncalwayspending_wrongbehaviour_android14-still-sometimes-fails branch from 6a6df4c to 3c0dc34 Compare September 10, 2025 10:01
@rfc2822 rfc2822 requested a review from Copilot September 10, 2025 10:01
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes test flakiness in AndroidSyncFrameworkTest by improving the state matching logic to properly handle optional sync states during partial and full comparisons.

  • Refactored state assertion logic by separating comparison from error handling
  • Added a fullMatch parameter to control whether all expected states must be matched
  • Added validation to ensure the last expected state is non-optional

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@rfc2822 rfc2822 marked this pull request as ready for review September 10, 2025 10:16
Copy link
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Context is not used, so we don't need to inject it. Furthermore no mocks are used so we don't need the mock rule. Looks good to me otherwise though.

@rfc2822 rfc2822 requested a review from sunkup September 11, 2025 10:37
@rfc2822 rfc2822 requested a review from ArnyminerZ September 12, 2025 08:29
- Add `fullMatch` parameter to control whether all expected states must be present
- Update `onStatusChanged` to override the interface method.
- Replace custom assertion with `assertTrue` for state comparison.
@rfc2822 rfc2822 force-pushed the 1681-testverifysyncalwayspending_wrongbehaviour_android14-still-sometimes-fails branch from 468eb7b to a7ba1af Compare September 15, 2025 06:56
@rfc2822 rfc2822 merged commit 00523d9 into main-ose Sep 16, 2025
9 checks passed
@rfc2822 rfc2822 deleted the 1681-testverifysyncalwayspending_wrongbehaviour_android14-still-sometimes-fails branch September 16, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

testVerifySyncAlwaysPending_wrongBehaviour_android14 still sometimes fails
3 participants