stop spamming 'stopping test driver timed out, killing it' when running rwt
Created attachment 110694 [details] Patch
Comment on attachment 110694 [details] Patch Oh, I have a better idea.
Created attachment 110695 [details] Patch
Comment on attachment 110695 [details] Patch So the intent is to give wait half as long as the default timeout, right? options.time_out_ms is always non-zero (see run_webkit_tests.py:139). You could just as easily do: kill_timeout_seconds = self._port.get_option('time_out_ms') / 2000.
(In reply to comment #4) > (From update of attachment 110695 [details]) > So the intent is to give wait half as long as the default timeout, right? I was trying to wait 3 sec by default, regardless of what the default timeout is.
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 110695 [details] [details]) > > So the intent is to give wait half as long as the default timeout, right? > > I was trying to wait 3 sec by default, regardless of what the default timeout is. I see. Well, the 'else' branch won't ever execute, because of the previously cited code, so I don't think you can easily get that behavior. Per the discussion in bug 68026, it seems to me like half of the timeout value is a reasonable heuristic to use, though.
Created attachment 110708 [details] Patch for landing
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (From update of attachment 110695 [details] [details] [details]) > > > So the intent is to give wait half as long as the default timeout, right? > > > > I was trying to wait 3 sec by default, regardless of what the default timeout is. > > I see. Well, the 'else' branch won't ever execute, because of the previously cited code, so I don't think you can easily get that behavior. Per the discussion in bug 68026, it seems to me like half of the timeout value is a reasonable heuristic to use, though. I've removed the else branch, but don't understand what you mean by not being able to easily get the behavior of defaulting to 3 sec. Any value for Manager.DEFAULT_TEST_TIMEOUT_MS should give us a 3 sec timeout.
(In reply to comment #8) > (In reply to comment #6) > > (In reply to comment #5) > > > (In reply to comment #4) > > > > (From update of attachment 110695 [details] [details] [details] [details]) > > > > So the intent is to give wait half as long as the default timeout, right? > > > > > > I was trying to wait 3 sec by default, regardless of what the default timeout is. > > > > I see. Well, the 'else' branch won't ever execute, because of the previously cited code, so I don't think you can easily get that behavior. Per the discussion in bug 68026, it seems to me like half of the timeout value is a reasonable heuristic to use, though. > > I've removed the else branch, but don't understand what you mean by not being able to easily get the behavior of defaulting to 3 sec. Any value for Manager.DEFAULT_TEST_TIMEOUT_MS should give us a 3 sec timeout. Oh, sorry, I think I was parsing your sentence differently. I was interpreting you to be saying that you wanted the kill value to default to 3 seconds regardless of whether someone had passed in a value via --time-out-ms, and I was saying that there was no way for you to tell if someone had used the command line switch or not. Of course, you couldn't have wanted that behavior, since the whole point of the prior change was to scale the kill value to the flag, and there isn't a way to specify the value directly, so fixing at that 3 seconds "by default" wouldn't have made sense. My mistake. And you are correct that kill_timeout will be set to 3 regardless of the value of Manager.DEFAULT_TEST_TIMEOUT_MS.
Comment on attachment 110708 [details] Patch for landing Clearing flags on attachment: 110708 Committed r97293: <http://trac.webkit.org/changeset/97293>
All reviewed patches have been landed. Closing bug.