Skip to content

[Draft] Address flaky tests#3754

Open
avazirna wants to merge 4 commits into
masterfrom
address-flaky-tests
Open

[Draft] Address flaky tests#3754
avazirna wants to merge 4 commits into
masterfrom
address-flaky-tests

Conversation

@avazirna

@avazirna avazirna commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

…esults

Product Description

Technical Summary

Safety Assurance

Safety story

Automated test coverage

Labels and Review

  • Do we need to enhance the manual QA test coverage ? If yes, RELEASES.md is updated accordingly
  • Does the PR introduce any major changes worth communicating ? If yes, RELEASES.md is updated accordingly
  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This change modifies CaseClaimTest to add explicit synchronization in the testCaseClaimByDifferentUser test flow. It introduces imports for Espresso view matchers and a waitForView utility, then inserts a wait action that pauses the test for up to 10 seconds after case creation, blocking until the case name text appears in the view hierarchy. This gating ensures subsequent test interactions proceed only after the UI has stabilized.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only empty template sections with no substantive content filled in, failing to document the change, rationale, or safety considerations. Complete the description by filling in Technical Summary with commit context, Safety Assurance with testing details, and Product Description if user-facing changes exist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title '[Draft] Address flaky tests' is vague and does not clearly describe the specific change, using generic language that obscures the actual modification. Replace with a more specific title such as 'Wait for case search results in CaseClaimTest' to clearly indicate the actual change being made.
✅ Passed checks (2 passed)
Check name Status Explanation
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch address-flaky-tests

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

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/instrumentation-tests/src/org/commcare/androidTests/CaseClaimTest.java`:
- Around line 131-132: The current wait uses waitForView(withText(name)) via
onView(isRoot()) which can match the search EditText; change the matcher to
exclude EditText so it targets the result row (e.g., combine with
not(isAssignableFrom(EditText.class)) or equivalent) and use that same matcher
both in the waitForView(...) call and when performing the subsequent click so
you synchronize on and interact with the actual case result rather than the
search field.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 24419774-58eb-4cb6-8ff8-68fe080fdd5c

📥 Commits

Reviewing files that changed from the base of the PR and between 3b36b92 and df088d8.

📒 Files selected for processing (1)
  • app/instrumentation-tests/src/org/commcare/androidTests/CaseClaimTest.java

Comment on lines +131 to +132
onView(isRoot())
.perform(waitForView(withText(name), 10000, true));

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wait condition is too broad and can pass before results load.

waitForView(withText(name), ...) can match the already-filled search EditText, so this may not actually wait for the result row. Use a matcher that excludes EditText (and reuse it for the click) so synchronization targets the case result itself.

Proposed fix
+import static org.hamcrest.Matchers.not;
@@
-        onView(isRoot())
-                .perform(waitForView(withText(name), 10000, true));
-        onView(withText(name))
+        onView(isRoot())
+                .perform(waitForView(allOf(withText(name), not(withClassName(endsWith("EditText")))), 10000, true));
+        onView(allOf(withText(name), not(withClassName(endsWith("EditText")))))
                 .perform(click());
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/instrumentation-tests/src/org/commcare/androidTests/CaseClaimTest.java`
around lines 131 - 132, The current wait uses waitForView(withText(name)) via
onView(isRoot()) which can match the search EditText; change the matcher to
exclude EditText so it targets the result row (e.g., combine with
not(isAssignableFrom(EditText.class)) or equivalent) and use that same matcher
both in the waitForView(...) call and when performing the subsequent click so
you synchronize on and interact with the actual case result rather than the
search field.

@codecov

codecov Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 24.80%. Comparing base (4f827b3) to head (2b79c62).
⚠️ Report is 136 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3754      +/-   ##
============================================
+ Coverage     24.71%   24.80%   +0.08%     
- Complexity     4224     4255      +31     
============================================
  Files           938      938              
  Lines         57220    57220              
  Branches       6847     6847              
============================================
+ Hits          14144    14191      +47     
+ Misses        41271    41224      -47     
  Partials       1805     1805              

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

@avazirna avazirna force-pushed the address-flaky-tests branch from df088d8 to d340d76 Compare June 7, 2026 18:20
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.

1 participant