From 36e9b61157ca5afc1a51abb4e757716e437233e9 Mon Sep 17 00:00:00 2001 From: Jeff Wear Date: Mon, 26 Aug 2013 13:00:04 -0700 Subject: [PATCH 1/4] Refactor `-[SLElementMatchingTest testMatchingTableViewHeaderChildElements]`' view. To use a custom view. --- .../SLElementMatchingTestViewController.m | 79 +++++++++++++------ 1 file changed, 53 insertions(+), 26 deletions(-) diff --git a/Integration Tests/Tests/SLElementMatchingTestViewController.m b/Integration Tests/Tests/SLElementMatchingTestViewController.m index f3a7c86..1d0bc55 100644 --- a/Integration Tests/Tests/SLElementMatchingTestViewController.m +++ b/Integration Tests/Tests/SLElementMatchingTestViewController.m @@ -29,6 +29,8 @@ @interface SLElementMatchingTestViewController : SLTestCaseViewController @end +#pragma mark - SLElementMatchingTestCell + @interface SLElementMatchingTestCell : UITableViewCell - (void)configureAccessibility; @@ -58,7 +60,7 @@ - (id)initWithStyle:(UITableViewCellStyle)style reuseIdentifier:(NSString *)reus _weatherTemp.textAlignment = NSTextAlignmentRight; [self.contentView addSubview:_weatherTemp]; } else { - NSAssert(NO, @"%@ reuse identifier was not of expected format: '%@_<%@ test case>'.", + NSAssert(NO, @"%@ reuse identifier was not of expected format ('%@_<%@ test case>') or was unexpected.", NSStringFromClass([self class]), NSStringFromClass([self class]), NSStringFromClass([SLElementMatchingTestViewController class])); } } @@ -105,6 +107,55 @@ - (void)layoutSubviews { @end +#pragma mark - SLElementMatchingTestHeader + +@interface SLElementMatchingTestHeader : UIView + +- (instancetype)initWithTestCaseWithSelector:(SEL)testCase; + +@end + +@implementation SLElementMatchingTestHeader { + UIView *_leftView, *_rightView; +} + +- (instancetype)initWithTestCaseWithSelector:(SEL)testCase { + self = [super initWithFrame:CGRectZero]; + if (self) { + if (testCase == @selector(testMatchingTableViewHeaderChildElements)) { + UILabel *leftLabel = [[UILabel alloc] initWithFrame:CGRectZero]; + leftLabel.textAlignment = NSTextAlignmentLeft; + leftLabel.text = @"left"; + _leftView = leftLabel; + + UILabel *rightLabel = [[UILabel alloc] initWithFrame:CGRectZero]; + rightLabel.textAlignment = NSTextAlignmentRight; + rightLabel.text = @"right"; + _rightView = rightLabel; + } else { + NSAssert(NO, @"Unexpected test case: %@", NSStringFromSelector(testCase)); + } + [self addSubview:_leftView]; + [self addSubview:_rightView]; + } + return self; +} + +- (void)layoutSubviews { + [super layoutSubviews]; + + CGRect contentRect = CGRectInset(self.bounds, 20.0f, 0.0f); + CGRect leftViewFrame, rightViewFrame; + CGRectDivide(contentRect, &leftViewFrame, &rightViewFrame, CGRectGetWidth(contentRect) / 2.0f, CGRectMinXEdge); + _leftView.frame = leftViewFrame; + _rightView.frame = rightViewFrame; +} + +@end + + +#pragma mark - SLElementMatchingTestViewController + @interface SLElementMatchingTestViewController () // fooButton is purposely strong so that we can hold onto it @@ -289,31 +340,7 @@ - (UIView *)tableView:(UITableView *)tableView viewForHeaderInSection:(NSInteger [label sizeToFit]; headerView = label; } else if (self.testCase == @selector(testMatchingTableViewHeaderChildElements)) { - CGFloat headerHeight = [self tableView:tableView heightForHeaderInSection:section]; - CGRect headerRect = (CGRect){ - CGPointZero, - CGSizeMake(CGRectGetWidth(tableView.frame), headerHeight) - }; - headerView = [[UIView alloc] initWithFrame:headerRect]; - CGRect contentRect = CGRectInset(headerRect, 20.0f, 0.0f); - CGFloat halfWidth = CGRectGetWidth(contentRect) / 2.0; - CGSize halfSize = CGSizeMake(halfWidth, CGRectGetHeight(contentRect)); - - UILabel *labelLeft = [[UILabel alloc] initWithFrame:(CGRect){ - contentRect.origin, - halfSize - }]; - labelLeft.textAlignment = NSTextAlignmentLeft; - labelLeft.text = @"left"; - [headerView addSubview:labelLeft]; - - UILabel *labelRight = [[UILabel alloc] initWithFrame:(CGRect){ - CGPointMake(CGRectGetMinX(contentRect) + halfWidth, CGRectGetMinY(contentRect)), - halfSize - }]; - labelRight.textAlignment = NSTextAlignmentRight; - labelRight.text = @"right"; - [headerView addSubview:labelRight]; + headerView = [[SLElementMatchingTestHeader alloc] initWithTestCaseWithSelector:self.testCase]; } return headerView; From 48e633d238a5a5086012fd8c581a761b69aa7c94 Mon Sep 17 00:00:00 2001 From: Jeff Wear Date: Mon, 26 Aug 2013 14:21:55 -0700 Subject: [PATCH 2/4] Instances of `SLElement` can be directed to double-check their validity using UIAutomation. To diagnose bugs in `SLAccessibilityPath` and/or `NSObject (SLAccessibilityHierarchy)`, where Subliminal believes an element to be valid but incorrectly identifies it to UIAutomation, as it is not guaranteed that UIAutomation will raise an exception if an accessibility path is invalid. This setting is disabled by default because it is more likely that there is a bug in a particular test than in UIAutomation and because enabling it will negatively affect the performance of the tests (by requiring an extra call to UIAutomation). --- .../User Interface Elements/SLElement.h | 25 ++++++++++++++++ .../User Interface Elements/SLElement.m | 30 ++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/Sources/Classes/UIAutomation/User Interface Elements/SLElement.h b/Sources/Classes/UIAutomation/User Interface Elements/SLElement.h index 00ad55e..912d4a6 100644 --- a/Sources/Classes/UIAutomation/User Interface Elements/SLElement.h +++ b/Sources/Classes/UIAutomation/User Interface Elements/SLElement.h @@ -174,3 +174,28 @@ /// Used with `+[SLElement elementWithAccessibilityLabel:value:traits:]` /// to match elements with any combination of accessibility traits. extern UIAccessibilityTraits SLUIAccessibilityTraitAny; + + +#pragma mark - Debugging Subliminal + +/** + The methods in the `SLElement (DebugSettings)` category may be useful in debugging Subliminal. + */ +@interface SLElement (DebugSettings) + +/** + Determines whether the specified element should use UIAutomation to confirm that it [is valid](-isValid) + after Subliminal has determined (to the best of its ability) that it is valid. + + If Subliminal misidentifies an element to UIAutomation, UIAutomation will not necessarily raise + an exception but instead may silently fail (e.g. it may return `null` from APIs like `UIAElement.hitpoint()`, + causing Subliminal to think that an element isn't tappable when really it's not valid). + Enabling this setting may help in diagnosing such failures. + + Validity double-checking is disabled (`NO`) by default, because it is more likely that there is a bug + in a particular test than a bug in Subliminal, and because enabling double-checking will + negatively affect the performance of the tests. + */ +@property (nonatomic) BOOL shouldDoubleCheckValidity; + +@end diff --git a/Sources/Classes/UIAutomation/User Interface Elements/SLElement.m b/Sources/Classes/UIAutomation/User Interface Elements/SLElement.m index dbee76d..93cd9db 100644 --- a/Sources/Classes/UIAutomation/User Interface Elements/SLElement.m +++ b/Sources/Classes/UIAutomation/User Interface Elements/SLElement.m @@ -36,6 +36,8 @@ @implementation SLElement { BOOL (^_matchesObject)(NSObject*); NSString *_description; + + BOOL _shouldDoubleCheckValidity; } + (void)load { @@ -161,6 +163,14 @@ - (BOOL)canDetermineTappability { return canDetermineTappability; } +- (BOOL)shouldDoubleCheckValidity { + return _shouldDoubleCheckValidity; +} + +- (void)setShouldDoubleCheckValidity:(BOOL)shouldDoubleCheckValidity { + _shouldDoubleCheckValidity = shouldDoubleCheckValidity; +} + - (SLAccessibilityPath *)accessibilityPathWithTimeout:(NSTimeInterval)timeout { __block SLAccessibilityPath *accessibilityPath = nil; NSDate *startDate = [NSDate date]; @@ -198,6 +208,16 @@ - (void)waitUntilTappable:(BOOL)waitUntilTappable // catch and rethrow exceptions so that we can unbind the path @try { NSString *UIARepresentation = [boundPath UIARepresentation]; + + if (self.shouldDoubleCheckValidity) { + BOOL uiaIsValid = [[[SLTerminal sharedTerminal] evalWithFormat:@"%@.isValid()", UIARepresentation] boolValue]; + if (!uiaIsValid) { + // Subliminal is not properly identifying the element to UIAutomation: + // there is a bug in `SLAccessibilityPath` or `NSObject (SLAccessibilityHierarchy)` + [NSException raise:SLUIAElementInvalidException format:@"Element '%@' does not exist at path '%@'.", self, UIARepresentation]; + } + } + // evaluate canDetermineTappability using the current path // because we can't retrieve another while the element is bound if (waitUntilTappable && [self canDetermineTappabilityUsingAccessibilityPath:accessibilityPath]) { @@ -255,7 +275,15 @@ - (void)examineMatchingObject:(void (^)(NSObject *))block timeout:(NSTimeInterva - (BOOL)isValid { // isValid evaluates the current state, no waiting to resolve the element - return ([self accessibilityPathWithTimeout:0.0] != nil); + SLAccessibilityPath *accessibilityPath = [self accessibilityPathWithTimeout:0.0]; + __block BOOL isValid = (accessibilityPath != nil); + if (isValid && self.shouldDoubleCheckValidity) { + [accessibilityPath bindPath:^(SLAccessibilityPath *boundPath) { + NSString *UIARepresentation = [boundPath UIARepresentation]; + isValid = [[[SLTerminal sharedTerminal] evalWithFormat:@"%@.isValid()", UIARepresentation] boolValue]; + }]; + } + return isValid; } /* From 913cd70e3079b9b89aece540e238379f482eeba6 Mon Sep 17 00:00:00 2001 From: Jeff Wear Date: Mon, 26 Aug 2013 14:22:22 -0700 Subject: [PATCH 3/4] Clarify `SLAlert`'s documentation. --- Sources/Classes/UIAutomation/User Interface Elements/SLAlert.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/Classes/UIAutomation/User Interface Elements/SLAlert.h b/Sources/Classes/UIAutomation/User Interface Elements/SLAlert.h index 1a21064..2012816 100644 --- a/Sources/Classes/UIAutomation/User Interface Elements/SLAlert.h +++ b/Sources/Classes/UIAutomation/User Interface Elements/SLAlert.h @@ -336,6 +336,8 @@ extern const NSTimeInterval SLAlertHandlerDidHandleAlertDelay; /** Returns `YES` if Subliminal should log alerts as they are handled. + Logging is disabled by default. + @return `YES` if alert-handling logging is enabled, `NO` otherwise. @see -setLoggingEnabled: From 5a712ac4b2ea0dcad9675a870fe469c34943acfa Mon Sep 17 00:00:00 2001 From: Jeff Wear Date: Mon, 26 Aug 2013 14:50:22 -0700 Subject: [PATCH 4/4] Subliminal now reloads the accessibility hierarchy as necessary while matching. In certain circumstances, accessibility elements may become "stale", in that they match views which are no longer in the hierarchy. When these elements are queried for their accessibility information, their container will reload _all_ its child elements. This could result in Subliminal finding a path to an element only to have the process of finding that path invalidate the path! We now preemptively reload the accessibility hierarchy while retrieving accessibility elements to ensure that accessibility paths remain valid. --- .../Tests/SLElementMatchingTest.m | 9 ++++ .../SLElementMatchingTestViewController.m | 41 ++++++++++++++++--- .../NSObject+SLAccessibilityHierarchy.m | 30 ++++++++++++-- 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/Integration Tests/Tests/SLElementMatchingTest.m b/Integration Tests/Tests/SLElementMatchingTest.m index 4f0dfb6..b7afc57 100644 --- a/Integration Tests/Tests/SLElementMatchingTest.m +++ b/Integration Tests/Tests/SLElementMatchingTest.m @@ -402,4 +402,13 @@ - (void)testSubliminalRestoresAccessibilityIdentifiersAfterMatchingEvenIfActionT @"After being matched, an object's identifier should have been restored."); } +- (void)testSubliminalReloadsTheAccessibilityHierarchyAsNecessaryWhenMatching { + SLElement *fooLabel = [SLElement elementWithAccessibilityLabel:@"foo"]; + SLAssertTrue([[UIAElement(fooLabel) label] isEqualToString:@"foo"], @"Could not match label."); + + SLAskApp(invalidateAccessibilityHierarchy); + + SLAssertTrue([[UIAElement(fooLabel) label] isEqualToString:@"foo"], @"Could not match label."); +} + @end diff --git a/Integration Tests/Tests/SLElementMatchingTestViewController.m b/Integration Tests/Tests/SLElementMatchingTestViewController.m index 1d0bc55..0e99927 100644 --- a/Integration Tests/Tests/SLElementMatchingTestViewController.m +++ b/Integration Tests/Tests/SLElementMatchingTestViewController.m @@ -132,6 +132,15 @@ - (instancetype)initWithTestCaseWithSelector:(SEL)testCase { rightLabel.textAlignment = NSTextAlignmentRight; rightLabel.text = @"right"; _rightView = rightLabel; + } else if (testCase == @selector(testSubliminalReloadsTheAccessibilityHierarchyAsNecessaryWhenMatching)) { + UILabel *label = [[UILabel alloc] initWithFrame:CGRectZero]; + label.textAlignment = NSTextAlignmentLeft; + label.text = @"foo"; + _leftView = label; + + UIButton *button = [UIButton buttonWithType:UIButtonTypeRoundedRect]; + [button setTitle:@"bar" forState:UIControlStateNormal]; + _rightView = button; } else { NSAssert(NO, @"Unexpected test case: %@", NSStringFromSelector(testCase)); } @@ -151,6 +160,10 @@ - (void)layoutSubviews { _rightView.frame = rightViewFrame; } +- (void)removeRightView { + [_rightView removeFromSuperview]; +} + @end @@ -181,6 +194,8 @@ @implementation SLElementMatchingTestViewController { UIPopoverController *_popoverController; UIActionSheet *_actionSheet; + + SLElementMatchingTestHeader *_headerView; } + (NSString *)nibNameForTestCase:(SEL)testCase { @@ -206,7 +221,8 @@ + (NSString *)nibNameForTestCase:(SEL)testCase { (testCase == @selector(testCannotMatchIndividualChildLabelsOfTableViewCell)) || (testCase == @selector(testMatchingNonLabelTableViewCellChildElement)) || (testCase == @selector(testMatchingTableViewHeader)) || - (testCase == @selector(testMatchingTableViewHeaderChildElements))) { + (testCase == @selector(testMatchingTableViewHeaderChildElements)) || + (testCase == @selector(testSubliminalReloadsTheAccessibilityHierarchyAsNecessaryWhenMatching))) { return @"SLTableViewChildElementMatchingTestViewController"; } else { return nil; @@ -227,6 +243,7 @@ - (instancetype)initWithTestCaseWithSelector:(SEL)testCase { [[SLTestController sharedTestController] registerTarget:self forAction:@selector(showPopoverWithActionSheet)]; [[SLTestController sharedTestController] registerTarget:self forAction:@selector(showActionSheet)]; [[SLTestController sharedTestController] registerTarget:self forAction:@selector(hideActionSheet)]; + [[SLTestController sharedTestController] registerTarget:self forAction:@selector(invalidateAccessibilityHierarchy)]; } return self; } @@ -278,7 +295,8 @@ - (void)viewDidLoad { if (self.tableView) { if ((self.testCase == @selector(testMatchingTableViewCellTextLabel)) || (self.testCase == @selector(testMatchingTableViewHeader)) || - (self.testCase == @selector(testMatchingTableViewHeaderChildElements))) { + (self.testCase == @selector(testMatchingTableViewHeaderChildElements)) || + (self.testCase == @selector(testSubliminalReloadsTheAccessibilityHierarchyAsNecessaryWhenMatching))) { _testTableViewCellClass = [UITableViewCell class]; } else if ((self.testCase == @selector(testMatchingNonLabelTableViewCellChildElement)) || (self.testCase == @selector(testMatchingTableViewCellWithCombinedLabel)) || @@ -316,7 +334,8 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N if ((self.testCase == @selector(testMatchingTableViewCellTextLabel)) || (self.testCase == @selector(testMatchingTableViewHeader)) || - (self.testCase == @selector(testMatchingTableViewHeaderChildElements))) { + (self.testCase == @selector(testMatchingTableViewHeaderChildElements)) || + (self.testCase == @selector(testSubliminalReloadsTheAccessibilityHierarchyAsNecessaryWhenMatching))) { cell.textLabel.text = @"fooLabel"; } else { NSAssert([cell isKindOfClass:[SLElementMatchingTestCell class]], @@ -339,8 +358,11 @@ - (UIView *)tableView:(UITableView *)tableView viewForHeaderInSection:(NSInteger label.text = @"fooHeader"; [label sizeToFit]; headerView = label; - } else if (self.testCase == @selector(testMatchingTableViewHeaderChildElements)) { - headerView = [[SLElementMatchingTestHeader alloc] initWithTestCaseWithSelector:self.testCase]; + } else if ((self.testCase == @selector(testMatchingTableViewHeaderChildElements)) || + (self.testCase == @selector(testSubliminalReloadsTheAccessibilityHierarchyAsNecessaryWhenMatching))) { + _headerView = [[SLElementMatchingTestHeader alloc] initWithTestCaseWithSelector:self.testCase]; + NSAssert([self numberOfSectionsInTableView:tableView] == 1, @"We only expect to track one header."); + headerView = _headerView; } return headerView; @@ -446,4 +468,13 @@ - (void)hideActionSheet { [_actionSheet dismissWithClickedButtonIndex:0 animated:NO]; } +- (void)invalidateAccessibilityHierarchy { + // Removing a subview of a table view header from the view hierarchy, + // without reloading the header, is one way to invalidate the accessibility hierarchy: + // the accessibility container that mocks the header will retain the accessibility element + // that mocked that subview until that element is queried by the accessibility system, + // at which time all child elements of the container will be replaced. + [_headerView removeRightView]; +} + @end diff --git a/Sources/Classes/UIAutomation/User Interface Elements/NSObject+SLAccessibilityHierarchy.m b/Sources/Classes/UIAutomation/User Interface Elements/NSObject+SLAccessibilityHierarchy.m index ca45a77..35a96ab 100644 --- a/Sources/Classes/UIAutomation/User Interface Elements/NSObject+SLAccessibilityHierarchy.m +++ b/Sources/Classes/UIAutomation/User Interface Elements/NSObject+SLAccessibilityHierarchy.m @@ -98,9 +98,33 @@ - (NSArray *)slChildAccessibilityElementsFavoringSubviews:(BOOL)favoringSubviews NSMutableArray *children = [NSMutableArray array]; NSInteger count = [self accessibilityElementCount]; if (count != NSNotFound && count > 0) { - for (NSInteger i = 0; i < count; i++) { - [children addObject:[self accessibilityElementAtIndex:i]]; - } + // Certain accessibility containers, like those that mock table view headers, + // may contain "stale" accessibility elements: elements which initially carry no information, + // but when queried (for some accessibility property) cause their container to reload + // and replace _all_ of its elements. + // We ensure we return valid children by asking for the accessibility label of each element, + // then checking if the container yet vends that element--if it doesn't, we retrieve all children again. + BOOL shouldReloadChildren, haveReloadedChildren = NO; + do { + shouldReloadChildren = NO; + for (NSInteger i = 0; i < count; i++) { + id element = [self accessibilityElementAtIndex:i]; + (void)[element accessibilityLabel]; + if (element != [self accessibilityElementAtIndex:i]) { + // Protect against tests entering an infinite loop, + // in case there's any scenario where the hierarchy might not stabilize. + if (haveReloadedChildren) { + SLLogAsync(@"The accessibility hierarchy is unstable: the accessibility children of %@ are likely invalid.", self); + } else { + shouldReloadChildren = YES, haveReloadedChildren = YES; + [children removeAllObjects]; + break; + } + } + + [children addObject:element]; + } + } while (shouldReloadChildren); } return children; }