Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When an alert handler fails to run, log useful information #230

Merged
merged 5 commits into from
Jul 11, 2014
Merged

When an alert handler fails to run, log useful information #230

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
487fa9a
Catch the alert handler exception
Jul 8, 2014
53231a9
Fix out-of-bounds index exception for JS exception failures in sublim…
Jul 8, 2014
e38828f
Don't rethrow the exception, just splice out the alert handler
Jul 9, 2014
b3eaa67
Add a 2 second timeout to tapping the alert button
Jul 10, 2014
79e55af
Put the UIA timeout around the entire UIA Alert handler
Jul 10, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions Sources/Classes/UIAutomation/User Interface Elements/SLAlert.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,21 @@ + (void)loadUIAAlertHandling {
// enumerate registered handlers, from first to last
@"for (var handlerIndex = 0; handlerIndex < SLAlertHandler.alertHandlers.length; handlerIndex++) {\
var handler = SLAlertHandler.alertHandlers[handlerIndex];"
// if a handler matches the alert...
@"if (handler.handleAlert(alert) === true) {\
if (SLAlertHandler.loggingEnabled) UIALogger.logMessage('Alert was handled by a test.');"
// ...ensure that the alert's delegate will receive its callbacks
// before the next JS command (i.e. -didHandleAlert) evaluates...
@"UIATarget.localTarget().delay(%g);"
// ...then remove the handler and return true
@"SLAlertHandler.alertHandlers.splice(handlerIndex, 1);\
return true;\
@"try {"
// if a handler matches the alert...
@"if (handler.handleAlert(alert) === true) {\
if (SLAlertHandler.loggingEnabled) UIALogger.logMessage('Alert was handled by a test.');"
// ...ensure that the alert's delegate will receive its callbacks
// before the next JS command (i.e. -didHandleAlert) evaluates...
@"UIATarget.localTarget().delay(%g);"
// ...then remove the handler and return true
@"SLAlertHandler.alertHandlers.splice(handlerIndex, 1);\
return true;\
}\
} catch (e) {\
UIALogger.logError('Handler for alert \"' + alert.name() + '\" threw an exception!');\
UIATarget.localTarget().logElementTree();\
SLAlertHandler.alertHandlers.splice(handlerIndex, 1);\
}\
}"

Expand Down Expand Up @@ -393,7 +399,12 @@ - (SLAlertDismissHandler *)dismissWithButtonTitled:(NSString *)buttonTitle {
NSString *UIAAlertHandler = [NSString stringWithFormat:@"\
var buttonElement = (%@)('%@');\
if (buttonElement && buttonElement.isValid()) {\
buttonElement.tap();\
UIATarget.localTarget().pushTimeout(2);\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about the onAlert function pushing/popping the timeout? Then that would cover any handler, not just -dismissWithButtonTitled:.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also let's make the timeout 5 seconds--that's the default (used by UIAutomation absent Subliminal; and by all of Subliminal's APIs).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is that we're going to wait 5 seconds for each registered alert that doesn't match on the title. I think I can fix it by modifying the dismissWithButton logic though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wrong, thanks for correcting me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, thanks for double-checking!

try {\
buttonElement.tap();\
} finally {\
UIATarget.localTarget().popTimeout();\
}\
return true;\
} else {\
return false;\
Expand Down
11 changes: 8 additions & 3 deletions Supporting Files/CI/subliminal-instrument/subliminal-instrument/SISLLogParser.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,15 @@ + (void)parseEventType:(SISLLogEventType *)type subtype:(SISLLogEventSubtype *)s
} else {
subtypeValue = SISLLogEventSubtypeTestFailure;

// message format: "SLKeyboardTest.m:62: ..."
NSArray *messageComponents = [message componentsSeparatedByString:@":"];
infoValue[@"fileName"] = messageComponents[0];
infoValue[@"lineNumber"] = @([messageComponents[1] integerValue]);

// In some rare circumstances, Subliminal may log an error with no call site,
// for example when an error is raised within the JS code.
if (messageComponents.count > 2) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good catch. I guess some of instruments' exceptions begin with "Error:" too. Can you amend the comment at 158 to say as much?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think a more apt description would be: "In rare circumstances, Subliminal may log an error with no call site." (the error's not coming from instruments--it's our use of UIALogger.logError above).

// Subliminal standard message format: "SLKeyboardTest.m:62: ..."
infoValue[@"fileName"] = messageComponents[0];
infoValue[@"lineNumber"] = @([messageComponents[1] integerValue]);
}
}
} else {
typeValue = SISLLogEventTypeDefault;
Expand Down