Closed Bug 1142344 Opened 9 years ago Closed 9 years ago

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

Categories

(Testing :: Marionette Client and Harness, defect)

Product:
Testing
Component:
Marionette Client and Harness
Platform:
x86_64
Windows 8.1
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

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

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox37 --- unaffected
firefox38 --- fixed
firefox39 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: KWierso, Assigned: chmanchester)

References

Details

(Keywords: intermittent-failure, pi-marionette-intermittent)

Attachments

(1 file, 1 obsolete file)

MozReview Request: Bug 1142344 - Remove a racy assertion added in marionette test for frequently failing on a pgo build.; r=jgriffin
39 bytes, text/x-review-board-request
jgriffin
: review+
Details 
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
I gave this some retriggers. It's still sort of a bad test, maybe I should just kill that assertion.
Flags: needinfo?(cmanchester)
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)
/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)
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+
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.
checkin-needed without try because this only deletes a flaky assertion in a test.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/b068aca286b5
Assignee: nobody → cmanchester
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b068aca286b5
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
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)

        Attachment #8576357 -
        Attachment is obsolete: true

        Attachment #8619735 -
        Flags: review+

    
  
Product: Testing → Remote Protocol

    
    

    
  
Moving bug to Testing::Marionette Client and Harness component per bug 1815831.
Component: Marionette → Marionette Client and Harness
Product: Remote Protocol → Testing

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: