Intermittent test_profile_management.py TestLog.test_in_app_restart_the_browser | AssertionError: JavascriptException not raised

RESOLVED FIXED in Firefox 38

Status

Testing
Marionette
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: KWierso, Assigned: chmanchester)

Tracking

({ateam-marionette-intermittent, intermittent-failure})

unspecified
mozilla39
x86_64
Windows 8.1
ateam-marionette-intermittent, intermittent-failure
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox37 unaffected, firefox38 fixed, firefox39 fixed, firefox-esr31 unaffected)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
14:45:52 INFO - TEST-START | test_profile_management.py TestLog.test_can_restart_the_browser
14:45:52 INFO - ERROR:root:[Errno 10054] An existing connection was forcibly closed by the remote host
14:46:16 INFO - TEST-PASS | test_profile_management.py TestLog.test_can_restart_the_browser | took 24419ms
14:46:16 INFO - TEST-START | test_profile_management.py TestLog.test_change_preset
14:46:35 INFO - TEST-PASS | test_profile_management.py TestLog.test_change_preset | took 18590ms
14:46:35 INFO - TEST-START | test_profile_management.py TestLog.test_clean_profile
14:46:53 INFO - TEST-PASS | test_profile_management.py TestLog.test_clean_profile | took 18320ms
14:46:53 INFO - TEST-START | test_profile_management.py TestLog.test_in_app_restart_the_browser
14:47:09 ERROR - TEST-UNEXPECTED-FAIL | test_profile_management.py TestLog.test_in_app_restart_the_browser | AssertionError: JavascriptException not raised
14:47:09 INFO - Traceback (most recent call last):
14:47:09 INFO - File "C:\slave\test\build\venv\lib\site-packages\marionette\marionette_test.py", line 296, in run
14:47:09 INFO - testMethod()
14:47:09 INFO - File "C:\slave\test\build\tests\marionette\tests\testing\marionette\client\marionette\tests\unit\test_profile_management.py", line 63, in test_in_app_restart_the_browser
14:47:09 INFO - "return SpecialPowers.getBoolPref('marionette.test.restart');")
14:47:09 INFO - TEST-INFO took 15447ms
14:47:09 INFO - TEST-START | test_profile_management.py TestLog.test_preferences_are_set
14:47:09 INFO - TEST-PASS | test_profile_management.py TestLog.test_preferences_are_set | took 48ms
14:47:09 INFO - TEST-START | test_set_window_size.py TestSetWindowSize.test_that_we_can_get_and_set_window_size
Comment hidden (Treeherder Robot)
(Assignee)

Comment 2

3 years ago
I gave this some retriggers. It's still sort of a bad test, maybe I should just kill that assertion.
Flags: needinfo?(cmanchester)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 5

3 years ago
2 out of five is quite bad, I think that assertion is just tickling a race, I'll get rid of it.
Flags: needinfo?(cmanchester)
(Assignee)

Comment 6

3 years ago
Created attachment 8576357 [details]
MozReview Request: bz://1142344/chmanchester

/r/5227 - Bug 1142344 - Remove a racy assertion added in marionette test for frequently failing on a pgo build.; r=jgriffin

Pull down this commit:

hg pull review -r 3e3f08e01cbd838771edacd24f20ac6471092c95
Attachment #8576357 - Flags: review?(jgriffin)
(Assignee)

Updated

3 years ago
Blocks: 1141679
Comment hidden (Treeherder Robot)
Keywords: ateam-marionette-intermittent
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment on attachment 8576357 [details]
MozReview Request: bz://1142344/chmanchester

https://reviewboard.mozilla.org/r/5225/#review4259

Do we undersrtand why this is racy?  I'm guessing restart() can sometimes do the wrong thing, but it's not clear.
Attachment #8576357 - Flags: review?(jgriffin) → review+
(Assignee)

Comment 11

3 years ago
https://reviewboard.mozilla.org/r/5225/#review4261

Only a theory, but restart kills the browser in a way that means user prefs aren't persisted before shutdown, which is what this test is trying to verify. From what I've seen it does this when the browser is in _some stage_ of persisting the prefs, so before bug 1141679 that meant the pref service knew about the pref but the value wasn't what was set right before shutdown, and after it meant that the pref service didn't know about the pref at all (most of the time).

The "blow away a user pref" behavior isn't really desired in the first place, so I don't think this assertion is adding much value.
(Assignee)

Comment 12

3 years ago
checkin-needed without try because this only deletes a flaky assertion in a test.
Keywords: checkin-needed
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)
https://hg.mozilla.org/integration/mozilla-inbound/rev/b068aca286b5
Assignee: nobody → cmanchester
Flags: in-testsuite+
Keywords: checkin-needed
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/mozilla-central/rev/b068aca286b5
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
status-firefox37: --- → unaffected
status-firefox38: --- → unaffected
status-firefox-esr31: --- → unaffected
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1143212
We want to uplift the patch on bug 1141679, so uplifting this patch would also be necessary. Setting ni? for David, who is most likely going to do it.
status-firefox38: unaffected → affected
Flags: needinfo?(dburns)
status-firefox38: affected → fixed
(Assignee)

Comment 26

3 years ago
Comment on attachment 8576357 [details]
MozReview Request: bz://1142344/chmanchester
Attachment #8576357 - Attachment is obsolete: true
Attachment #8619735 - Flags: review+
(Assignee)

Comment 27

3 years ago
Created attachment 8619735 [details]
MozReview Request: Bug 1142344 - Remove a racy assertion added in marionette test for frequently failing on a pgo build.; r=jgriffin
You need to log in before you can comment on or make changes to this bug.