1105792 - testHomeBanner | Exception caught - junit.framework.AssertionFailedError: View with id: '2131558597' is not found!

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Geoff Brown [:gbrown]]

Robocop testHomeBanner consistently throws an exception. Because Robocop exception handling is missing in some tests (bug 1105522), the exception is not reported and the test ends suddenly, but otherwise appears to pass.

When exception handling is restored, the failure becomes evident.

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

http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/[e-mail]-22a4eb2e6cc9/try-android/try_panda_android_test-robocop-2-bm89-tests1-panda-build3338.txt.gz

16:49:10     INFO -  TEST-PASS | testHomeBanner | url is not null - about:firefox should not equal null
16:49:10     INFO -  TEST-PASS | testHomeBanner | url is not null - about:firefox should not equal null
16:49:10     INFO -  TEST-PASS | testHomeBanner | The toolbar is not in the editing state -
16:49:10     INFO -  TEST-PASS | testHomeBanner | Waiting for Toolbar to enter editing mode. -
16:49:10     INFO -  TEST-PASS | testHomeBanner | Waiting for UrlEditText to be input method target. -
16:49:10     INFO -  TEST-PASS | testHomeBanner | url is not null - about:firefox should not equal null
16:49:10     INFO -  TEST-PASS | testHomeBanner | The toolbar is in the editing state -
16:49:10     INFO -  TEST-PASS | testHomeBanner | The UrlEditText is the input method target -
16:49:10     INFO -  TEST-PASS | testHomeBanner | The toolbar is in the editing state -
16:49:10     INFO -  TEST-PASS | testHomeBanner | initiatingAction is not null - org.mozilla.gecko.tests.components.ToolbarComponent$2@4135b3f8 should not equal null
16:49:10     INFO -  ToolbarTitleTextChangeVerifier: stored title, "".
16:49:10     INFO -  TEST-PASS | testHomeBanner | Given message occurred for registered event: {"type":"DOMTitleChanged","title":"About Nightly","tabID":0} - DOMTitleChanged should equal DOMTitleChanged
16:49:10     INFO -  TEST-PASS | testHomeBanner | Given message occurred for registered event: {"bgColor":"rgb(206, 215, 222)","errorType":"","type":"DOMContentLoaded","metadata":null,"tabID":0} - DOMContentLoaded should equal DOMContentLoaded
16:49:10     INFO -  EventExpecter: no longer listening for DOMContentLoaded
16:49:10     INFO -  EventExpecter: no longer listening for DOMTitleChanged
16:49:10     INFO -  ToolbarTitleTextChangeVerifier: state changed to title, "about:firefox".
16:49:10     INFO -  ToolbarTitleTextChangeVerifier: was satisfied.
16:49:10     INFO -  TEST-PASS | testHomeBanner | Waiting for Toolbar to exit editing mode. -
16:49:10     INFO -  0 ERROR Exception caught during test! - junit.framework.AssertionFailedError: View with id: '2131558597' is not found!
16:49:10  WARNING -  TEST-UNEXPECTED-FAIL | testHomeBanner | Exception caught - junit.framework.AssertionFailedError: View with id: '2131558597' is not found!
16:49:10     INFO -  TEST-OK | testHomeBanner | took 44324ms
16:49:10     INFO -  TEST-START | Shutdown
16:49:10     INFO -  Passed: 51
16:49:10     INFO -  Failed: 1
16:49:10     INFO -  Todo: 0

Comment 1

[Geoff Brown [:gbrown]]

With the fix for bug 1093137, we can see the stack trace:

Exception caught during test! - junit.framework.AssertionFailedError: View with id: '2131361985' 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.Solo.getView(Solo.java:1990)
	at com.jayway.android.robotium.solo.Solo.getView(Solo.java:1970)
	at org.mozilla.gecko.tests.components.AboutHomeComponent.getHomeBannerView(AboutHomeComponent.java:71)
	at org.mozilla.gecko.tests.components.AboutHomeComponent.assertBannerNotVisible(AboutHomeComponent.java:98)
	at org.mozilla.gecko.tests.testHomeBanner.addBannerTest(testHomeBanner.java:58)
	at org.mozilla.gecko.tests.testHomeBanner.testHomeBanner(testHomeBanner.java:20)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
	at org.mozilla.gecko.tests.UITest.runTest(UITest.java:102)
	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:537)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1551)

Comment 2

[Geoff Brown [:gbrown]]

Attachment: allow view to be missing when asserting home banner not visible

Note that we are in assertBannerNotVisible() and throwing when the R.id.home_banner view is not found. This patch allows for the home banner view to not be found in assertBannerNotVisible()...that seems reasonable to me, but I'm not 100% sure about it. Should we only expect the home banner view to be found but not visible?

Anyway, once I apply this patch, that part of the test passes, but there is another failure further on:

TEST-PASS | testHomeBanner | Waiting for Toolbar to exit editing mode. - 
TEST-PASS | testHomeBanner | The HomePager is visible - 
1 ERROR Exception caught during test! - junit.framework.AssertionFailedError: View with id: '2131361985' 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.Solo.getView(Solo.java:1990)
	at com.jayway.android.robotium.solo.Solo.getView(Solo.java:1970)
	at org.mozilla.gecko.tests.components.AboutHomeComponent.getHomeBannerView(AboutHomeComponent.java:71)
	at org.mozilla.gecko.tests.components.AboutHomeComponent.assertBannerVisible(AboutHomeComponent.java:111)
	at org.mozilla.gecko.tests.testHomeBanner.hideOnToolbarFocusTest(testHomeBanner.java:99)
	at org.mozilla.gecko.tests.testHomeBanner.testHomeBanner(testHomeBanner.java:23)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)

This is a harder problem because now the home banner is not found when we expect it to be visible -- in assertBannerVisible(). hideOnToolbarFocusTest() is running, failing at http://hg.mozilla.org/mozilla-central/annotate/47f0671e2c65/mobile/android/base/tests/testHomeBanner.java#l100. At the end of addBannerTest(), http://hg.mozilla.org/mozilla-central/annotate/47f0671e2c65/mobile/android/base/tests/testHomeBanner.java#l58, we asserted that the banner was not visible; hideOnToolbarFocusTest() then loads about:home and expects the banner to be visible -- that doesn't seem right to me.

Is this part of the test just wrong?

Comment 3

[:Margaret Leibovic]

(In reply to Geoff Brown [:gbrown] from comment #2)
> Created attachment 8533839 [details] [diff] [review]
> allow view to be missing when asserting home banner not visible
> 
> Note that we are in assertBannerNotVisible() and throwing when the
> R.id.home_banner view is not found. This patch allows for the home banner
> view to not be found in assertBannerNotVisible()...that seems reasonable to
> me, but I'm not 100% sure about it. Should we only expect the home banner
> view to be found but not visible?
> 
> Anyway, once I apply this patch, that part of the test passes, but there is
> another failure further on:
> 
> TEST-PASS | testHomeBanner | Waiting for Toolbar to exit editing mode. - 
> TEST-PASS | testHomeBanner | The HomePager is visible - 
> 1 ERROR Exception caught during test! -
> junit.framework.AssertionFailedError: View with id: '2131361985' 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.Solo.getView(Solo.java:1990)
> 	at com.jayway.android.robotium.solo.Solo.getView(Solo.java:1970)
> 	at
> org.mozilla.gecko.tests.components.AboutHomeComponent.
> getHomeBannerView(AboutHomeComponent.java:71)
> 	at
> org.mozilla.gecko.tests.components.AboutHomeComponent.
> assertBannerVisible(AboutHomeComponent.java:111)
> 	at
> org.mozilla.gecko.tests.testHomeBanner.hideOnToolbarFocusTest(testHomeBanner.
> java:99)
> 	at
> org.mozilla.gecko.tests.testHomeBanner.testHomeBanner(testHomeBanner.java:23)
> 	at java.lang.reflect.Method.invokeNative(Native Method)
> 	at java.lang.reflect.Method.invoke(Method.java:511)
> 	at
> android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:
> 214)
> 
> This is a harder problem because now the home banner is not found when we
> expect it to be visible -- in assertBannerVisible().
> hideOnToolbarFocusTest() is running, failing at
> http://hg.mozilla.org/mozilla-central/annotate/47f0671e2c65/mobile/android/
> base/tests/testHomeBanner.java#l100. At the end of addBannerTest(),
> http://hg.mozilla.org/mozilla-central/annotate/47f0671e2c65/mobile/android/
> base/tests/testHomeBanner.java#l58, we asserted that the banner was not
> visible; hideOnToolbarFocusTest() then loads about:home and expects the
> banner to be visible -- that doesn't seem right to me.
> 
> Is this part of the test just wrong?

At the end of addBannerTest, we're asserting that the banner isn't visible after loading a new page, so this part of the test is correct.

However, in addBannerTest, we click on the banner, and I believe we have logic in place to make sure we don't show that banner again if the user has interacted with it.

I think we should update this test to make clicking the last thing that happens. I can write up a patch to show you what I'm thinking, and you can let me know if that works.

Comment 4

[:Margaret Leibovic]

Comment on attachment 8533839 [details] [diff] [review]
allow view to be missing when asserting home banner not visible

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

::: mobile/android/base/tests/components/AboutHomeComponent.java
@@ +102,5 @@
> +                        banner.getVisibility() != View.VISIBLE ||
> +                        banner.getTranslationY() == banner.getHeight());
> +        } else {
> +            fAssertTrue("The HomeBanner is not visible (view not found)", true);
> +        }

This sounds reasonable to me, but maybe we should encapsulate this waitForView check inside of getHomeBannerView. If we do that, we can return null if the view isn't found, and just update the existing fAssertTrue in here to pass if the banner is null.

Comment 5

[:Margaret Leibovic]

(In reply to :Margaret Leibovic from comment #3)
> (In reply to Geoff Brown [:gbrown] from comment #2)
> > Created attachment 8533839 [details] [diff] [review]
> > allow view to be missing when asserting home banner not visible
> > 
> > Note that we are in assertBannerNotVisible() and throwing when the
> > R.id.home_banner view is not found. This patch allows for the home banner
> > view to not be found in assertBannerNotVisible()...that seems reasonable to
> > me, but I'm not 100% sure about it. Should we only expect the home banner
> > view to be found but not visible?
> > 
> > Anyway, once I apply this patch, that part of the test passes, but there is
> > another failure further on:
> > 
> > TEST-PASS | testHomeBanner | Waiting for Toolbar to exit editing mode. - 
> > TEST-PASS | testHomeBanner | The HomePager is visible - 
> > 1 ERROR Exception caught during test! -
> > junit.framework.AssertionFailedError: View with id: '2131361985' 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.Solo.getView(Solo.java:1990)
> > 	at com.jayway.android.robotium.solo.Solo.getView(Solo.java:1970)
> > 	at
> > org.mozilla.gecko.tests.components.AboutHomeComponent.
> > getHomeBannerView(AboutHomeComponent.java:71)
> > 	at
> > org.mozilla.gecko.tests.components.AboutHomeComponent.
> > assertBannerVisible(AboutHomeComponent.java:111)
> > 	at
> > org.mozilla.gecko.tests.testHomeBanner.hideOnToolbarFocusTest(testHomeBanner.
> > java:99)
> > 	at
> > org.mozilla.gecko.tests.testHomeBanner.testHomeBanner(testHomeBanner.java:23)
> > 	at java.lang.reflect.Method.invokeNative(Native Method)
> > 	at java.lang.reflect.Method.invoke(Method.java:511)
> > 	at
> > android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:
> > 214)
> > 
> > This is a harder problem because now the home banner is not found when we
> > expect it to be visible -- in assertBannerVisible().
> > hideOnToolbarFocusTest() is running, failing at
> > http://hg.mozilla.org/mozilla-central/annotate/47f0671e2c65/mobile/android/
> > base/tests/testHomeBanner.java#l100. At the end of addBannerTest(),
> > http://hg.mozilla.org/mozilla-central/annotate/47f0671e2c65/mobile/android/
> > base/tests/testHomeBanner.java#l58, we asserted that the banner was not
> > visible; hideOnToolbarFocusTest() then loads about:home and expects the
> > banner to be visible -- that doesn't seem right to me.
> > 
> > Is this part of the test just wrong?
> 
> At the end of addBannerTest, we're asserting that the banner isn't visible
> after loading a new page, so this part of the test is correct.
> 
> However, in addBannerTest, we click on the banner, and I believe we have
> logic in place to make sure we don't show that banner again if the user has
> interacted with it.
> 
> I think we should update this test to make clicking the last thing that
> happens. I can write up a patch to show you what I'm thinking, and you can
> let me know if that works.

Actually, looking at this test more carefully, this must have been written before we refined some of the banner behavior, since the addBannerMessage method definitely won't make the banner appear again... I think we'll either need to figure out a way to re-work this test, or dumb it down to avoid the need to re-show the banner after it's been hidden. However, if we take the latter approach, we won't be able to test both clicking the banner and dismissing it :/

Comment 6

[:Margaret Leibovic]

Looking into the code, we totally remove the view from the hierarchy whenever it is clicked or dismissed:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#2250

So I think we should just dumb down the test. This is not an area of the code where we see regressions, so I'm not worried. I think we'll get bug reports if clicking on the banner stops working.

I'll attach a patch.

Comment 7

[:Margaret Leibovic]

Attachment: Simplify testHomeBanner to account for the fact that banner view is removed when banner is dismissed

Testing locally, it looks like this runs to completion (passing all the checks along the way).

Comment 8

[Geoff Brown [:gbrown]]

Comment on attachment 8534076 [details] [diff] [review]
Simplify testHomeBanner to account for the fact that banner view is removed when banner is dismissed

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

Thankyou!

I included your patch in https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d7d21c9e20ba. It looks like I just need to sort out bug 1107002 and bug 1085837 now.

Comment 9

[Geoff Brown [:gbrown]]

Attachment: allow view to be missing when asserting home banner not visible

Updated as suggested.

Comment 10

[:Margaret Leibovic]

Comment on attachment 8534352 [details] [diff] [review]
allow view to be missing when asserting home banner not visible

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

Great, thanks.

Comment 11

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dd49c68402d

https://hg.mozilla.org/integration/mozilla-inbound/rev/eeb02917e3c4
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0d0d1cd812a

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/eeb02917e3c4
https://hg.mozilla.org/mozilla-central/rev/b0d0d1cd812a