Closed Bug 1236864 Opened 5 years ago Closed 4 years ago

Create RobotiumHelper.waitForExactText() and RobotiumHelper.searchExactText()

Categories

(Firefox for Android :: Testing, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: sebastian, Assigned: daniel, Mentored)

References

Details

(Whiteboard: [lang=java][good first bug])

Attachments

(1 file, 1 obsolete file)

Follow-up from bug 1233693.

Robotium's methods to search and wait for text actually do not exactly match the text but treat the text as regex pattern. This means search or waiting for "Page" will match every text that contains "Page".

In bug 1233693 I fixed this locally in AppMenuComponent but we want to have a generic helper for that.

* Create RobotiumHelper in the tests/helpers/ directory 
* Add methods for waitForExactText() and searchExactText()
* Use a regex pattern that searches for the exact text - see code in AppMenuComponent
* Use Pattern.quote() on the text to avoid side effects
* Replace code in AppMenuComponent with the new helper
Attached patch Bug 1236864 Patch (obsolete) — Splinter Review
Hello. I tried my hand at fixing this bug; I hope that's OK. One clarification: There are multiple methods with different signatures for Solo.searchText() and Solo.waitForText(). I implemented helpers for the methods that you used in AppMenuComponent, but not the others. Is this what you were looking for, or would it be best to make helpers for all the different methods?
Thanks,
Daniel
Flags: needinfo?(s.kaspari)
Attachment #8707966 - Flags: review?(s.kaspari)
(In reply to Daniel Vucci from comment #1)
> Hello. I tried my hand at fixing this bug; I hope that's OK. One
> clarification: There are multiple methods with different signatures for
> Solo.searchText() and Solo.waitForText(). I implemented helpers for the
> methods that you used in AppMenuComponent, but not the others. Is this what
> you were looking for, or would it be best to make helpers for all the
> different methods?

That's perfect. Just implement the methods that are actually needed/used.
Assignee: nobody → daniel
Status: NEW → ASSIGNED
Flags: needinfo?(s.kaspari)
Comment on attachment 8707966 [details] [diff] [review]
Bug 1236864 Patch

Review of attachment 8707966 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Thanks! I'd remove some of the empty lines but this shouldn't stop us.

I'll push the patch to try to run the tests.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/helpers/RobotiumHelper.java
@@ +13,5 @@
> +/**
> + * Provides helper functions for using Robotium.
> + */
> +public final class RobotiumHelper {
> +

NIT: Remove empty line

@@ +29,5 @@
> +    public static boolean waitForExactText(final String text,
> +                                           final int minimumNumberOfMatches,
> +                                           final long timeout) {
> +        String matchText = "^" + Pattern.quote(text) + "$";
> +

NIT: Remove empty line

@@ +39,5 @@
> +     */
> +    public static boolean searchExactText(final String text,
> +                                          final boolean onlyVisible) {
> +        String matchText = "^" + Pattern.quote(text) + "$";
> +

NIT: Remove empty line
Attachment #8707966 - Flags: review?(s.kaspari) → review+
Thank you for doing the review. I've attached an updated patch that addresses the NITs. Is the next step to add the checkin-needed keyword to this bug now? Regards, Daniel
Attachment #8707966 - Attachment is obsolete: true
Flags: needinfo?(s.kaspari)
(In reply to Daniel Vucci from comment #5)
> Created attachment 8709139 [details] [diff] [review]
> Bug 1236864 Patch - Fixed NITs
> 
> Thank you for doing the review. I've attached an updated patch that
> addresses the NITs. Is the next step to add the checkin-needed keyword to
> this bug now? Regards, Daniel

Yeah, some tasks on treeherder seem to stuck. I re-triggered them and everything looks good.
Flags: needinfo?(s.kaspari)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/842eb52cc03f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
You need to log in before you can comment on or make changes to this bug.