Closed
Bug 1105792
Opened 10 years ago
Closed 10 years ago
testHomeBanner | Exception caught - junit.framework.AssertionFailedError: View with id: '2131558597' is not found!
Categories
(Firefox for Android Graveyard :: Testing, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 37
People
(Reporter: gbrown, Assigned: gbrown)
References
Details
Attachments
(2 files, 1 obsolete file)
Simplify testHomeBanner to account for the fact that banner view is removed when banner is dismissed
6.41 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
2.01 KB,
patch
|
Margaret
:
review+
|
Details | Diff | Splinter Review |
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/gbrown@mozilla.com-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
Assignee | ||
Comment 1•10 years ago
|
||
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)
Assignee | ||
Comment 2•10 years ago
|
||
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?
Attachment #8533839 -
Flags: feedback?(margaret.leibovic)
Comment 3•10 years ago
|
||
(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•10 years ago
|
||
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.
Attachment #8533839 -
Flags: feedback?(margaret.leibovic) → feedback+
Comment 5•10 years ago
|
||
(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•10 years ago
|
||
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•10 years ago
|
||
Testing locally, it looks like this runs to completion (passing all the checks along the way).
Attachment #8534076 -
Flags: review?(gbrown)
Assignee | ||
Comment 8•10 years ago
|
||
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.
Attachment #8534076 -
Flags: review?(gbrown) → review+
Assignee | ||
Comment 9•10 years ago
|
||
Updated as suggested.
Attachment #8533839 -
Attachment is obsolete: true
Attachment #8534352 -
Flags: review?(margaret.leibovic)
Comment 10•10 years ago
|
||
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.
Attachment #8534352 -
Flags: review?(margaret.leibovic) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/eeb02917e3c4
https://hg.mozilla.org/mozilla-central/rev/b0d0d1cd812a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Updated•4 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
•