Bug 155367 - Python test webkitpy.common.system.executive_unittest.ExecutiveTest.serial_test_kill_process is flaky
Summary: Python test webkitpy.common.system.executive_unittest.ExecutiveTest.serial_te...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-11 09:45 PST by Ryan Haddad
Modified: 2016-04-12 20:57 PDT (History)
4 users (show)

See Also:

Attachments
proposed fix (1.19 KB, text/plain)
2016-04-12 13:48 PDT, Alexey Proskuryakov 		no flags Details
proposed fix (1.43 KB, patch)
2016-04-12 14:28 PDT, Alexey Proskuryakov 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2016-03-11 09:45:38 PST 
webkitpy.common.system.executive_unittest.ExecutiveTest.serial_test_kill_process is flaky

This webkitpy test has been flaky and failing intermittently.

https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/4200
https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/4168
https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/4156

https://build.webkit.org/builders/Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/3909
https://build.webkit.org/builders/Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/3893
https://build.webkit.org/builders/Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/3886

[1446/1451] webkitpy.common.system.executive_unittest.ExecutiveTest.serial_test_kill_process failed:
  Traceback (most recent call last):
    File "/Volumes/Data/slave/elcapitan-release-tests-wk2/build/Tools/Scripts/webkitpy/common/system/executive_unittest.py", line 184, in serial_test_kill_process
      self.assertEqual(process.wait(), expected_exit_code)
  AssertionError: 0 != -15
Comment 1 Alexey Proskuryakov 2016-04-12 13:39:21 PDT 
I think that this was introduced by this change, unclear if it was even intentional: <http://trac.webkit.org/changeset/89400/trunk/Tools/Scripts/webkitpy/common/system/executive.py>

Doing a non-blocking waitpid followed by another waitpid is super racy, and no wonder that the second one sometimes fails.

Can't say that I like overall design of this code either - it relies on an exception as part of normal operation.
Comment 2 Alexey Proskuryakov 2016-04-12 13:48:24 PDT 
Created attachment 276272 [details]
proposed fix

Tests pass on Mac without this. I didn't test on Windows.
Comment 3 WebKit Commit Bot 2016-04-12 13:50:54 PDT 
Attachment 276272 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Alexey Proskuryakov 2016-04-12 14:28:47 PDT 
Created attachment 276276 [details]
proposed fix

With a proper ChangeLog.
Comment 5 WebKit Commit Bot 2016-04-12 20:57:29 PDT 
Comment on attachment 276276 [details]
proposed fix

Clearing flags on attachment: 276276

Committed r199403: <http://trac.webkit.org/changeset/199403>
Comment 6 WebKit Commit Bot 2016-04-12 20:57:33 PDT 
All reviewed patches have been landed.  Closing bug.