Closed Bug 1161820 Opened 5 years ago Closed 5 years ago

Replace all back button presses with Solo.goBack in tests

Categories

(Firefox for Android :: Testing, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: mcomella, Assigned: jonalmeida, Mentored)

Details

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

Attachments

(2 files)

Specifically, replace `mActions.sendSpecialKey(Actions.SpecialKey.BACK);` [1] with (usually) `mSolo.goBack();`. I'd recommend using sed.

The former method caused some issues with test failures in the past (bug 1137483 comment 34). A notable difference between the two methods is that the robotium Solo method waits 500ms after pressing the back button.

To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android

The tests you would be fixing are in our Robocop test framework, so you'll need to set that up too: https://wiki.mozilla.org/Auto-tools/Projects/Robocop

When you make changes in the mobile/android/tests/browser/robocop directory, you should only need to recompile Robocop, not the entire browser, for these changes.

While you're developing, I recommend running just a single test at a time because the full robocop test suite takes a long time to run (at least 30 minutes).

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

Thanks and happy coding! ^_^

[1]: https://mxr.mozilla.org/mozilla-central/search?string=mActions.sendSpecialKey%28Actions.SpecialKey.BACK%29%3B&find=mobile%2Fandroid%2Ftests%2Fbrowser%2Frobocop&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Mentor: michael.l.comella, liuche
Whiteboard: [lang=java][good first bug]
Hi Micheal,

I would like to work on this issue. I have just set up the build environment. I will setup Robocop today. And any set of additional instruction would be very helpful. Thank you very much.Please let me know if I can work on this. And this would be my first contribution towards Mozilla so I am very excited.

Sincerely,
Nithya
Hey, Nithya - welcome to Bugzilla! :)

Sure, go ahead and work on this bug - I'll assign you after you post a patch.

(In reply to Nithyakumaran Gnanasekar from comment #1)
> And any set of additional instruction would be very helpful.

Is there something specific that you would find helpful? I've given a general summary of what to do when solving this bug in comment 0.
Flags: needinfo?(nithyakumaran08)
Hi Micheal,

Thanks. I will work on it and get back to you incase I have any questions.

Nice know you.

Regards,
Nithya
Flags: needinfo?(nithyakumaran08)
I'm going to assign this to Jonathan because this needs to get fixed.
Assignee: nobody → jalmeida
My fix seems to build on try.
Flags: needinfo?(liuche)
Great, you should push the commit to reviewboard, and then flag me for review.
Flags: needinfo?(liuche)
Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche
Comment on attachment 8624403 [details]
MozReview Request: Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche

Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche
Attachment #8624403 - Flags: review?(liuche)
Comment on attachment 8624403 [details]
MozReview Request: Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche

https://reviewboard.mozilla.org/r/11685/#review10167

I think you're missing some replacements in testBookmarksPanel. Also, check to see if you can remove the org.mozilla.gecko.Actions imports.

One other thing that you should do is add a comment into org.mozilla.gecko.Actions stating that BACK is deprecated, and all subsequent usages should use mSolo.goBack now.
Attachment #8624403 - Flags: review?(liuche)
Comment on attachment 8624403 [details]
MozReview Request: Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche

A few comments, so re-push to reviewboard and flag me for review again.
Attachment #8624403 - Flags: feedback+
Attached patch msolo-fix.patchSplinter Review
Attachment #8624403 - Attachment is obsolete: true
Attachment #8624588 - Flags: review?(liuche)
Comment on attachment 8624588 [details] [diff] [review]
msolo-fix.patch

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

Looks good to me! (in the future, you should just use reviewboard for reviews with hg push review)

Now, you should put checkin-needed in the keywords field, and also add a comment pointing to the comment where you posted the green try push (usually you don't have to do this, but that comment was a while ago) - the sheriffs will push commits/patches that are all of the following: checkin-needed keyword, a green try push, and r+.

::: build/mobile/robocop/Actions.java
@@ +17,5 @@
> +        MENU,
> +        /**
> +         * @deprecated Use Solo.goBack() in Robocop instead.
> +         */
> +        @Deprecated

Nice.

@@ -84,5 @@
>       * will result in undefined behaviour.
>       */
>      RepeatedEventExpecter expectPaint();
>  
> -    /** 

Generally, we try not to do whitespace changes because then the hg blame points to whitespace change instead of content change, but since you're already touching this file, and these are trailing spaces on comments, I'm fine with this.
Attachment #8624588 - Flags: review?(liuche) → review+
Flags: needinfo?(jalmeida)
Flags: needinfo?(jalmeida)
Keywords: checkin-needed
Builds on try (see comment 5: https://bugzilla.mozilla.org/show_bug.cgi?id=1161820#c5) with minor comment edits added.
https://hg.mozilla.org/integration/fx-team/rev/324f8652a187
Whiteboard: [lang=java][good first bug] → [lang=java][good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/324f8652a187
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=java][good first bug][fixed-in-fx-team] → [lang=java][good first bug]
Target Milestone: --- → Firefox 41
Attachment #8624403 - Attachment is obsolete: false
Attachment #8624403 - Flags: feedback+ → review?(liuche)
Comment on attachment 8624403 [details]
MozReview Request: Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche

Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche
Comment on attachment 8624403 [details]
MozReview Request: Bug 1161820 - Replace all back button presses with Solo.goBack in tests. r=liuche

Clearing review flag, because this looks like it's already landed, more than a month ago.
Attachment #8624403 - Flags: review?(liuche)
You need to log in before you can comment on or make changes to this bug.