Intermittent composer_test.js | Messages Composer Messages Composer Test Suite Message char counter and MMS label

RESOLVED FIXED

Status

Firefox OS
Gaia::SMS
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: kgrandon, Assigned: azasypkin)

Tracking

({intermittent-failure, leave-open})

unspecified
ARM
Gonk (Firefox OS)
intermittent-failure, leave-open

Firefox Tracking Flags

(b2g-v2.1S unaffected, b2g-v2.2 fixed)

Details

Attachments

(5 attachments)

(Reporter)

Description

3 years ago
Not much in the logs for this one:

13:40:39     INFO -  TEST-UNEXPECTED-FAIL | apps/sms/test/marionette/composer_test.js | Messages Composer Messages Composer Test Suite Message char counter and MMS label
13:40:39     INFO -  AssertionError: Element should not be displayed: expected true to be false

http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-inbound-linux64_gecko/1421268101/mozilla-inbound_ubuntu64_vm-b2gdt_test-gaia-js-integration-4-bm51-tests1-linux64-build123.txt.gz
(Reporter)

Comment 1

3 years ago
Created attachment 8549357 [details]
Possible screenshot at point of assertion error

This might be the screenshot at the point of failure. It seems we fail to clear the email recipient, and the MMS label is still there failing the assertion at the end of the test.
(Reporter)

Comment 2

3 years ago
I tried a number of solutions to get this test working, and unfortunately I was not successful. It appears that marionette is having a hard time clearing the input, but I'm not sure why. For now I'm going to disable the test while we investigate re-enabling it.

Julien - could you investigate or assign to someone? Thanks!

https://github.com/mozilla-b2g/gaia/commit/1d9a8206cb259ffe79c6f2169ce8e3f6eb77bd00
Flags: needinfo?(felash)
Keywords: leave-open
Maybe Oleg would have a clue.
Flags: needinfo?(felash) → needinfo?(azasypkin)
(Assignee)

Comment 4

3 years ago
Created attachment 8551730 [details] [review]
GitHub pull request URL

Here is my attempt to fix this. Though I can't say yet what is the root cause, but my only guess is that email recipient node isn't "tapable" yet for some reason. So I just wait for the particular node to become visible for sure.

Try results are here: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=a8c095c20fd2

From ~90 runs of Gij-4 (that one contains composer test) there are 4 failures:
* 2 x Bug 1105675 Intermittent improve_settings_test.js | improve b2g "before each" hook;
* 2 x Bug 1024573 Gaia jobs should wrap node git clones with a retry ("Automation Error: mozprocess timed out after 300 seconds running ['make', 'node_modules', 'NODE_MODULES_GIT_URL=https://git.mozilla.org/b2g/gaia-node-modules.git']");
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Flags: needinfo?(azasypkin)
Attachment #8551730 - Flags: feedback?(felash)
(Assignee)

Comment 5

3 years ago
Comment on attachment 8551730 [details] [review]
GitHub pull request URL

Removing feedback request from here for now as it seems that root cause for this intermittent is the same as for bug 1104961, so testing all disabled tests there.
Attachment #8551730 - Flags: feedback?(felash)
(Reporter)

Updated

3 years ago
No longer blocks: 1120508
Hey Oleg, you didn't reenable this test in bug 1104961, can you please check this is resolved and unblacklist this test if it's the case?
Flags: needinfo?(azasypkin)
(Assignee)

Comment 7

3 years ago
(In reply to Julien Wajsberg [:julienw] from comment #6)
> Hey Oleg, you didn't reenable this test in bug 1104961, can you please check
> this is resolved and unblacklist this test if it's the case?

Yeah, unfortunately I still saw other unrelated intermittent failures at that time, just pushed rebased PR to Treeherder to get current status. Going to fix\enable this test in the upcoming days.
Flags: needinfo?(azasypkin)
(Assignee)

Comment 8

3 years ago
Comment on attachment 8551730 [details] [review]
GitHub pull request URL

Hey Julien,

I don't see any related intermittent failures with this patch anymore (~100 runs for Gij-4) [1]. Looks like we can safely land it, but what do you think?


[1] https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=80bf71838a77
Attachment #8551730 - Flags: review?(felash)
Comment on attachment 8551730 [details] [review]
GitHub pull request URL

I couldn't really run it locally, I'll trust you on this one. r=me

I restarted a little more the tests on treeherder, please merge once they're all finished and green :)
Attachment #8551730 - Flags: review?(felash) → review+
All tests are finished, all are green (except for the one that's unrelated, bug 1024573).

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

Thanks!
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Created attachment 8569364 [details]
screenshot

Really, it's still happening ? :(

It's not the same error message though (timeout instead of Element should not be displayed).

here is the image at the moment of failure.

I think we should file a separate bug though.
Comment hidden (Treeherder Robot)
Grrr, disappointing :( Will take a look tomorrow again, it's weird that I don't see anything like this on gaia pull requests
(Assignee)

Updated

3 years ago
Flags: needinfo?(azasypkin)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Comment 22

3 years ago
Being that this still fails after a retry, it could possibly be some gecko issue we are hitting.

Reopening as we are still seeing this on master.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Quick update: still can't reproduce it on my local workstation, experimenting with low-resource VM on Vagrant & VirtualBox at the moment...
Comment hidden (Treeherder Robot)
As seen on [1], we can see that it's not an issue with a single build because the same build can both pass or fail.

[1] https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=23f0b767c77e&filter-searchStr=gaia
Oleg, maybe I'm doing something wrong, but I have an issue when I try to run the test file locally:

  NoSuchElement: (7) Unable to locate element: #threads-composer-link

Do you have an idea?
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 42

3 years ago
Created attachment 8572565 [details] [review]
[gaia] azasypkin:bug-1121766-composer-test-screen-timeout > mozilla-b2g:master
Quick update: I see test failures (though not the same, but very similar) on my very-low-resource VM (1 CPU, 512 MB of RAM) and as far as I can tell it happens only when screen is off by timeout, disabling screen timeout in test fixed the issue for my VM. 

This may explain the fact why only composer test is so troublesome - it's the slowest one (~22 sec to execute on my local machine), but let's wait for Treeherder results first.
Status: REOPENED → ASSIGNED
Flags: needinfo?(azasypkin)
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Comment hidden (Treeherder Robot)
Comment on attachment 8572565 [details] [review]
[gaia] azasypkin:bug-1121766-composer-test-screen-timeout > mozilla-b2g:master

Looks pretty good so far, 200 successful-only runs [1].

Julien, what do you think about it? You should be able to reproduce similar issue on you local machine if you set 'screen.timeout' test setting to ~20 or lower. At some point I was thinking about splitting this test, but I'd prefer to not to as it tests pretty valuable use case.

[1] https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=ac64fd652382
Attachment #8572565 - Flags: feedback?(felash)
Comment on attachment 8572565 [details] [review]
[gaia] azasypkin:bug-1121766-composer-test-screen-timeout > mozilla-b2g:master

On my slow computer:
100% failures without the patch
100% success with the patch

r=me
ship it !
Attachment #8572565 - Flags: feedback?(felash) → review+
and please uplift to v2.2 and v2.1 with "a=npotb".
(Reporter)

Comment 48

3 years ago
Nice fine. It seems that this may be an option that we want to set by default so we don't run into this again in the future. What do you guys think?
I actually think these 3 settings (lockscreen.enabled, ftu.manifestURL, screen.timeout) should be set at these values by default. Only some tests (FTU, LockScreen) would need to set them back to some other values.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Master: https://github.com/mozilla-b2g/gaia/commit/4bb827780fa2a90042158d0714aac839149b3532

(In reply to Julien Wajsberg [:julienw] from comment #47)
> and please uplift to v2.2 and v2.1 with "a=npotb".
Sure.

> I actually think these 3 settings (lockscreen.enabled, ftu.manifestURL,
> screen.timeout) should be set at these values by default. Only some tests
> (FTU, LockScreen) would need to set them back to some other values.

Yeah, I think the same, there's a ton of tests that set lockscreen.enabled and ftu.manifestURL.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED

Comment 53

3 years ago
Created attachment 8572837 [details] [review]
[gaia] azasypkin:v2.2-bug-1121766-composer-test-1 > mozilla-b2g:v2.2
Luckily we don't have this test in v2.1 (there is only phone_number_service_test.js) :)
(Reporter)

Updated

3 years ago
See Also: → bug 1139631
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Last one is a wrong star: the fail is in verticalhome. The first line happens only because this is the change that triggered the build.

NI Ed, I think the error detection both in the enhanced log viewer and the bug suggestion could be made better?
Flags: needinfo?(emorley)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(In reply to Julien Wajsberg [:julienw] (PTO March 7th -> 15th) from comment #57)
> Last one is a wrong star: the fail is in verticalhome. The first line
> happens only because this is the change that triggered the build.
> 
> NI Ed, I think the error detection both in the enhanced log viewer and the
> bug suggestion could be made better?

Thank you for the needinfo :-)

There are a few things here that contributed to this:
1) Various points of releng/gaia code dump commit messages into the logs. I'm not convinced they should do this (SHA is fine, just not the commit message).
2) We could try to adjust the Treeherder log parser regex, but it's quite hard to exclude false positives, without missing other cases. I'm not convinced this is the easiest out of this list to change.
3) People classifying failures on Treeherder still need to check before selecting a bug, though a false positive rate is still to be expected even with the best of intentions.
4) The commit message for that bug was:
   "Bug 1121766 - Intermittent failing test, TEST-UNEXPECTED-FAIL | apps/sms/test/marionette/composer_test.js | Messages Composer Messages Composer Test Suite Message char counter and MMS label. Disable screen timeout."
...when really it should have been:
   "Bug 1121766  - Disable screen timeout in composer_test.js to avoid intermittent failures"
...we should always use commit messages that describe what was changed, not what the problem was, and definitely not the summary of an intermittent failure bug. See:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Checkin_comment
5) Commit messages should also be reviewed as well as the content of the patch.

I'll file some releng/gaia bugs for #1 :-)
Flags: needinfo?(emorley)
(In reply to Ed Morley [:edmorley] from comment #60)
> I'll file some releng/gaia bugs for #1 :-)

The buildbot parts are hard to change, since it comes from dumping all the properties out, so we'd have to go through the filter them. The gaia part is easy - have a patch in bug 1139827.
Comment hidden (Treeherder Robot)
Looks like it's still not fixed :/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

3 years ago
Summary: Intermittent failing test, TEST-UNEXPECTED-FAIL | apps/sms/test/marionette/composer_test.js | Messages Composer Messages Composer Test Suite Message char counter and MMS label → Intermittent composer_test.js | Messages Composer Messages Composer Test Suite Message char counter and MMS label
Mmmm, but they use old gaia, if I'm not wrong:

comment 58: eff3321ab4e65da3f906688ebb55ddf1e93d9452
comment 59: eff3321ab4e65da3f906688ebb55ddf1e93d9452
comment 62: eff3321ab4e65da3f906688ebb55ddf1e93d9452

I opened raw log like [1] and CTRL+F for "checkout" to know gaia revision. Or I'm looking revision in a wrong place?

[1] http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-inbound-linux64_gecko/1425560447/mozilla-inbound_ubuntu64_vm-b2gdt_test-gaia-js-integration-4-bm114-tests1-linux64-build5.txt.gz
Flags: needinfo?(felash)
I haven't looked but yeah maybe it's too early for the new gaia to propagate to mozilla-inbound.
Flags: needinfo?(felash)
(Reporter)

Comment 66

3 years ago
I think we just did a merge this morning, so I think we can close it as we haven't see it on b2g-inbound. Thanks guys!
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
(In reply to Oleg Zasypkin [:azasypkin] from comment #64)
> I opened raw log like [1] and CTRL+F for "checkout" to know gaia revision.
> Or I'm looking revision in a wrong place?

Search for "git log" :-)
(Assignee)

Updated

3 years ago
Depends on: 1140304
Comment on attachment 8572837 [details] [review]
[gaia] azasypkin:v2.2-bug-1121766-composer-test-1 > mozilla-b2g:v2.2

Carrying over r+, as patch is exactly the same.
Attachment #8572837 - Flags: review+
Treeherder has a lot of failures for v2.2 branch, but nothing related to composer test.

v2.2: https://github.com/mozilla-b2g/gaia/commit/f6942d4edcbb36ed84b78cc16a8366ae6e033981
status-b2g-v2.1S: --- → unaffected
status-b2g-v2.2: --- → fixed
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
We should file a separate bug for these 2 failures as they're new.
I'll keep an eye on these tests and file appropriate issues in case they keep failing.
Comment hidden (Treeherder Robot)
You need to log in before you can comment on or make changes to this bug.