Bug 166271 – Throw Exception when test doesn't clean up HID Events properly

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Throw Exception when test doesn't clean up HID Events properly

[Megan Gardner]

Created attachment 297555 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 297555 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=297555&action=review

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:772
> +- (bool)checkHIDCallbacksClear

I would call this checkForOutstandingCallbacks Return type should be BOOL

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:776
> +    if ([_eventCallbacks count])
> +        return NO;
> +    return YES;

Or just return [_eventCallbacks count] > 0

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:57
> +void UIScriptController::checkForClean()

Call this checkForOutstandingCallbacks() too.

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:60
> +        [NSException raise:@"WebKitTestRunnerTestProblem" format:@"Unhandled HID Event exists after NotifyDone, please make sure you are not adding additional HID calls after calling NotifyDone"];

What happens in a test run the this exception fires? Would anyone notice it? Would it be better to RELEASE_ASSERT?

The message is also hard to understand for test writers. What's HID? It should say something more like: "The test completed before all synthesized events had been handled. Perhaps you're calling notifyDone() too early?".

[Megan Gardner]

Created attachment 297567 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 297567 [details]
Patch

Clearing flags on attachment: 297567

Committed r210048: <http://trac.webkit.org/changeset/210048>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.