If the test is not running(already done), we should not set waitUntilDone flag, because this could make subsequent tests timeout.
Created attachment 346727 [details] Patch
Comment on attachment 346727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346727&action=review > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 > + if (!injectedBundle.isTestRunning()) Could this be an assertion? Presumably, it is the test that is written badly in this case.
Comment on attachment 346727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346727&action=review >> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 >> + if (!injectedBundle.isTestRunning()) > > Could this be an assertion? Presumably, it is the test that is written badly in this case. Yes, I think that a release assert would be appropriate.
Created attachment 346750 [details] Patch
(In reply to Chris Dumez from comment #2) > Comment on attachment 346727 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=346727&action=review > > > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 > > + if (!injectedBundle.isTestRunning()) > > Could this be an assertion? Presumably, it is the test that is written badly > in this case. Done.
Comment on attachment 346750 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346750&action=review > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 > + ASSERT(!injectedBundle.isTestRunning()); This assertion looks wrong / resersed. Please run layout tests in debug locally to make sure your assertion does not hit as we do not have wk2 debug configuration on EWS.
(In reply to Chris Dumez from comment #6) > Comment on attachment 346750 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=346750&action=review > > > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 > > + ASSERT(!injectedBundle.isTestRunning()); > > This assertion looks wrong / resersed. Please run layout tests in debug > locally to make sure your assertion does not hit as we do not have wk2 debug > configuration on EWS. Oops..it is reversed. I am running the layout tests locally, will upload patch after verification. Thanks!
Comment on attachment 346750 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346750&action=review >>> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 >>> + ASSERT(!injectedBundle.isTestRunning()); >> >> This assertion looks wrong / resersed. Please run layout tests in debug locally to make sure your assertion does not hit as we do not have wk2 debug configuration on EWS. > > Oops..it is reversed. I am running the layout tests locally, will upload patch after verification. Thanks! But also, please make it a *release* assert. There is no reason why we would only want to catch this situation in debug builds.
Created attachment 346850 [details] Patch
Comment on attachment 346850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346850&action=review > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 > + ASSERT(injectedBundle.isTestRunning()); Alexey asked for a release assertion.
Created attachment 346854 [details] Patch
(In reply to Chris Dumez from comment #10) > Comment on attachment 346850 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=346850&action=review > > > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:189 > > + ASSERT(injectedBundle.isTestRunning()); > > Alexey asked for a release assertion. Forgot to add that after local layout tests done. Uploading again...
Comment on attachment 346854 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346854&action=review r=me if Mac-wk2 EWS is green. > Tools/ChangeLog:6 > + If test is not running, we should not set the waitUntilDone flag, or it may cause subsequent tests timeout. This should come *after* the Reviewed by line.
Comment on attachment 346854 [details] Patch Attachment 346854 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8811097 New failing tests: css3/filters/backdrop/add-remove-add-backdrop-filter.html
Created attachment 346856 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 347039 [details] Patch for landing
Comment on attachment 347039 [details] Patch for landing Clearing flags on attachment: 347039 Committed r234819: <https://trac.webkit.org/changeset/234819>
All reviewed patches have been landed. Closing bug.
<rdar://problem/43255316>