matcherForEnabledElement default matched value should be NO/false (#479)

[RichardGuion]

No description provided.

[tirodkar]

Hmm, looks like this seems to make a lot of our tests fail. Were you able to get the FunctionalTests running fine?

[RichardGuion]

ok - a LOT of functional tests are failing. After an initial investigation it is this:

The test has a view, and it wants to tap on it. Tap Actions like in GreyTapAction.m use grey_enabled() as part of the criteria:

- (instancetype)initWithType:(GREYTapType)tapType
                numberOfTaps:(NSUInteger)numberOfTaps
                    duration:(CFTimeInterval)duration
                    location:(CGPoint)tapLocation {
  NSAssert((numberOfTaps > 0), @"You cannot initialize a tap action with zero taps.");

  NSString *name = [GREYTapAction grey_actionNameWithTapType:tapType
                                                    duration:duration
                                                numberOfTaps:numberOfTaps];
  self = [super initWithName:name
                 constraints:grey_allOf(grey_not(grey_systemAlertViewShown()),
                                        grey_anyOf(grey_accessibilityElement(),
                                                   grey_kindOfClass([UIView class]),
                                                   nil),
                                        grey_enabled(),

However stepping through the code, the matcher is looking for a UIControl:

+ (id<GREYMatcher>)matcherForEnabledElement {
  MatchesBlock matches = ^BOOL(id element) {
    BOOL matched = NO;
    if ([element isKindOfClass:[UIControl class]]) {
      UIControl *control = (UIControl *)element;
      matched = control.enabled;
    }

However the test (testAccessibilityElementTappedSuccessfullyWithTapAtPoint for example) is passing in a UIView derived object like a UITableViewLabel -> this is not a UIControl and so it would always return NO on a view.

[khandpur]

The enabled matcher is meant to provide a way to check whether the element is actively accepting input events. This isn't clear from the documentation. Maybe this calls for a change in behavior of the matcher (and subsequently the documentation)?

Proposing the following behavior for grey_enabled matcher:

1. For UIControls, check the enabled property and userInteractionEnabled property because they are a subclass of UIView
2. For UIViews check userInteractionEnabled property

With this change in behavior, grey_userInteractionEnabled matcher becomes obsolete. So, I propose to deprecate it by adding a NSLog(@"Deprecated. Use <the_new_matcher> instead"); and remove it 2 releases from now.

@RichardGuion In terms of changes to matcherForEnabledElement, it would look something like:

+ (id<GREYMatcher>)matcherForEnabledElement {
  MatchesBlock matches = ^BOOL(UIView *element) {
    BOOL enabled = [element isUserInteractionEnabled];
    if ([element isKindOfClass:[UIControl class]]) {
      enabled = enabled && [element enabled];
    }
    return enabled;
  };
  DescribeToBlock describe = ^void(id<GREYDescription> description) {
    [description appendText:@"enabled"];
  };
  id<GREYMatcher> isEnabledMatcher =
      [[GREYElementMatcherBlock alloc] initWithMatchesBlock:matches descriptionBlock:describe];
  // We also check that we don't have any disabled ancestors because checking for enabled ancestor
  // will return the first enabled ancestor even through there might be disabled ancestors.
  return grey_allOf(grey_kindOfClass([UIView class]), isEnabledMatcher, grey_not(grey_ancestor(grey_not(isEnabledMatcher))), nil);
}

What do you guys think?

[RichardGuion]

@khandpur - I tried a slight variation of your suggested code:

+ (id<GREYMatcher>)matcherForEnabledElement {
  MatchesBlock matches = ^BOOL(UIView *element) {
    BOOL enabled = [element isUserInteractionEnabled];
    if ([element isKindOfClass:[UIControl class]]) {
      UIControl *control = (UIControl *)element;
      enabled = enabled && control.enabled;
    }
    return enabled;
  };
  DescribeToBlock describe = ^void(id<GREYDescription> description) {
    [description appendText:@"enabled"];
  };
  id<GREYMatcher> isEnabledMatcher =
      [[GREYElementMatcherBlock alloc] initWithMatchesBlock:matches descriptionBlock:describe];
  // We also check that we don't have any disabled ancestors because checking for enabled ancestor
  // will return the first enabled ancestor even through there might be disabled ancestors.
  return grey_allOf(grey_kindOfClass([UIView class]), isEnabledMatcher, grey_not(grey_ancestor(grey_not(isEnabledMatcher))), nil);
}

Tests still fail even though it can access the label view -> isUserInteractionEnabled returns NO.

[khandpur]

I am going to try it locally to see if there's something we can do to make those tests happy.

[tirodkar]

PING. Any update on this @RichardGuion ?

[tirodkar]

Sorry, ignore the previous comment. We'll test it out and update this issue. Thanks!