Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/commcare-android-pr-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -224,5 +224,5 @@ jobs:
BROWSERSTACK_PASSWORD: ${{ secrets.BROWSERSTACK_PASSWORD }}
RELEASE_APP_LOCATION: commcare-release-apk/app-commcare-release.apk
TEST_APP_LOCATION: commcare-release-androidTest-apk/app-commcare-release-androidTest.apk
BROWSERSTACK_DEVICES: "Samsung Galaxy S10-9.0"
BROWSERSTACK_DEVICES: "Samsung Galaxy S20-10.0"
PR_NUMBER: ${{ github.event.number }}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
import static androidx.test.espresso.assertion.ViewAssertions.doesNotExist;
import static androidx.test.espresso.assertion.ViewAssertions.matches;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.isRoot;
import static androidx.test.espresso.matcher.ViewMatchers.withClassName;
import static androidx.test.espresso.matcher.ViewMatchers.withId;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.commcare.utils.InstrumentationUtility.waitForView;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.endsWith;

Expand Down Expand Up @@ -126,6 +128,8 @@ public void testCaseClaimByDifferentUser() {
.perform(click());

// Close the new case of cordelia with this user.
onView(withId(R.id.screen_entity_select_list))
.perform(waitForView(withText(name), 10000, true));
onView(withText(name))
.perform(click());
onView(withText("Continue"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ import org.junit.runner.RunWith
@RunWith(AndroidJUnit4::class)
@LargeTest
@BrowserstackTests
class DialogTests: BaseTest() {

class DialogTests : BaseTest() {
companion object {
const val CCZ_NAME = "integration_test_app.ccz"
const val APP_NAME = "Integration Tests"
Expand All @@ -36,26 +35,26 @@ class DialogTests: BaseTest() {
fun testDialogCreation() {
InstrumentationUtility.openModule("Errors")
onView(withText("Error on open"))
.perform(click())
.perform(click())

checkDialogExistence_withRotation("Error Occurred")

onView(withText("Error on repeat creation"))
.perform(click())
.perform(click())
onView(withId(R.id.nav_btn_next))
.perform(click())
.perform(click())

onView(withId(R.id.choice_dialog_panel_2)).check(matches(withText("Add a new Error on add?")))

InstrumentationUtility.rotateLeft()
//TODO Expect dialog to not persist due to a activity lifecycle bug in our dialog framework.
// TODO Expect dialog to not persist due to a activity lifecycle bug in our dialog framework.
withText(R.id.choice_dialog_panel_2).doesNotExist()

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify matcher misuse patterns in instrumentation tests.
# Expected: no matches for withText(R.id....)
rg -nP 'withText\s*\(\s*R\.id\.' app/instrumentation-tests/src/org/commcare/androidTests

Repository: dimagi/commcare-android

Length of output: 201


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -nP 'withText\s*\(\s*R\.id\.' app/instrumentation-tests/src | head -n 50
echo "----"
# Show surrounding lines for the specific occurrence
sed -n '35,70p' app/instrumentation-tests/src/org/commcare/androidTests/DialogTests.kt

Repository: dimagi/commcare-android

Length of output: 1558


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -nP 'withText\s*\(\s*R\.id\.' app/instrumentation-tests/src | head -n 50
echo "----"
# Show surrounding lines for the specific occurrence
sed -n '35,70p' app/instrumentation-tests/src/org/commcare/androidTests/DialogTests.kt

Repository: dimagi/commcare-android

Length of output: 1558


Use withId instead of withText for R.id matcher in DialogTests.kt (Line 51)

withText(R.id.choice_dialog_panel_2) uses the view-id integer as a text matcher, which can invalidate the assertion. Switch to withId(R.id.choice_dialog_panel_2).

Proposed fix
-        withText(R.id.choice_dialog_panel_2).doesNotExist()
+        withId(R.id.choice_dialog_panel_2).doesNotExist()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
withText(R.id.choice_dialog_panel_2).doesNotExist()
withId(R.id.choice_dialog_panel_2).doesNotExist()
🤖 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/DialogTests.kt` at
line 51, The test is using withText(R.id.choice_dialog_panel_2) which treats the
view ID as text; update the matcher to use withId(R.id.choice_dialog_panel_2) in
DialogTests.kt (replace the withText(...) call where the assertion with
doesNotExist() is made) so the assertion targets the view ID correctly; ensure
any imports/reference to withId are available in the test file.


InstrumentationUtility.rotatePortrait()
onView(withId(R.id.nav_btn_next))
.perform(click())
.perform(click())
onView(withId(R.id.choice_dialog_panel_2))
.perform(click())
.perform(click())

checkDialogExistence_withRotation("Error Occurred")

Expand All @@ -69,8 +68,7 @@ class DialogTests: BaseTest() {
InstrumentationUtility.rotateLeft()
withText(text).isDisplayed()
InstrumentationUtility.rotatePortrait()
onView(withText("OK"))
.perform(click())
onView(withText(R.string.ok))
.perform(click())
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ import org.junit.runner.RunWith
@LargeTest
@BrowserstackTests
class FormAttachmentLimitTest : BaseTest() {
private val CCZ_NAME = "media_capture.ccz"
private val APP_NAME = "Media Capture Test"
companion object {
const val CCZ_NAME = "media_capture.ccz"
const val APP_NAME = "Media Capture Test"
}

@Before
fun setup() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public void testAttachmentUpload() {

onView(withText("Gather Signature"))
.perform(click());
InstrumentationUtility.waitForView(instanceOf(DrawView.class), 5000, true);
onView(instanceOf(DrawView.class))
.perform(new GeneralSwipeAction(Swipe.SLOW, GeneralLocation.CENTER_LEFT, GeneralLocation.TOP_RIGHT, Press.FINGER));
Comment on lines 59 to 64

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show the exact context in the test file around the reported lines
FILE="app/instrumentation-tests/src/org/commcare/androidTests/FormAttachmentUploadTest.java"
echo "---- Context: $FILE (lines 40-90) ----"
nl -ba "$FILE" | sed -n '40,90p'

# 2) Find InstrumentationUtility definition
echo "---- Locating InstrumentationUtility ----"
rg -n --hidden --no-ignore -S "class InstrumentationUtility|InstrumentationUtility" app/instrumentation-tests src . || true

# 3) Locate waitForView implementation/signature
echo "---- Locating waitForView(...) definition ----"
rg -n --hidden --no-ignore -S "waitForView\s*\(" . || true

# 4) Extract the method definition (best effort): show around matches
echo "---- Showing method bodies/usages around waitForView(...) ----"
rg -n --hidden --no-ignore -S "waitForView\s*\(" . | head -n 50

# If we can find a file that defines InstrumentationUtility, print around waitForView there
UTIL_FILE="$(rg -l --hidden --no-ignore -S "class InstrumentationUtility" . | head -n 1 || true)"
if [[ -n "${UTIL_FILE}" ]]; then
  echo "---- Context: waitForView in $UTIL_FILE ----"
  nl -ba "$UTIL_FILE" | sed -n '1,200p'
fi

# 5) Find other usages of waitForView(...) to see intended invocation pattern
echo "---- Other usages of InstrumentationUtility.waitForView(...) ----"
rg -n --hidden --no-ignore -S "InstrumentationUtility\.waitForView\s*\(" . | head -n 50

Repository: dimagi/commcare-android

Length of output: 247


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="app/instrumentation-tests/src/org/commcare/androidTests/FormAttachmentUploadTest.java"

echo "---- Context: $FILE (lines 40-90) ----"
awk 'NR>=40 && NR<=90 {printf "%5d\t%s\n", NR, $0}' "$FILE"

echo "---- Locating InstrumentationUtility ----"
rg -n --no-ignore -S "class InstrumentationUtility|InstrumentationUtility" app/instrumentation-tests src . || true

echo "---- Locating waitForView(...) definition ----"
rg -n --no-ignore -S "waitForView\s*\(" . || true

echo "---- Showing a subset of usages around waitForView(...) ----"
rg -n --no-ignore -S "InstrumentationUtility\.waitForView\s*\(" . || true

# Show around the first definition we find for InstrumentationUtility (if any)
UTIL_FILE="$(rg -l --no-ignore -S "class InstrumentationUtility" . | head -n 1 || true)"
if [[ -n "${UTIL_FILE}" ]]; then
  echo "---- Context: waitForView in $UTIL_FILE ----"
  awk 'NR>=1 && NR<=220 {printf "%5d\t%s\n", NR, $0}' "$UTIL_FILE"
fi

Repository: dimagi/commcare-android

Length of output: 50442


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show the top of the test file (imports + class header)
FILE="app/instrumentation-tests/src/org/commcare/androidTests/FormAttachmentUploadTest.java"
echo "---- File header/imports: $FILE ----"
awk 'NR>=1 && NR<=120 {print NR"\t"$0}' "$FILE"

# 2) Locate waitForView in InstrumentationUtility.kt
UTIL="app/instrumentation-tests/src/org/commcare/utils/InstrumentationUtility.kt"
if [[ -f "$UTIL" ]]; then
  echo "---- waitForView matches in $UTIL ----"
  rg -n --no-ignore -S "fun\s+waitForView|waitForView\s*\(" "$UTIL"

  echo "---- Context around waitForView definition (approx) ----"
  # Print regions around any "fun waitForView" line numbers
  LINES="$(rg -n --no-ignore -S "fun\s+waitForView" "$UTIL" | cut -d: -f1 | tr '\n' ' ')"
  for ln in $LINES; do
    start=$((ln-20)); end=$((ln+80))
    echo "---- Lines $start..$end around $ln ----"
    awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {print NR"\t"$0}' "$UTIL"
  done
else
  echo "InstrumentationUtility.kt not found at expected path: $UTIL"
fi

# 3) Search for usages in instrumentation tests only
echo "---- waitForView(...) usages under app/instrumentation-tests/src ----"
rg -n --no-ignore -S "InstrumentationUtility\.waitForView\s*\(|\bwaitForView\s*\(" app/instrumentation-tests/src --glob='*.{kt,java}' --glob='*.kt' --glob='*.java' | head -n 80

Repository: dimagi/commcare-android

Length of output: 9126


🏁 Script executed:

rg -n "isRoot|waitForView" app/instrumentation-tests/src/org/commcare/androidTests/FormAttachmentUploadTest.java

Repository: dimagi/commcare-android

Length of output: 154


Fix the waitForView step to actually execute in Espresso

InstrumentationUtility.waitForView(...) returns a ViewAction, but in FormAttachmentUploadTest it’s created and then discarded; it’s not run via onView(...).perform(...), so there’s no synchronization before swiping the DrawView (intended usage is onView(isRoot()).perform(waitForView(...))).

Suggested fix
+import static androidx.test.espresso.matcher.ViewMatchers.isRoot;
@@
-        InstrumentationUtility.waitForView(instanceOf(DrawView.class), 5000, true);
+        onView(isRoot())
+                .perform(InstrumentationUtility.waitForView(instanceOf(DrawView.class), 5000, true));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onView(withText("Gather Signature"))
.perform(click());
InstrumentationUtility.waitForView(instanceOf(DrawView.class), 5000, true);
onView(instanceOf(DrawView.class))
.perform(new GeneralSwipeAction(Swipe.SLOW, GeneralLocation.CENTER_LEFT, GeneralLocation.TOP_RIGHT, Press.FINGER));
onView(withText("Gather Signature"))
.perform(click());
onView(isRoot())
.perform(InstrumentationUtility.waitForView(instanceOf(DrawView.class), 5000, true));
onView(instanceOf(DrawView.class))
.perform(new GeneralSwipeAction(Swipe.SLOW, GeneralLocation.CENTER_LEFT, GeneralLocation.TOP_RIGHT, Press.FINGER));
🤖 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/FormAttachmentUploadTest.java`
around lines 58 - 62, The current test discards the ViewAction returned by
InstrumentationUtility.waitForView and never performs it, so replace the no-op
call with an actual perform: call
onView(isRoot()).perform(InstrumentationUtility.waitForView(instanceOf(DrawView.class),
5000, true)) before invoking the swipe; keep the subsequent
onView(instanceOf(DrawView.class)).perform(new GeneralSwipeAction(...)) as-is so
the DrawView is properly synchronized and present when swiped in
FormAttachmentUploadTest.

onView(instanceOf(DrawView.class))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class LoginTest: BaseTest() {
InstrumentationUtility.logout()

// login offline with bad password
InstrumentationUtility.changeWifi(false)
InstrumentationUtility.login("user_with_no_data", "badpass")
onView(withText("Either the password you entered was incorrect, or CommCare couldn't reach the server"))
.check(matches(isDisplayed()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
import androidx.test.filters.SdkSuppress
import org.commcare.dalvik.R
import org.commcare.utils.*
import org.commcare.utils.CustomMatchers
import org.commcare.utils.InstrumentationUtility
import org.commcare.utils.doesNotExist
import org.commcare.utils.isDisplayed
import org.hamcrest.Matchers.allOf
import org.junit.After
import org.junit.Before
Expand All @@ -23,8 +26,7 @@ import org.junit.runners.MethodSorters
@LargeTest
@SdkSuppress(maxSdkVersion = Build.VERSION_CODES.Q)
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
class ManualQuarantineTest: BaseTest() {

class ManualQuarantineTest : BaseTest() {
companion object {
const val CCZ_NAME = "ccqa.ccz"
const val APP_NAME = "Basic Tests"
Expand All @@ -49,90 +51,94 @@ class ManualQuarantineTest: BaseTest() {
InstrumentationUtility.gotoHome()
InstrumentationUtility.selectOptionItem(withText("Advanced"))
onView(withText("Enable Manual Form Quarantine"))
.perform(click())
.perform(click())
InstrumentationUtility.gotoHome()
}

@Test
fun test_A_Quarantine() {
InstrumentationUtility.openModule(MODULE_DISPLAY_FORM)
onView(withId(R.id.nav_btn_finish))
.perform(click())
.perform(click())
InstrumentationUtility.openModule(MODULE_DISPLAY_FORM)
onView(withId(R.id.nav_btn_finish))
.perform(click())
.perform(click())

// Go to saved forms and quarantine them
InstrumentationUtility.selectOptionItem(withText("Saved Forms"))

// Quarantine first form.
onView(CustomMatchers.find(
onView(
CustomMatchers.find(
allOf(withText("Display Form")),
1
)).perform(longClick())
1,
),
).perform(longClick())
// Unsent forms should not be delete-able
withText("Delete Record").doesNotExist()
onView(withText("Scan Record Integrity"))
.perform(click())
.perform(click())
onView(withText("QUARANTINE FORM"))
.perform(click())
.perform(click())

// After quarantining one form we can't quarantine another before re-enabling it from setting.
onView(withText("Display Form"))
.perform(longClick())
.perform(longClick())
onView(withText("Scan Record Integrity"))
.perform(click())
.perform(click())
withText("QUARANTINE FORM").doesNotExist()
onView(withText("OK"))
.perform(click())
onView(withText(R.string.ok))
.perform(click())

enableFormQuarantine()

// Quarantine second form
InstrumentationUtility.selectOptionItem(withText("Saved Forms"))
onView(withText("Display Form"))
.perform(longClick())
.perform(longClick())
withText("Delete Record").doesNotExist()
onView(withText("Scan Record Integrity"))
.perform(click())
.perform(click())
onView(withText("QUARANTINE FORM"))
.perform(click())
.perform(click())
}

@Test
fun test_B_FormSubmission_withQuarantineForm() {
InstrumentationUtility.selectOptionItem(withText("Saved Forms"))
onView(withId(R.id.entity_select_filter_dropdown))
.perform(click())
.perform(click())
onView(withText("Filter: Quarantined Forms"))
.perform(click())
.perform(click())

// Send 1 form back to unsent queue
onView(CustomMatchers.find(
onView(
CustomMatchers.find(
allOf(withText("Display Form")),
1
)).perform(longClick())
1,
),
).perform(longClick())
onView(withText("Add Record Back to Unsent Queue"))
.perform(click())
.perform(click())

// Confirm there is 1 form left in quarantine, and that you can delete it
onView(withText("Display Form"))
.perform(longClick())
.perform(longClick())
onView(withText("Delete Record"))
.perform(click())
.perform(click())
withText("Display Form").doesNotExist()

// Confirm 1 form is now in unsent
onView(withId(R.id.entity_select_filter_dropdown))
.perform(click())
.perform(click())
onView(withText("Filter By: Only Unsent Forms"))
.perform(click())
.perform(click())
withText("Display Form").isDisplayed()

InstrumentationUtility.changeWifi(true)
InstrumentationUtility.gotoHome()
onView(withText("Sync with Server"))
.perform(click())
.perform(click())
withText("Sync Successful! Your information is up to date.").isDisplayed()
}
}
Loading
Loading