kif-framework / KIF

Keep It Functional - An iOS Functional Testing Framework
Other
6.2k stars 915 forks source link

Discussion: AccessibilityIdentifier method parity with accessibilityLabel methods #1028

Open mikelupo opened 6 years ago

mikelupo commented 6 years ago

The original KIF authors (@efirestone, @bnickel to name a few) allowed an addition of finding/tapping elements by accessibilityIdentifier as an addition (think plugin) to KIF. I vaguely recall reading a discussion regarding accessibility identifiers, stating that we should focus on the use accessibilityLabels, as that was Apple's recommendation. People contributing to KIF added identifier tests as per their own need, but parity between accessibility label methods and identifier methods was never maintained.

Now that Apple has shifted gears in that they suggest using accessibility identifiers, we need to revisit two things:

  1. How KIF integrates the accessibility identifier methods. Currently, it's like a plugin. You don't get them for free by merely installing KIF. You have to explicitly include them.
  2. Parity between label and identifier methods.

An example of the lack of parity is the fact that you can call tryFindingTappableViewWithAccessibilityLabel: with traits, but you cannot call tryFindingTappableViewWithAccessibilityIdentifier: with traits (in retrospect, that method does not exist at all for you readers, except for me as I've added it to my own KIF implementation but have not pushed it up in a PR.) which further proves my point.

@justinseanmartin @RoyalPineapple I invite your input to this discussion.

justinseanmartin commented 6 years ago

If you use the new API exposed by KIFUIViewTestActor.h, it has 100% parity between accessibilityIdentifier and accessibilityLabel. This is because it decouples matchers (e.g. usingIdentifier: or usingTraits: - which can be used in combination) from the actions themselves (e.g. tap or enterText). It is also fully exposed in KIF.h and doesn't require the CocoaPods subspec for matching on identifiers.

In general, I think the original intent is still reasonable and valuable for people to keep in mind regarding not using accessibilityIdentifiers as that information isn't exposed for accessibility users. I do think though that we're moving away from being opinionated on that front in the KIF API itself, and possibly get rid of the subspec in the future (it isn't useful when using viewTester).

Do you have a link regarding Apple suggesting using accessibility identifiers for automation any more than they have in the past?

justinseanmartin commented 6 years ago

Any followup thoughts on this @mikelupo? Still worth tracking anything with this issue?

justinseanmartin commented 6 years ago

Ping on this @mikelupo and @RoyalPineapple for thoughts.

mikelupo commented 6 years ago

Hi @justinseanmartin I apologize for my inattention to this matter.

I agree wholeheartedly that decoupling the methods from the matchers is the right way to go. Many other tools in the industry (including cough-cough Android's espresso). From where I sit, there is no need to discuss further. I will leave it up to you to close this topic at your convenience.

RoyalPineapple commented 6 years ago

Agree with @justinseanmartin that we should be pushing everyone towards the KIFUIViewTestActor which supports identifiers out of the box.

I'd like to begin work on deprecating the old API. I started a PR a few weeks ago, but didn't get time to push it over the line.

I agree that identifiers are a bad tool to use all the time, but when they're needed they're often a really great tool for the job, I don't know that hiding it in a "dark toolbox" is the best solution. Its really about user education and helping people understand when is the appropriate time to be adding identifiers.

dostrander commented 4 years ago

@RoyalPineapple @mikelupo @justinseanmartin I'm doing some house keeping and noticed there was this discussion. Is there any more discussion that should be had and/or any action items that should come out of this?

justinseanmartin commented 4 years ago

I think we should remove the subspec for accessibility identifier if/when we ever do a major version bump and introduce breaking changes. That would likely be the time wheer we'd remove KIFUIViewTester entirely.