1234689 - Intermittent TEST-UNEXPECTED-FAIL | apps/sms/test/marionette/inbox_test.js | Inbox View tests Long list of conversations User could navigate without waiting for the app to be fully loaded AssertionError: expected false to be true

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Michael Henretty [:mikehenrty][:mhenretty]]

Note, when I disable the retry logic in bug 1222215, I see this failing almost every time. And indeed if I run the test locally, I see this same error. I'm not sure why it passes on current master, but we will need to fix this to land bug 1222215.

TEST-UNEXPECTED-FAIL | apps/sms/test/marionette/inbox_test.js | Inbox View tests Long list of conversations User could navigate without waiting for the app to be fully loaded
AssertionError: expected false to be true
    at Function.assert.isTrue (node_modules/chai/lib/chai/interface/assert.js:317:31)
    at Context.<anonymous> (apps/sms/test/marionette/inbox_test.js:72:14)
    at Test.MarionetteTest.run (node_modules/marionette-js-runner/lib/ui.js:25:31)

Comment 1

[Oleg Zasypkin [:azasypkin]]

Looks like just a race and this test is kind of sensitive to timing issues - will try to increase number of threads rendered.

Comment 2

[Michael Henretty [:mikehenrty][:mhenretty]]

Thanks for picking this up Oleg. I meant to comment that I disabled this test last week: https://github.com/mozilla-b2g/gaia/commit/d4be6321eabdb28d824a0d28e5efef828c1afece

Comment 3

[Julien Wajsberg [:julienw]]

In the mean time, Oleg, maybe it's possible to just skip the failing test and reenable the full file ? (unless all tests are affected)

Comment 4

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] (PTO -> 2016) from comment #3)
> In the mean time, Oleg, maybe it's possible to just skip the failing test
> and reenable the full file ? (unless all tests are affected)

IIRC there is only one, but I have a WIP [1] that should make it more stable (depends on the PR in bug 1179604). So if it turns out that WIP requires more time to land than I expect - then yeah I think we can just skip this single test and let split view mode tests to run.

[1] https://github.com/azasypkin/gaia/commit/7aa2b76b0fbbbda38b4809c69ecb652080f30e26

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] azasypkin:intermittent-gij-tester > mozilla-b2g:master

Comment 6

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8703115 [details] [review]
[gaia] azasypkin:intermittent-gij-tester > mozilla-b2g:master

Hey Steve,

How does it look?

Here is Treeherder results for ~30 runs of Gij that includes only all Messages tests - inbox tests are always green, only one attachments test is failing sometimes (bug 1236418): https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=037f91a5acd8d1e67bda5eff12fe58c907de9068

Thanks!

Comment 7

[Steve Chung [:steveck]]

Comment on attachment 8703115 [details] [review]
[gaia] azasypkin:intermittent-gij-tester > mozilla-b2g:master

I'm just wondering about the necessity of the 2 different launch case, but it seems we might still need this if you want to prove that thread still kkp loading after you enter the new message view. How about we check with app ready flag before/after entering another view instead of checking fixed conversation number in the inbox view? Or you prefer to check the actual number in the list since it's more straightforward?

Comment 8

[Oleg Zasypkin [:azasypkin]]

(In reply to Steve Chung [:steveck] from comment #7)
> Comment on attachment 8703115 [details] [review]
> [gaia] azasypkin:intermittent-gij-tester > mozilla-b2g:master
> 
> I'm just wondering about the necessity of the 2 different launch case, but
> it seems we might still need this if you want to prove that thread still kkp
> loading after you enter the new message view.

I've replied at GitHub, but leave it here is as well:

Our app is marked as ready on "fully-loaded" and in one of the inbox tests we want to check that user can enter Composer or Conversation _before_ all conversation are loaded. We had a bug (bug 1217859) when we waited for the "load" event before allowing user to navigate further, but because of a big number of contact images user had to wait for fully loaded. Does it make sense?

> How about we check with app
> ready flag before/after entering another view instead of checking fixed
> conversation number in the inbox view? Or you prefer to check the actual
> number in the list since it's more straightforward?

Well I have mixed feelings, "app readiness" is a bit too technical to me and implies a variety of things, more than just fully loaded inbox, but don't have really strong opinion on that :) Tell me if you prefer something like "messagesApp.isFullyLoaded()" (under the hood it will use client scope with reduced search timeout to make sure it won't wait for the fully loaded flag to return :)) and I'll make it happen!

Comment 9

[Steve Chung [:steveck]]

I agree that "app readiness" might be too technical here, and we are not necessary to wait for app loaded completely. But having a "messagesApp.isFullyLoaded" method but won't wait for the fully loaded flag might be confusing. Could we just assert that thread number is still increasing after entering another view(query the number before/after new message view and compare)?

Comment 10

[Oleg Zasypkin [:azasypkin]]

(In reply to Steve Chung [:steveck] from comment #9)
> I agree that "app readiness" might be too technical here, and we are not
> necessary to wait for app loaded completely. But having a
> "messagesApp.isFullyLoaded" method but won't wait for the fully loaded flag
> might be confusing. Could we just assert that thread number is still
> increasing after entering another view(query the number before/after new
> message view and compare)?

Replied at GitHub with more details :)

Comment 11

[Steve Chung [:steveck]]

Comment on attachment 8703115 [details] [review]
[gaia] azasypkin:intermittent-gij-tester > mozilla-b2g:master

Considering that we might not have apparently better way for verification now, current assertion is good enough for me. Let's land this fix now and focus on other intermittent issues.

Comment 12

[Oleg Zasypkin [:azasypkin]]

(In reply to Steve Chung [:steveck] from comment #11)
> Comment on attachment 8703115 [details] [review]
> [gaia] azasypkin:intermittent-gij-tester > mozilla-b2g:master
> 
> Considering that we might not have apparently better way for verification
> now, current assertion is good enough for me. Let's land this fix now and
> focus on other intermittent issues.

Thanks for review! I wish we had better and easier way to fix this :/ But you're right let's return all our integration tests back first.

Master: https://github.com/mozilla-b2g/gaia/commit/4cdac959cf4838dc5ea14f75b16d983182f7a02a