Closed Bug 1085837 Opened 9 years ago Closed 9 years ago

Intermittent testAppMenuPathways | application crashed [None]

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
x86
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox35 fixed, firefox36 fixed, firefox37 fixed, firefox-esr31 unaffected)

Status:
RESOLVED FIXED
Milestone:
Firefox 37
Tracking Flags:
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
firefox37 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: cbook, Assigned: gbrown)

References

(
URL
)

Details

(Keywords: crash, intermittent-failure)

Attachments

(2 files, 1 obsolete file)

Open legacy "More" menu before asserting menu item is disabled
5.34 KB, patch
liuche
: review+
Details | Diff | Splinter Review
allow for different menu classes on android 2.3
2.29 KB, patch
mcomella
: review+
Details | Diff | Splinter Review 
Android 2.3 Emulator mozilla-inbound opt test robocop-4

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3158205&repo=mozilla-inbound

23:09:30 WARNING - PROCESS-CRASH | testAppMenuPathways | application crashed [None]
The logcat in Comment 0 shows:

23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): java.lang.NullPointerException
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at org.mozilla.gecko.tests.components.AppMenuComponent.assertMenuItemIsDisabledAndVisible(AppMenuComponent.java:128)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at org.mozilla.gecko.tests.testAppMenuPathways._testSaveAsPDFPathway(testAppMenuPathways.java:26)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at org.mozilla.gecko.tests.testAppMenuPathways.testAppMenuPathways(testAppMenuPathways.java:21)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at java.lang.reflect.Method.invokeNative(Native Method)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at java.lang.reflect.Method.invoke(Method.java:507)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at junit.framework.TestCase.runBare(TestCase.java:127)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at junit.framework.TestResult$1.protect(TestResult.java:106)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at junit.framework.TestResult.runProtected(TestResult.java:124)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at junit.framework.TestResult.run(TestResult.java:109)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at junit.framework.TestCase.run(TestCase.java:118)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:529)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): 	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1448)
23:09:30     INFO -  10-20 22:50:58.527 I/TestRunner( 1345): ----- end exception -----
Another issue here is that the main test log does not report any failures, and appears as though the test ran completed:

02:59:25     INFO -  TEST-START | testAppMenuPathways
02:59:25     INFO -  TEST-PASS | testAppMenuPathways | Robocop tests need the test device screen to be powered on. - 
02:59:25     INFO -  TEST-PASS | testAppMenuPathways | Given message occurred for registered event: {"type":"Gecko:Ready"} - Gecko:Ready should equal Gecko:Ready
02:59:25     INFO -  EventExpecter: no longer listening for Gecko:Ready
02:59:25     INFO -  TEST-PASS | testAppMenuPathways | Menu is not open - 
02:59:25     INFO -  TEST-PASS | testAppMenuPathways | Waiting for menu to open. - 
02:59:25     INFO -  TEST-OK | testAppMenuPathways | took 45017ms
02:59:25     INFO -  TEST-START | Shutdown
02:59:25     INFO -  Passed: 4
02:59:25     INFO -  Failed: 0
02:59:25     INFO -  Todo: 0
02:59:25     INFO -  SimpleTest FINISHED

In fact, the exception occurs early in the test. There's should be much more:

05:26:03     INFO -  TEST-START | testAppMenuPathways
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Robocop tests need the test device screen to be powered on. -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Given message occurred for registered event: {"type":"Gecko:Ready"} - Gecko:Ready should equal Gecko:Ready
05:26:03     INFO -  EventExpecter: no longer listening for Gecko:Ready
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Menu is not open -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Waiting for menu to open. -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The parent 'page' menu item is not enabled -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The parent 'page' menu item is visible - 0 should equal 0
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Menu is not open -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Waiting for menu to open. -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The parent 'page' menu item is enabled -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The parent 'page' menu item is visible - 0 should equal 0
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Menu is open -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The menu item Page is enabled -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The menu item Page is visible - 0 should equal 0
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The page menu item is not null - org.mozilla.gecko.menu.MenuItemDefault@413fba20 should not equal null
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The page menu item is not enabled -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The page menu item is visible - 0 should equal 0
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | url is not null - /robocop/robocop_blank_01.html should not equal null
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | url is not null - /robocop/robocop_blank_01.html should not equal null
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The toolbar is not in the editing state -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Waiting for Toolbar to enter editing mode. -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Waiting for UrlEditText to be input method target. -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | url is not null - http://mochi.test:8888/tests/robocop/robocop_blank_01.html should not equal null
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The toolbar is in the editing state -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The UrlEditText is the input method target -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The toolbar is in the editing state -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | initiatingAction is not null - org.mozilla.gecko.tests.components.ToolbarComponent$2@41406648 should not equal null
05:26:03     INFO -  ToolbarTitleTextChangeVerifier: stored title, "http://10.26.128.22:30154/tests/big-buck-bunny_trailer.webm".
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Given message occurred for registered event: {"type":"DOMTitleChanged","title":"Browser Blank Page 01","tabID":0} - DOMTitleChanged should equal DOMTitleChanged
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Given message occurred for registered event: {"bgColor":"transparent","errorType":"","type":"DOMContentLoaded","metadata":null,"tabID":0} - DOMContentLoaded should equal DOMContentLoaded
05:26:03     INFO -  EventExpecter: no longer listening for DOMContentLoaded
05:26:03     INFO -  EventExpecter: no longer listening for DOMTitleChanged
05:26:03     INFO -  ToolbarTitleTextChangeVerifier: state changed to title, "Browser Blank Page 01".
05:26:03     INFO -  ToolbarTitleTextChangeVerifier: was satisfied.
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Waiting for Toolbar to exit editing mode. -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The title argument is not null - Browser Blank Page 01 should not equal null
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The url argument is not null - /robocop/robocop_blank_01.html should not equal null
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The toolbar is not in the editing state -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The Toolbar title is Browser Blank Page 01 - Browser Blank Page 01 should equal Browser Blank Page 01
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Menu is not open -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Waiting for menu to open. -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Menu is open -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The menu item Page is enabled -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The menu item Page is visible - 0 should equal 0
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | Menu is open -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The menu item Save as PDF is enabled -
05:26:03     INFO -  TEST-PASS | testAppMenuPathways | The menu item Save as PDF is visible - 0 should equal 0
05:26:03     INFO -  TEST-OK | testAppMenuPathways | took 47643ms
05:26:03     INFO -  TEST-START | Shutdown
05:26:03     INFO -  Passed: 41
05:26:03     INFO -  Failed: 0
05:26:03     INFO -  Todo: 0
05:26:03     INFO -  SimpleTest FINISHED
Thanks for the analysis, Geoff. I have no idea why this would cause the "application crashed" error, but regarding the NPE, the problem lines are [1]:

 final View pageMenuItemView = findAppMenuItemView(pageMenuItem.getString(mSolo));
 fAssertFalse("The page menu item is not enabled", pageMenuItemView.isEnabled());

This branch is intended to run only when we are on legacy devices, so this is in-line with most of the failures (and presumably all of the relevant ones) being on 2.3.

According to the javadoc, findAppMenuItemView will return null when the Android legacy menu is in use [2], but I imagine what is happening is the "More" menu is loaded into memory and we manage to find the "Save as PDF" menu item. However, sometimes the "More" menu will be evicted from memory, so we should probably open the menu and click "More" before running this method, rather than just opening the menu as we do now.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/components/AppMenuComponent.java?rev=4b558c9c4a24#126
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/components/AppMenuComponent.java?rev=4b558c9c4a24#143
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Sloppy qref. Also, forgot to mention that I also made the searchText calls more correct by ensuring they only search for visible views.
Attachment #8515376 - Flags: review?(liuche)
Comment on attachment 8515376 [details] [diff] [review]
Open legacy "More" menu before asserting menu item is disabled

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

A few nit comments. Sorry this took so long! The rest of the logic in this file for pressing menu items for 2.3 or post-2.3 devices is really ugly :(

::: mobile/android/base/tests/components/AppMenuComponent.java
@@ +122,5 @@
>                  fAssertFalse("The parent 'page' menu item is not enabled", parentMenuItemView.isEnabled());
>                  fAssertEquals("The parent 'page' menu item is visible", View.VISIBLE, parentMenuItemView.getVisibility());
>              }
>          } else {
> +            // Legacy devices don't have the parent menu, "Page", so check directly for the menu

"Legacy devices" - can you update the comment to specify an Android version cutoff here?

@@ +148,5 @@
>  
>      /**
>       * Try to find a MenuItemActionBar/MenuItemDefault with the given text set as contentDescription / text.
>       *
> +     * When using legacy menus, make sure the menu has been opened to the appropriate level

Can you make this comment more direct? It definitely sounds like a too-polite comment that would get glossed over.

"For legacy menus, the target menu view *MUST* have been created [or _must be open_, if you figure out the TODO] - ensure the menu has been opened to the appropriate level (base menu or "More" menu)."

(optionally you can also refer to usage in assurtMenuItemIsDisabledAndVisible)

or something to that effect.

legacy = 2.3, or pre-honeycomb - I think legacy made more sense in the past, but not that there are tons of android versions, let's just be explicit.

@@ -139,5 @@
>  
>      /**
>       * Try to find a MenuItemActionBar/MenuItemDefault with the given text set as contentDescription / text.
>       *
> -     * Will return null when the Android legacy menu is in use.

...so this comment has just been completely wrong? *sigh*

@@ +247,5 @@
> +    private void openLegacyMoreMenu() {
> +        fAssertTrue("The base menu is already open", isMenuOpen());
> +
> +        // Since there may be more views with "More" on the screen,
> +        // this is not robust. However, there may not be a better way.

This is the same thing that we do in BaseTest.selectMenuItem. Doesn't make this less hacky...but this is at least an improvement on the existing code.

@@ +278,5 @@
>      }
>  
> +    private boolean isLegacyMoreMenuOpen() {
> +        // Check if the first menu option is visible.
> +        return mSolo.searchText(mSolo.getString(R.string.share), true);

There's an edge case here where if the legacy "More" menu is scrolled, subsequent reopens of the menu will retain the scroll position, so Share may not be visible - you could try mSolo.scrollListToTop.
Attachment #8515376 - Flags: review?(liuche) → review+
See Also: → 1105522
Blocks: 1105522
See Also: 1105522 → 
With the exception handling fix in bug 1105522 applied, this test fails consistently on Android 2.3. I tried applying the patch here, but failures continue:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=91a101eb9384

http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/gbrown@mozilla.com-91a101eb9384/try-android/try_ubuntu64_vm_mobile_test-robocop-4-bm51-tests1-linux64-build310.txt.gz

23:48:37     INFO -  TEST-PASS | testAppMenuPathways | The base menu is already open - 
23:48:37     INFO -  TEST-PASS | testAppMenuPathways | Waiting for legacy "More" menu to open. - 
23:48:37     INFO -  0 ERROR Exception caught during test! - java.lang.NullPointerException
23:48:37     INFO -  	at org.mozilla.gecko.tests.components.AppMenuComponent.assertMenuItemIsDisabledAndVisible(AppMenuComponent.java:134)
23:48:37     INFO -  	at org.mozilla.gecko.tests.testAppMenuPathways._testSaveAsPDFPathway(testAppMenuPathways.java:26)
23:48:37     INFO -  	at org.mozilla.gecko.tests.testAppMenuPathways.testAppMenuPathways(testAppMenuPathways.java:21)
23:48:37     INFO -  	at java.lang.reflect.Method.invokeNative(Native Method)
23:48:37     INFO -  	at java.lang.reflect.Method.invoke(Method.java:507)
23:48:37     INFO -  	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204)
23:48:37     INFO -  	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194)
23:48:37     INFO -  	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
23:48:37     INFO -  	at org.mozilla.gecko.tests.UITest.runTest(UITest.java:102)
23:48:37     INFO -  	at junit.framework.TestCase.runBare(TestCase.java:127)
23:48:37     INFO -  	at junit.framework.TestResult$1.protect(TestResult.java:106)
23:48:37     INFO -  	at junit.framework.TestResult.runProtected(TestResult.java:124)
23:48:37     INFO -  	at junit.framework.TestResult.run(TestResult.java:109)
23:48:37     INFO -  	at junit.framework.TestCase.run(TestCase.java:118)
23:48:37     INFO -  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169)
23:48:37     INFO -  	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154)
23:48:37     INFO -  	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:529)
23:48:37     INFO -  	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1448)
On Android 2.3, a visually disabled item is a com.android.internal.view.menu.ListMenuItemView which is not enabled and which has a child android.widget.RelativeLayout which is enabled and has a child TextView which is also enabled and has the appropriate text.
This extends findAppMenuItemView() so that if the existing searches fail, we search for a matching text view and return its parent's parent (which should be the list item view, with an appropriate enabled state).

With this patch I can finally get a green try run with my UITest exception handling:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=25f78f824078
Attachment #8537627 - Flags: review?(michael.l.comella)
Comment on attachment 8537627 [details] [diff] [review]
allow for different menu classes on android 2.3

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

Nice thorough analysis! Thanks for working on these patches, Geoff! Sory for the review delays.

::: mobile/android/base/tests/components/AppMenuComponent.java
@@ +174,5 @@
>                  return menuItem;
>              }
>          }
>  
> +        // On Android 2.3, menu items may be instances of

Update the comment at the top of this method that says it returns null on Android 2.3.

@@ +182,5 @@
> +        final List<TextView> textViewList = RobotiumUtils.filterViews(TextView.class, views);
> +        for (TextView textView : textViewList) {
> +            if (textView.getText().equals(text)) {
> +                View parent = (View)textView.getParent();
> +                View grandparent = (View)parent.getParent();

nit: (View) textView...

nit: parent -> relativeLayoutContainer, or similar, to match comment.
nit: grandparent -> listMenuItemView, or similar
Attachment #8537627 - Flags: review?(michael.l.comella) → review+
By the way, if this is without my patch in comment 13, please obsolete it.
(In reply to Michael Comella (:mcomella) from comment #33)
> By the way, if this is without my patch in comment 13, please obsolete it.

Actually, we need both your patch and mine to produce a functional testAppMenuPathways on 2.3. Can I push your patch with mine?
Flags: needinfo?(michael.l.comella)
(In reply to Geoff Brown [:gbrown] from comment #34)
> Actually, we need both your patch and mine to produce a functional
> testAppMenuPathways on 2.3. Can I push your patch with mine?

Go for it - just address the following comment from:

(In reply to Chenxia Liu [:liuche] from comment #15)
> ::: mobile/android/base/tests/components/AppMenuComponent.java
> @@ +122,5 @@
> >                  fAssertFalse("The parent 'page' menu item is not enabled", parentMenuItemView.isEnabled());
> >                  fAssertEquals("The parent 'page' menu item is visible", View.VISIBLE, parentMenuItemView.getVisibility());
> >              }
> >          } else {
> > +            // Legacy devices don't have the parent menu, "Page", so check directly for the menu
> 
> "Legacy devices" - can you update the comment to specify an Android version
> cutoff here?

i.e. Versions <= 2.3
Flags: needinfo?(michael.l.comella)
(In reply to Chenxia Liu [:liuche] from comment #15)
> > +    private boolean isLegacyMoreMenuOpen() {
> > +        // Check if the first menu option is visible.
> > +        return mSolo.searchText(mSolo.getString(R.string.share), true);
> 
> There's an edge case here where if the legacy "More" menu is scrolled,
> subsequent reopens of the menu will retain the scroll position, so Share may
> not be visible - you could try mSolo.scrollListToTop.

Filed bug 1113427 to investigate this.
https://hg.mozilla.org/mozilla-central/rev/73b2de63fc8d
https://hg.mozilla.org/mozilla-central/rev/aa47abc62817
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Depends on: 1114655
Depends on: 1114658
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.