Summary: | Simplify WebKitTestRunner timeout tracking | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||
Component: | Tools / Tests | Assignee: | Alexey Proskuryakov <ap> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cgarcia, clopez, kbalazs, ossy | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2015-01-01 16:46:53 PST
Created attachment 243875 [details]
proposed patch
Created attachment 243882 [details]
proposed patch
With Efl and Gtk build fixes.
(In reply to comment #2) > Created attachment 243882 [details] > proposed patch > > With Efl and Gtk build fixes. ./../Tools/WebKitTestRunner/TestController.cpp:73:45: error: missing binary operator before token "(" #if defined (__has_feature) && __has_feature(address_sanitizer) ^ Unfortunately GCC doesn't support short circuit evaluation, so we need to use nested ifs for has_feature and has_include. It's strange how GTK built it. I thought GTK uses GCC 4.8 too. Created attachment 243895 [details]
proposed patch
OK, replaced with an ugly nested #if for now. I'm looking into adding a macro to WTF, but need to figure out how an existing ADDRESS_SANITIZER macro is used.
Comment on attachment 243895 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=243895&action=review > Tools/WebKitTestRunner/TestController.cpp:78 > +#else Missing #endif before this #else. Created attachment 243901 [details]
proposed patch
Comment on attachment 243901 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=243901&action=review I think it’s a little strange to compile in this 10 second vs. 5 second default timeout. > Tools/WebKitTestRunner/TestController.cpp:81 > +#if defined(__has_feature) > +#if __has_feature(address_sanitizer) > +const double TestController::shortTimeout = 10.0; > +#else > +const double TestController::shortTimeout = 5.0; > +#endif > +#else > +const double TestController::shortTimeout = 5.0; > +#endif I would have written: #if defined(__has_feature) && __has_feature(address_sanitizer) const double TestController::shortTimeout = 10; #else const double TestController::shortTimeout = 5; #endif But maybe the nested syntax is needed to compile on compilers that don't have __has_feature? > Tools/WebKitTestRunner/TestController.h:56 > + constexpr static double noTimeout = -1; Does this work on Windows? I had problems with constexpr on Windows. Comment on attachment 243901 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=243901&action=review >> Tools/WebKitTestRunner/TestController.cpp:81 >> +#endif > > I would have written: > > #if defined(__has_feature) && __has_feature(address_sanitizer) > const double TestController::shortTimeout = 10; > #else > const double TestController::shortTimeout = 5; > #endif > > But maybe the nested syntax is needed to compile on compilers that don't have __has_feature? Oops, I see you discussing this earlier in this bug! Committed <http://trac.webkit.org/r177870>. Removed the use of constexpr to be safe, as Windows EWS is not functional. |