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.

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

Option to use accessabilityIdentifier instead of accessibilityLabel #243

Closed
grgcombs opened this issue Aug 27, 2013 · 25 comments
Closed

Option to use accessabilityIdentifier instead of accessibilityLabel #243

grgcombs opened this issue Aug 27, 2013 · 25 comments

Comments

@grgcombs
Copy link

Given that accessibilityLabel is an outwardly-facing string that is actually used by accessibility screen readers (and should be localized to the device user's language), Apple now provides an alternate property (iOS 5+) that is specifically intended for UI Automation purposes:

@property(nonatomic, copy) NSString *accessibilityIdentifier;

An identifier can be used to uniquely identify an element in the scripts you write using the UI Automation interfaces. Using an identifier allows you to avoid inappropriately setting or accessing an element’s accessibility label.

Availability
Available in iOS 5.0 and later.

_Source:_ UIAccessibilityIdentification Protocol Reference

Ideally, the KIF framework would give us an easy means to swap out the logic that looks for accessibilityLabel and instead use accessibilityIdentifier.

@bnickel
Copy link
Contributor

bnickel commented Aug 27, 2013

Are there specific cases you're looking to address? In general, accessibilityLabel would be preferable to accessibilityIdentifier since the former is something that provides meaning to the user and makes the app functional while the latter is meaningful only to the developer. The value of KIF is that is does what human testers would do and human testers are completely unaware of the accessibility identifier.

Where I see accessibilityIdentifier adding value is in things like -tapRowInTableViewWithAccessibilityLabel:atIndexPath: which creates exactly the situation described in the docs. The table view doesn't need a label and an identifier is more appropriate. Of course, this is only required because it is the easiest way to convey "select the fourth row in the table view on the left side of the screen".

For the case where devices may have different localizations, this is something you need to address in a test strategy. Should you have all your test devices set to English? Should you have one device for each locale? Should you swizzle -[NSBundle localizedStringForKey:] in your tests? Etc. Would be an interesting discussion for the Google Group.

@grgcombs
Copy link
Author

Excellent points. For the project I'm on at the moment, the labels and values are very dynamic, determined at runtime through some localized metadata on the server. So we're looking for something that is just as you say, developer-oriented that can be more stable.

@bnickel
Copy link
Contributor

bnickel commented Sep 16, 2013

Closing in favor of #258.

@bnickel bnickel closed this as completed Sep 16, 2013
@michaelmcguire
Copy link

I think being able to use the accessibilityIdentifier might be useful for certain sorts of verifications. I would like to verify the contents of a table that is dynamically filled with data. My first inclination is to get the UITableView (via the accessibilityIdentifier) and verify the number of sections and number of rows in that section.

Perhaps I'm not doing it the "KIF-way" though?

@sansari
Copy link

sansari commented Feb 11, 2014

Hmm, I'm a bit confused by this. In the latest KIF versions (HEAD, beyond 2.0), I noticed that the following method is deprecated in the KIF TestActor:

scrollViewWithAccessibilityLabel

The deprecation warning suggests using this as the alternative:

scrollViewWithAccessibilityIdentifier

Now, when I first started using KIF, it was a great tool for forcing us to place accessibility labels in places that are critical for the user. However, for the case of these scroll lists (i.e. table views), it would now seem that I should instead be setting an identifier, and not a label.

Just so I understand the best practice here, should we be setting both identifiers and labels, or just identifiers for things like a tableVIew that is scrolled. Imagine a message list -- if it was an accessible view, would you want "Message List" to show up? I'm thinking yes, but am unsure here.

Thoughts?

@blastum
Copy link

blastum commented Jul 27, 2014

I'm a latecomer.

"The value of KIF is that is does what human testers would do and human testers are completely unaware of the accessibility identifier."

While I appreciate the high motives of this, I'm not sure where the advantage lies in using the accessibility label over the accessibility identifier. I guess it will make sure the accessibility label is set, but it's won't verify that a label is coherent and will just cruft up the code to have lots of NSLocalizedLabelWithDefaultValues copied from one piece of code to another.

@grgcombs
Copy link
Author

Yeah ... My interest is in making my test code more agnostic, whether about late-term design choices or about the selected locale (unless the test is targeting the localization output directly).

When using accessibilityIdentifier, we can make the strings (and thus the tests) more specific, more immune to design changes (when they decide a button should say "Submit" instead of "Save") and programmer-oriented.

If your test is to see that an editable form will submit the right data to the server, it shouldn't matter (at that point) whether the completion button has a label saying Submit, Send, Save, Done or "Envoyer"

When the designers and PMs decide this specific button should instead say "Do Stuff" in English (while other forms still say "Save"), do I really need to go through and update all of this form's tests that otherwise don't care about the display string? If my tests are instead using the identifier AccountCreationFormSendButton, the only test that should need updating is the one that looks to see if the display label is correct for the locale of choice.

Does this help explain the use-case better?

Greg

Sent from my iPhone.

On Jul 27, 2014, at 11:32 AM, blastum notifications@github.com wrote:

I'm a latecomer.

"The value of KIF is that is does what human testers would do and human testers are completely unaware of the accessibility identifier."

While I appreciate the high motives of this, I'm not sure where the advantage lies in using the accessibility label over the accessibility identifier. I guess it will make sure the accessibility label is set, but it's won't verify that a label is coherent and will just cruft up the code to have lots of NSLocalizedLabelWithDefaultValues copied from one piece of code to another.


Reply to this email directly or view it on GitHub.

@bnickel
Copy link
Contributor

bnickel commented Jul 29, 2014

Late term design choices aren't that exciting to me as a reason for accessibilityIdentifier because functional tests are there to validate that the app is functioning as designed, and that includes a "Done" button when the design says "Done" or a "Send" button if the design says "Send". If you expect to have 10 forms all with the same submit button, hopefully your test code (and your app code) will be able to abstract that out:

@implementation KIFUITestActor (MyForms)
- (void)submitStandardForm
{
    [self tapViewWithAccessibilityLabel:@"Done"];
}

Locale on the other hand is easier to deal with simply by setting the locale environment variable in your test configuration. This is especially valuable because if you have different behaviors for certain locales you can test it out, although the configuration is more of a pain.

Of course all things in testing are subjective and these are just my opinions, but that's why I've not been interested in adding them.

@bnickel
Copy link
Contributor

bnickel commented Jul 29, 2014

@sansari Just saw you comment. The reason accessibilityIdentifier is favored for scrolling is because UIScrollView and UITableView don't actually announce or otherwise use their accessibilityLabel in terms of actually accessibility functionality. In essence the deprecated methods were using accessibilityLabel as an accessibilityIdentifier, just a searchable string for finding the control.

@babbage
Copy link

babbage commented Sep 30, 2014

There are places where this would be extremely useful to me. For instance, I'm currently including the following code:

#ifdef DEBUG
[self.background setAccessibilityLabel:@"Header background"];
#endif

The reason why I want to set this, is so that in my KIF tests I can:

UIView * headerBackground = [tester waitForViewWithAccessibilityLabel:@"Header background"];

Using this reference to the view, I can then get its frame, get the frames of other elements that are supposed to overlay the same background, and then do some maths in XCTAssertEqual that confirm that the background view is correctly expanding as the content gets larger, to test my auto layout constraints are all continuing to work correctly.

Wrapping the label in #ifdef DEBUG means that end users won't ever get it announced as an accessibility label, but it'd be so much better to just be able to use the accessibility identifier in this context.

@phatmann
Copy link
Contributor

@babbage Would it be possible to put the accessibility label on the header, and then find the header background via viewWithTag?

@bnickel has made it very clear that he does not want KIF to use accssibilityIdentifiers except when necessary, and I will honor that philosophy. Perhaps you can convince him otherwise, in order to handle your use case.

@benblechman
Copy link

I'm evaluating KIF, and would like to push for adopting it with my large team. I'm going to need to modify KIF to use accessibility identifiers (I see there is already a PR to do this). Using labels is problematic. If I want to run the same test across multiple languages and verify labels aren't truncating and that autolayout is working in each localization I cannot do this if I must identify views by the label. Similarly, if I am displaying any view with dynamic content (most of our applications), say a text field that displays the price of a search results, the accessibiltyLabel is going to be the price, but I might still want to uniquely identify the label in my test.

We often build up our accessibilityLabels dynamically based on the needs of a VoiceOver user. Forcing automated testing to use labels means that any dynamic content can't be located by KIF.

I like the idea of verifying that accessible elements have labels, but I'd rather do that explicitly as a testing step than having that forced by KIF.

I understand there may be reasons people may not want or need to use identifiers, but it's the only sensible option for our needs (and Apple's recommended path for automation). Is there a reason this couldn't just be a preprocessor definition (or a runtime configuration variable) KIF supports? Like KIF_PRIORITIZE_ACCESSIBILITY_IDENTIFIERS

@bnickel
Copy link
Contributor

bnickel commented Oct 22, 2014

Are you running the tests in multiple languages, taking screenshots and verifying the screenshots are okay? This is the best justification I've seen thus far.

As far as the dynamic content issue is concerned, how are you controlling your test data? The contents of the dynamic label are usually a result to be validated by the test and should be predictable with consistent test data.

I do not want to ever look at an accessibility identifier with a parameter called accessibilityLabel. Instead there should be a category on KIFUITestActor to provide similar methods with identifiers, provided as a subspec.

Sent from my iPhone

On Oct 22, 2014, at 11:23 AM, benblechman notifications@github.com wrote:

I'm evaluating KIF, and would like to push for adopting it with my large team. I'm going to need to modify KIF to use accessibility identifiers (I see there is already a PR to do this). Using labels is problematic. If I want to run the same test across multiple languages and verify labels aren't truncating and that autolayout is working in each localization I cannot do this if I must identify views by the label. Similarly, if I am displaying any view with dynamic content (most of our applications), say a text field that displays the price of a search results, the accessibiltyLabel is going to be the price, but I might still want to uniquely identify the label in my test.

We often build up our accessibilityLabels dynamically based on the needs of a VoiceOver user. Forcing automated testing to use labels means that any dynamic content can't be located by KIF.

I like the idea of verifying that accessible elements have labels, but I'd rather do that explicitly as a testing step than having that forced by KIF.

I understand there may be reasons people may not want or need to use identifiers, but it's the only sensible option (and Apple's recommended path for automation). Is there a reason this couldn't just be a preprocessor definition (or a runtime configuration variable) KIF supports? Like KIF_PRIORITIZE_ACCESSIBILITY_IDENTIFIERS


Reply to this email directly or view it on GitHub.

@benblechman
Copy link

Yes, we do run in multiple languages and we have automated calabash based tools for taking screenshots. I could imagine automating some of this too with KIF, where we can detect in code if truncation has occurred where it should not. Or detecting values that aren't localized. Or ?

I am in the process of investigating how we will add fine grained automated UI testing to our overall testing plan. We already do some calabash based testing, and have been migrating away from accessibiltyLabels towards accessibilityIdentifiers. The biggest problem with calabash for iOS developers is the need to learn a new platform/language, and the limits having a web server between us and the code imposes

I would like to be able to run some tests that use static test data as you mention, but also be able to run some tests against live from the server data that won't be static. Obviously live data will limit the kinds of tests that could be run reliably, but there is still some full stack integration testing I'd like to be able to do with KIF. For instance, the price field I mentioned won't have a static value, but I should be able to run a test to say that it has a value, and that under no circumstances should it be truncated.

More to the point, the accessibilityLabel is part of the VoiceOver UI. As such, it is potentially as variable as any other GUI element. The VoiceOver UI and the visual UI need to be tested independently. If I have a test that is intended to verify the visual UI, I should be able to modify the VoiceOver UI without any impact on the test. This is something we do frequently, for instance, a label will have abbreviated text which does not read well with VoiceOver, so we modify the accessibility label to have the non-abbreviated version. In this case I might want my test to find the label with an identifier, and verify both the accessibilityLabel and the labels text.

Really I don't think accessibilityIdentifier should even be part of the accessibility API. It should just be called something like automationIdentifier, and used for this purpose. It has nothing to do with accessibility.

@klazuka
Copy link

klazuka commented Nov 5, 2014

I agree 100% with @benblechman. Automation testing and accessibility are separate concerns. I don't want test engineers writing accessibility code. Their job is to write tests, not VoiceOver UI.

@grgcombs
Copy link
Author

grgcombs commented Nov 6, 2014

I'm sold. From what I can surmise, the necessary code to transition to accessibilityIdentifier isn't particularly daunting -- if the hesitation is labor, I/we can set it up as a build setting condition and issue a pull request. With that, @bnickel, are we good? By default everything would behave exactly as it does today, but those that need/want accessibilityIdentifier can enable a switch and they'll be good to go for their own tests.

@bnickel
Copy link
Contributor

bnickel commented Nov 6, 2014

I'm starting to come around that it's a reasonable use case in certain circumstances. I say certain circumstances again because you are not actually testing user functionalty when you use hooks that are not available to actual users. Your testers shouldn't be writing accessibility code, your coders should be writing code that is accessible and thus consumable by a testing system that impersonates a user.

That said, a flag that changes the search from a label to identifier is not a viable approach because it radically changes the behavior of the framework and makes the method names lies. Instead there should be an additional category on KIFUITestActor with methods like:

- (UIView *)tapViewWithAccessibilityIdentifier:(NSString *)identifier;

This would behave similarly to the existing API but instead wrap the already publicly available - waitForAccessibilityElement:view:withIdentifier:tappable: method. I would want this packaged as a category and included in a Cocoapods subspec. I would cynically call it pod KIF/KeepItNotQuiteFunctional or more diplomatically pod KIF/IdentifierTests.

@benblechman
Copy link

I agree with @bnickel, it would be cleaner to just to keep the interface explicit with Identifier versions. In some ways having it fall back from identifier to label eases the migration path for those who want to migrate, but I think making it explicit removes the ambiguity, and avoids having to rename or change the behavior of existing methods. I've never used Cocoapods, and we don't use it currently with our projects; I'm not sure I understand the motivation for making this be something separate. Why not just include both as standard, and let people choose what suits their needs?

I've already modified our local copy of KIF, but it's going to make it pain when we want to update.

I'm willing and available to do this work to create the PR for review.

@bnickel
Copy link
Contributor

bnickel commented Nov 6, 2014

Why not just include both as standard, and let people choose what suits their needs?

Because I want people using it to be making a conscious choice rather than just falling into it. Have I mentioned that I really don't like it?

I went ahead and laid the groundwork in 1feee1e with KIFUITestActor (IdentifierTests) and the corresponding suite AccessibilityIdentifierTests. Feel free to build from there.

@benblechman
Copy link

OK, thank you.

@babbage
Copy link

babbage commented Nov 7, 2014

I really like the philosophy, which is predicated on the two ideas that the testing should remain functional, and that it is a great thing to ensure that every aspect of an application that is being tested is accessible. I think it is clear that this philosophy has been without question positive, works for more than 95% of the use cases, and it has certainly meant I have added accessibility labels to parts of my app under development earlier than I might have otherwise.

It is important however not to confuse the two issues, as they should not be tightly coupled. Whether using the accessibiltyLabel, or accessibilityIdentifier, the testing is still functional. That's because either of those is simply getting a pointer to the element, on which a tap is being registered using an extension of UIView, tapAtPoint:, which itself uses an extension of UITouch, initAtPoint:. What this means is that the functional aspect of the testing is simulating the functional interactions of a sighted user, not a user using the accessibility interface.

It's an admirable goal to still ensure that all user interaction is with elements that are accessible. One way to ensure this would for all methods that simulate user interaction, like a proposed tapViewWithAccessibilityIdentifier, to return a error (or could be a warning) if the accessibilityLabel is blank, or if accessibility is disabled for the element, but it would be the value of the accessibilityIdentifier only that the code is searching for to actually get the reference to the element. This would maintain the current desirable outcome of ensuring that all elements the user interacts with are accessible, while avoiding the current problems various people have outlined. People could of course build in tests to their code that confirm the value of the accessibilityLabel is set to the expected value, where this can be predicted, in the same way that we currently test for the expected value of a UILabel.

Meanwhile, any methods that do not simulate user interaction (e.g., waitForViewWithAccessibilityIdentifier) would match on the accessibiltyIdentifier, without including any assumptions about accessibiltyLabels being set, since frequently these are being used simply to manage the functional testing process or to get a reference to an element for other purposes (e.g., to test if its bounds fall entirely within that of another element, for example). Obviously this doesn't preclude having a waitForViewWithAccessibilityLabel also.

Best of both worlds? :) Also raises the interesting question of actually developing functional tests that genuinely do use and thus functionally test the accessibility interface. Not sure if this is already possible with the current setup of KIF. May be?

@benblechman
Copy link

@babbage My opinion is that testing the VoiceOver UI should be completely decoupled from testing the visual UI. I intend to have verifications I can run on our view hierarchy as part of our tests. One of these will be that accessible elements should have localized labels (regardless of whether they are "tappable"). For that matter tappable elements should have a trait like UIAccessibilityTraitButton. We intend our applications to be fully accessible, but to really support a VoiceOver UI requires careful consideration beyond just enforcing labels are present. Labels need to be localized. Elements need to be "walkable" in a coherent order (iOS default behavior easily broken here). Elements need to be grouped/combined together in a way that makes sense. etc. As I've been going through this exercise of writing some initial KIF tests, looking at existing code used by older Calabash tests, I see accessibility labels in our codebase such as not localized "Icon Foo Button". These were obviously put in place to support automated testing, but are clearly wrong for a VoiceOver user. So my opinion has moved more strongly into the camp that our tests should only use identifiers to identify elements, and if we want to assert anything by default about labels it should be that they not equal the identifiers and should not take the format of our string keys "ICON_FOO_BUTTON". Forcing labels to be present but not carefully considered makes it harder to verify they are helpful labels for a VoiceOver user.

@babbage
Copy link

babbage commented Nov 8, 2014

Forcing labels to be present but not carefully considered makes it harder to verify they are helpful labels for a VoiceOver user.

Forcing labels to be present proves nothing about how carefully considered or helpful they are. There is however no basis for arguing that forcing them to be present would make this any harder to verify than it would be otherwise.

Forcing them to not equal the identifiers doesn't seem sensible to me—it's not hard to imagine straightforward use cases where there would be no particular advantage in having to come up with an identifier that was different to what the correct accessible label should be.

We clearly agree however that the current strictures don't actually result in a shortcut on the work required to properly support a VoiceOver UI, and that there is no shortcut for that.

@rsaunders100
Copy link

I am a bit late to the party. I Just want to say I really like the new category: KIFUITestActor (IdentifierTests) - I agree that its better to have a separate interface rather than search labels and identifiers.

I have one suggestion - put IdentifierTests into the main pod not a subspec.

Why not just include both as standard, and let people choose what suits their needs?

Because I want people using it to be making a conscious choice rather than just falling into it.

Could this goal be achieved with header documentation instead? What I love about KIF is the fast integration time. I feel like this flow could be improved:

  1. Hit the 5% use case that needs accessibilityIdentifier
  2. Search the header doc for tapViewWithAccessibilityIdentifier:
  3. Check the readme.md for notes on accessibilityIdentifier
  4. Search the issues for accessibilityIdentifier. and write a comment in the wrong issue (like I did)
  5. Find the right issue and read though the whole conversation.
  6. Work out how pod sub specs work.
  7. Integrate and use the IdentifierTests category.

Alternatively - could we add a summary of this identifier vrs label discussion + referece to pod KIF/IdentifierTests in the readme.md?

@benblechman
Copy link

Note, I added additional accessibilityIdentifier support in this PR, waiting to be reviewed/merged:

#494

As I've already mentioned I do not think this should be a separate category, but it's not a critical issue. I had a need to wrap many of these actor methods anyways, so I've extended KIFUITestActor for our project, and "hidden" this category in the extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants