Closed
Bug 822256
Opened 13 years ago
Closed 12 years ago
Robocop: Add test for 'System Pages' feature
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 21
People
(Reporter: paul.feher, Assigned: paul.feher)
References
Details
Attachments
(1 file, 10 obsolete files)
6.58 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
This bug tracks the Robocop tests part of System Pages feature on Firefox for Android.
The tests check first the loading of system pages from the awesome bar and then from Firefox menu.
about:rights https://moztrap.mozilla.org/manage/case/1029/
Opening a local page (about:) in another local page should open in the same tab: https://moztrap.mozilla.org/manage/case/1030/
The search input is not in focus https://moztrap.mozilla.org/manage/case/1033/
about:home and about:firefox: https://moztrap.mozilla.org/manage/case/1027/
Assignee | ||
Comment 1•13 years ago
|
||
Modified the selectMenuItem method in BaseTest.java.in to cover the changes of the menu layout (the tools submenu) on ICS+ devices.
Created tests to cover the tests done on system pages.
Assignee | ||
Comment 2•13 years ago
|
||
Cleaned up some spaces
Attachment #693333 -
Attachment is obsolete: true
Attachment #693335 -
Flags: review?(jmaher)
Comment 3•13 years ago
|
||
Comment on attachment 693335 [details] [diff] [review]
SystemPages test V1.1
Review of attachment 693335 [details] [diff] [review]:
-----------------------------------------------------------------
Great start on this.
A few nits:
* clean up all whitespace. Look for pink in the splinter review
* ensure all indentation is correct at 4 spaces for each level of code
As mentioned in the rest of the review, lets refactor the test to make it more compact and reuse code as much as possible by utilizing simple loops.
::: mobile/android/base/tests/BaseTest.java.in
@@ -337,2 @@
> //build the item name ready to be used
> - String itemName = "^" + menuItemName + "$";
we did we remove the ^ and $ from item name? that will ensure we match on the name and only the name.
@@ +350,4 @@
>
> mSolo.waitForText(itemName);
> + mSolo.clickOnText(itemName);
> + }
please indent this properly and remove the trailing whitespace.
::: mobile/android/base/tests/testSystemPages.java.in
@@ +36,5 @@
> + loadAndPaint(aboutFirefox);
> + verifyTabCount(expectedTabCount);
> +
> + // I've used here verifyUrl because verifyPageTitle fails at this step
> + verifyUrl(aboutFirefox);
is there a bug about this? What is the title? I am thinking we could make this a simple loop:
for (url in urls) {
title = url
if (url == about:firefox)
title = '';
loadAndPaint(url);
verifyPageTitle(title);
verifyTabCount(expectedTabCount);
}
@@ +72,5 @@
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +
> + PaintedSurface painted = waitForPaint(paintExpecter);
> +
> + /* Verify that the search field is not in the focus by verifying that when
nit: extra space after the 'when'
@@ +87,5 @@
> +
> + // Set up listeners to catch the page load we're about to do
> + Actions.EventExpecter tabEventExpecter = mActions.expectGeckoEvent("Tab:Added");
> + Actions.EventExpecter contentEventExpecter = mActions.expectGeckoEvent("DOMContentLoaded");
> +
nit: extra white space on this line and at the end of the previous line.
@@ +118,5 @@
> + verifyTabCount(expectedTabCount);
> +
> + // Set up listeners to catch the page load we're about to do
> + tabEventExpecter = mActions.expectGeckoEvent("Tab:Added");
> + contentEventExpecter = mActions.expectGeckoEvent("DOMContentLoaded");
for page in [download, apps, about]:
tabEventExpecter = ...
contentEventExpecter = ...
//Load page
select MenuItem
expectedTabcount++
// wait for the new tab
tabEventExpecter.blockForEvent()
contentEventExpecter.blockForEvent()
This can simplify repeated code.
Attachment #693335 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 4•13 years ago
|
||
Made all the changes asked for in the review.
Attachment #693335 -
Attachment is obsolete: true
Assignee | ||
Comment 5•13 years ago
|
||
Cleaned up some spaces
Attachment #694346 -
Attachment is obsolete: true
Attachment #694347 -
Flags: review?(jmaher)
Comment 6•13 years ago
|
||
Comment on attachment 694347 [details] [diff] [review]
SystemPages test V2.1
Review of attachment 694347 [details] [diff] [review]:
-----------------------------------------------------------------
I really like how cleaned up this is. The looping makes this scalable and easier to read.
r- because this fails, although it might be a simple fix.
::: mobile/android/base/tests/testSystemPages.java.in
@@ +12,5 @@
> +Actions.EventExpecter tabEventExpecter;
> +Actions.EventExpecter contentEventExpecter;
> +Actions.RepeatedEventExpecter paintExpecter;
> +
> +final int expectedTabCount = 1;
indentation of these member variables
@@ +22,5 @@
> +
> + public void testSystemPages() {
> + blockForGeckoReady();
> +
> + String Urls [] = { "about:firefox", "about:rights", "about:home", "about:addons", "about:downloads", "about:buildconfig", "about:", "about:config"};
nit: lowercase: urls
@@ +24,5 @@
> + blockForGeckoReady();
> +
> + String Urls [] = { "about:firefox", "about:rights", "about:home", "about:addons", "about:downloads", "about:buildconfig", "about:", "about:config"};
> + String expectedUrls [] = { "about:apps", "about:downloads", "about:addons", "about:"};
> + String MenuItems [] = { "Apps", "Downloads", "Add-ons", "Settings"};
nit: menuItems
@@ +28,5 @@
> + String MenuItems [] = { "Apps", "Downloads", "Add-ons", "Settings"};
> +
> + /* This first section loads system pages from the awesome bar
> + and checks that the pages are loaded in the same tab */
> + ceckUrl(Urls);
can we call this checkUrl?
@@ +35,5 @@
> + the awesomebar and the keyboard, then verify that the previous about: page is loaded */
> + paintExpecter = mActions.expectPaint();// Set up listener to catch the page load
> +
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
is there a chance we could close the browser if we were not in the right place?
@@ +38,5 @@
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +
> + PaintedSurface painted = waitForPaint(paintExpecter);
> + verifyUrl("about:");
I get this on all my test runs:
12-21 00:45:09.366 I/Robocop (28041): 26 INFO TEST-UNEXPECTED-FAIL | testSystemPages | Awesomebar URL stayed the same - got about:config, expected about:
Is it possible that after clearing the keyboard and awesomebar that we are at the lats page in the url list "about:config" and this is working properly?
@@ +49,5 @@
> + public void ceckUrl(String Urls []) {
> + for (String url:Urls) {
> + loadAndPaint(url);
> + verifyTabCount(expectedTabCount);
> + verifyUrl(url);
didn't we have to do something different for about:firefox last time?
@@ +53,5 @@
> + verifyUrl(url);
> + }
> + }
> + public void loadFromMenu(String MenuItems [], String expectedUrls []) {
> + int mExpectedTabCount = expectedTabCount;
please use mExpectedTabCount for the class level variable:
int expectedTabCount = mExpectedTabCount;
@@ +58,5 @@
> + for (String item:MenuItems) {
> + mExpectedTabCount++;
> + int i = Arrays.asList(MenuItems).indexOf(item);
> + if (item != "Settings") {
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
we send the BACK key for both conditions, lets pull this out of the if/else clauses.
Attachment #694347 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 7•12 years ago
|
||
Made all the changes asked for in the review.
Attachment #694347 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #697434 -
Flags: review?(jmaher)
Comment 8•12 years ago
|
||
Comment on attachment 697434 [details] [diff] [review]
SystemPages test V3
Review of attachment 697434 [details] [diff] [review]:
-----------------------------------------------------------------
overall this is great. A couple small code style things, and the obvious thing that it fails to loadmenu items.
::: mobile/android/base/tests/testSystemPages.java.in
@@ +32,5 @@
> + checkUrl(urls);
> +
> + /* Verify that the search field is not in the focus by pressing Back to dismiss
> + the keyboard, then verify that the previous about: page is loaded */
> + loadUrl("about:config");
nit: whitespace after the ;
@@ +57,5 @@
> + expectedTabCount++;
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +
> + int i = Arrays.asList(menuItems).indexOf(item);
> + if (item != "Settings") {
can we do:
if (!item.equals("Settings")) {
@@ +63,5 @@
> + // Set up listeners to catch the page load we're about to do
> + tabEventExpecter = mActions.expectGeckoEvent("Tab:Added");
> + contentEventExpecter = mActions.expectGeckoEvent("DOMContentLoaded");
> +
> + selectMenuItem(item);
this is failing for me. It is failing to find '^Tools$'. This is on the panda board. From the adb logcat, it appears to be in the awesomebar, I tried tweaking this to add another menu.back and it still didn't work;
Attachment #697434 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 9•12 years ago
|
||
Made all the changes asked for in the review. Also I introduced a delay before the menu item is searched to be sure that the page loads.
I've verify this patch on several devices with different OS's:
Samsung galaxy R - Android 2.3.4
Samsung galaxy S2 - Android 4.0.3
Acer Iconia Tab A500 - Android 3.1
Asus EE Transformer TF101 - Android 4.0.3
Works every time for me.
Attachment #697434 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #697855 -
Flags: review?(jmaher)
Comment 10•12 years ago
|
||
Comment on attachment 697855 [details] [diff] [review]
SystemPages test V4
Review of attachment 697855 [details] [diff] [review]:
-----------------------------------------------------------------
the code looks great, but the test still fails. It passes on the panda board with android 4.0.4, but it is failing on our tegra platform. I tried to debug this and there is no Tools available. I found that if I hacked around in the changes to BaseTest.java.in I could click the item all the time except for settings. This is odd because the failure I see is (from logcat, but in mochitest log):
01-05 00:58:42.596 I/Robocop (25731): 30 INFO TEST-PASS | testSystemPages | Awesomebar URL stayed the same - about:addons should equal about:addons
01-05 00:59:35.396 D/Robotium(25731): ^Tools$ not found. Have found: Tabs
01-05 00:59:35.396 D/Robotium(25731): ^Tools$ not found. Have found: Private
01-05 00:59:35.396 D/Robotium(25731): ^Tools$ not found. Have found: Synced
01-05 00:59:35.396 D/Robotium(25731): ^Tools$ not found. Have found: About Fennec
01-05 00:59:35.406 D/Robotium(25731): ^Tools$ not found. Have found: Apps
01-05 00:59:35.406 D/Robotium(25731): ^Tools$ not found. Have found: Downloads
01-05 00:59:35.406 D/Robotium(25731): ^Tools$ not found. Have found: Add-ons
01-05 00:59:35.406 D/Robotium(25731): ^Tools$ not found. Have found: Add-ons
01-05 00:59:35.406 D/Robotium(25731): ^Tools$ not found. Have found: 4
01-05 00:59:35.406 I/Robocop (25731): Exception caught during test!
01-05 00:59:35.406 I/Robocop (25731): junit.framework.AssertionFailedError: The text: ^Tools$ is not found!
01-05 00:59:35.406 I/Robocop (25731): at junit.framework.Assert.fail(Assert.java:47)
01-05 00:59:35.406 I/Robocop (25731): at junit.framework.Assert.assertTrue(Assert.java:20)
01-05 00:59:35.406 I/Robocop (25731): at com.jayway.android.robotium.solo.Clicker.clickOnText(Clicker.java:291)
01-05 00:59:35.406 I/Robocop (25731): at com.jayway.android.robotium.solo.Solo.clickOnText(Solo.java:823)
01-05 00:59:35.406 I/Robocop (25731): at org.mozilla.fennec_jmaher.tests.BaseTest.selectMenuItem(BaseTest.java:357)
01-05 00:59:35.406 I/Robocop (25731): at org.mozilla.fennec_jmaher.tests.testSystemPages.loadFromMenu(testSystemPages.java:78)
01-05 00:59:35.406 I/Robocop (25731): at org.mozilla.fennec_jmaher.tests.testSystemPages.testSystemPages(testSystemPages.java:43)
01-05 00:59:35.406 I/Robocop (25731): at java.lang.reflect.Method.invokeNative(Native Method)
01-05 00:59:35.406 I/Robocop (25731): at java.lang.reflect.Method.invoke(Method.java:521)
01-05 00:59:35.406 I/Robocop (25731): at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204)
01-05 00:59:35.406 I/Robocop (25731): at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194)
01-05 00:59:35.406 I/Robocop (25731): at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
01-05 00:59:35.406 I/Robocop (25731): at org.mozilla.fennec_jmaher.tests.BaseTest.runTest(BaseTest.java:130)
01-05 00:59:35.406 I/Robocop (25731): at junit.framework.TestCase.runBare(TestCase.java:127)
01-05 00:59:35.406 I/Robocop (25731): at junit.framework.TestResult$1.protect(TestResult.java:106)
01-05 00:59:35.406 I/Robocop (25731): at junit.framework.TestResult.runProtected(TestResult.java:124)
01-05 00:59:35.406 I/Robocop (25731): at junit.framework.TestResult.run(TestResult.java:109)
01-05 00:59:35.406 I/Robocop (25731): at junit.framework.TestCase.run(TestCase.java:118)
01-05 00:59:35.406 I/Robocop (25731): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169)
01-05 00:59:35.406 I/Robocop (25731): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154)
01-05 00:59:35.406 I/Robocop (25731): at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:520)
01-05 00:59:35.406 I/Robocop (25731): at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447)
01-05 00:59:35.406 I/Robocop (25731): 31 INFO TEST-UNEXPECTED-FAIL | testSystemPages | Exception caught - junit.framework.AssertionFailedError: The text: ^Tools$ is not found!
01-05 00:59:35.406 I/Robocop (25731): 32 INFO TEST-END | testSystemPages | finished in 250642ms
01-05 00:59:35.406 I/Robocop (25731): 33 INFO TEST-START | Shutdown
01-05 00:59:35.416 I/Robocop (25731): 34 INFO Passed: 29
01-05 00:59:35.416 I/Robocop (25731): 35 INFO Failed: 1
01-05 00:59:35.416 I/Robocop (25731): 36 INFO Todo: 0
01-05 00:59:35.416 I/Robocop (25731): 37 INFO SimpleTest FINISHED
Attachment #697855 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 11•12 years ago
|
||
Modified the selectMenuItem method in BaseTest.java.in to avoid this error. I've checked on different Android versions and it works for me every time now. Please verify again.
Attachment #697855 -
Attachment is obsolete: true
Attachment #702697 -
Flags: review?(jmaher)
Comment 12•12 years ago
|
||
22 INFO TEST-PASS | testSystemPages | Awesomebar URL stayed the same - about: should equal about:
23 INFO TEST-PASS | testSystemPages | Awesomebar URL typed properly - about:config should equal about:config
24 INFO TEST-PASS | testSystemPages | Awesomebar URL stayed the same - about: should equal about:
25 INFO TEST-PASS | testSystemPages | The correct number of tabs are opened - 2 should equal 2
26 INFO TEST-PASS | testSystemPages | Awesomebar URL stayed the same - about:apps should equal about:apps
27 INFO TEST-PASS | testSystemPages | The correct number of tabs are opened - 3 should equal 3
28 INFO TEST-PASS | testSystemPages | Awesomebar URL stayed the same - about:downloads should equal about:downloads
29 INFO TEST-PASS | testSystemPages | The correct number of tabs are opened - 4 should equal 4
30 INFO TEST-PASS | testSystemPages | Awesomebar URL stayed the same - about:addons should equal about:addons
Exception caught during test!
junit.framework.AssertionFailedError: The text: ^More$ is not found!
at junit.framework.Assert.fail(Assert.java:47)
at junit.framework.Assert.assertTrue(Assert.java:20)
at com.jayway.android.robotium.solo.Clicker.clickOnText(Clicker.java:291)
at com.jayway.android.robotium.solo.Solo.clickOnText(Solo.java:823)
at org.mozilla.fennec_jmaher.tests.BaseTest.selectMenuItem(BaseTest.java:354)
at org.mozilla.fennec_jmaher.tests.testSystemPages.loadFromMenu(testSystemPages.java:78)
at org.mozilla.fennec_jmaher.tests.testSystemPages.testSystemPages(testSystemPages.java:43)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:521)
at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204)
at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194)
at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
at org.mozilla.fennec_jmaher.tests.BaseTest.runTest(BaseTest.java:129)
at junit.framework.TestCase.runBare(TestCase.java:127)
at junit.framework.TestResult$1.protect(TestResult.java:106)
at junit.framework.TestResult.runProtected(TestResult.java:124)
at junit.framework.TestResult.run(TestResult.java:109)
at junit.framework.TestCase.run(TestCase.java:118)
at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169)
at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154)
at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:520)
at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447)
31 INFO TEST-UNEXPECTED-FAIL | testSystemPages | Exception caught - junit.framework.AssertionFailedError: The text: ^More$ is not found!
32 INFO TEST-END | testSystemPages | finished in 217491ms
33 INFO TEST-START | Shutdown
34 INFO Passed: 29
35 INFO Failed: 1
36 INFO Todo: 0
37 INFO SimpleTest FINISHED
Comment 13•12 years ago
|
||
seems like we switched from not finding tools to not finding more.
Assignee | ||
Comment 14•12 years ago
|
||
I've made changes in BaseTest.java. to avoid errors on tegras with Android 2.3, wich run in tablet mode.
Attachment #702697 -
Attachment is obsolete: true
Attachment #702697 -
Flags: review?(jmaher)
Attachment #703180 -
Flags: review?(jmaher)
Comment 15•12 years ago
|
||
Comment on attachment 703180 [details] [diff] [review]
SystemPages test V6
Review of attachment 703180 [details] [diff] [review]:
-----------------------------------------------------------------
this is failing on try server:
https://tbpl.mozilla.org/php/getParsedLog.php?id=19004095&tree=Try&full=1
on try server we have android 2.3/tablet mode. This passes for me locally though. Also this fails similarly on the panda boards (android 4.0/tablet).
Attachment #703180 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 16•12 years ago
|
||
I've made the necessary adjustments including the modifications in BaseTest.java. Please verify
Attachment #703180 -
Attachment is obsolete: true
Attachment #705817 -
Flags: review?(jmaher)
Comment 17•12 years ago
|
||
Comment on attachment 705817 [details] [diff] [review]
SettingsMenuItems test V7
Review of attachment 705817 [details] [diff] [review]:
-----------------------------------------------------------------
this is green, r- for the sleeps, feel free to argue your case if you really need them.
::: mobile/android/base/tests/testSystemPages.java.in
@@ +48,5 @@
> + for (String url:urls) {
> + loadAndPaint(url);
> + verifyTabCount(mExpectedTabCount);
> + verifyUrl(url);
> + }
nit: 4 space indentation, the for loop is too far indented.
@@ +49,5 @@
> + loadAndPaint(url);
> + verifyTabCount(mExpectedTabCount);
> + verifyUrl(url);
> + }
> + }
nit: } is indented too much.
@@ +62,5 @@
> +
> + // Set up listeners to catch the page load we're about to do
> + tabEventExpecter = mActions.expectGeckoEvent("Tab:Added");
> + contentEventExpecter = mActions.expectGeckoEvent("DOMContentLoaded");
> + mSolo.sleep(4000); // waiting for the page to load
Can we waitForText(item) or something like that?
@@ +74,5 @@
> + verifyUrl(expectedUrls[i]);
> + }
> + // Make sure the about: page was loaded without opening a new tab and verify the page Url
> + else {
> + mSolo.sleep(4000); // waiting for the page to load
can we do something else besides mSolo.sleep() here? maybe waitfortext(???) on something in the page we are loading?
@@ +78,5 @@
> + mSolo.sleep(4000); // waiting for the page to load
> + selectMenuItem(item);
> + mSolo.waitForText("About");
> + mSolo.clickOnText("About");
> + mSolo.sleep(4000); // waiting for the page to load
can we waitForText(About)? or something similar?
@@ +83,5 @@
> + expectedTabCount--; // Decreasing since we don't expect this in a new tab
> + verifyTabCount(expectedTabCount);
> + verifyUrl(expectedUrls[i]); // Since the page is already loaded this should be instantly
> + }
> + }
nit: block is indented too far, 4 spaces only.
Attachment #705817 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 18•12 years ago
|
||
I've made the necessary adjustments. Please verify
Attachment #705817 -
Attachment is obsolete: true
Attachment #708526 -
Flags: review?(jmaher)
![]() |
||
Comment 19•12 years ago
|
||
Comment on attachment 708526 [details] [diff] [review]
SystemPages test V8
Review of attachment 708526 [details] [diff] [review]:
-----------------------------------------------------------------
Stepping in for the review since :jmaher is away for a little while....
This is looking really good and seems quite reliable. I am tempted to r+ but would like to see the BACK explained and spacing corrected (check the indent level throughout please: 4 spaces for each indent level).
::: mobile/android/base/tests/BaseTest.java.in
@@ +347,4 @@
> //build the item name ready to be used
> String itemName = "^" + menuItemName + "$";
> mActions.sendSpecialKey(Actions.SpecialKey.MENU);
> + if (mSolo.waitForText(itemName)) {
I am a little uneasy about this because, in the fairly-common case that the item is hidden behind "More", we time-out on the first waitForText, adding 15 seconds or so to the test time. I cannot think of a more reliable option though...perhaps it is best this way.
@@ +352,5 @@
> + }
> + else {
> + if (mSolo.searchText("(^More$|^Tools$)")) {
> + mSolo.clickOnText("(^More$|^Tools$)");
> + }
nit: spacing/indent level is off here
::: mobile/android/base/tests/testSystemPages.java.in
@@ +36,5 @@
> + loadUrl("about:config");
> +
> + paintExpecter = mActions.expectPaint();// Set up listener to catch the page load
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> + PaintedSurface painted = waitForPaint(paintExpecter);
This BACK + waitForPaint works fine...but it seems a little unusual. Can you add a comment explaining what's happening here?
@@ +54,5 @@
> + }
> + }
> +
> + public void loadFromMenu(String menuItems [], String expectedUrls []) {
> + int expectedTabCount = mExpectedTabCount;
nit: indent/spacing
@@ +59,5 @@
> + for (String item:menuItems) {
> + paintExpecter = mActions.expectPaint();// Set up listener to catch the page load
> +
> + expectedTabCount++;
> + mActions.sendSpecialKey(Actions.SpecialKey.BACK);
Again, comment on BACK.
Attachment #708526 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 20•12 years ago
|
||
> ::: mobile/android/base/tests/BaseTest.java.in
> @@ +347,4 @@
> > //build the item name ready to be used
> > String itemName = "^" + menuItemName + "$";
> > mActions.sendSpecialKey(Actions.SpecialKey.MENU);
> > + if (mSolo.waitForText(itemName)) {
>
> I am a little uneasy about this because, in the fairly-common case that the
> item is hidden behind "More", we time-out on the first waitForText, adding
> 15 seconds or so to the test time. I cannot think of a more reliable option
> though...perhaps it is best this way.
Yes you are right but In some cases the time-out is more than welcome because it avoids accessing items from menu before this is properly loaded and populated. There where several problems, especially with the low end devices concerning this waiting period.
I added comments for the back actions also.
Thanks Geoff.
Attachment #708526 -
Attachment is obsolete: true
Attachment #709027 -
Flags: review?(gbrown)
![]() |
||
Comment 21•12 years ago
|
||
Comment on attachment 709027 [details] [diff] [review]
SystemPages test V9
Review of attachment 709027 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks Paul. Looks good.
Attachment #709027 -
Flags: review?(gbrown) → review+
Comment 22•12 years ago
|
||
Assignee: nobody → paul.feher
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Updated•5 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•