ViewInspector icon indicating copy to clipboard operation
ViewInspector copied to clipboard

add convenience finding methods and XCTestCase to test by accessibility

Open FrauR opened this issue 3 years ago • 6 comments

Why these changes

We would like to remove boiler plate code and encourage developers to use internal architecture independent find(..) methods that don't rely on internal implementation logic (such as the specific view types and even knowledge about the view tree).

Types of changes

What types of changes does your code introduce to View Inspector? Put an x in the boxes that apply

  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x] Documentation Update (if none of the other choices apply) // TODO

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • [x] I have read the README doc
  • [x] I have added tests that prove my fix is effective or that my feature works
  • TODO: [x] I have added necessary documentation (if appropriate)
  • [x] Any dependent changes have been merged and published in upstream modules

FrauR avatar Mar 23 '22 14:03 FrauR

Hi @nalexn, would you be interested in having these added to the project. If yes, I will go ahead and also add documentation to complete the PR.

FrauR avatar Mar 23 '22 16:03 FrauR

Hey @FrauR

Thank you for submitting the PR! The new methods you've added: find(viewWithAccessibilityIdentifier: ) and find(viewWithAccessibilityLabel:) are great - I'm happy to merge them in. Your other helper ViewInspectorTestCase is also quite handy, however, I'd keep it as a separate addition to the library for these reasons:

  1. I saw many people use Quick / Nimble frameworks instead of XCTest, your helper is tied to XCTest
  2. The wrapper does save some boilerplate code, however, it significantly limits the API for testing. That is, for your case it could be sufficient to have shouldFind(accessibilityLabel:), func shouldNOTFind(accessibilityLabel:), and a few more, but there are dosens of other view search conditions and combinations that may be needed in other projects. Covering all available search queries would need significant efforts and bloat up the size of your helper as well.

You can certainly publish your helper separately as a snippet and we can leave a reference to it from the readme or other place in the project.

Let me know what you think, and possibly update the PR as needed.

Thanks!

nalexn avatar Mar 24 '22 08:03 nalexn

I think the additions here, if they are not preventing the use of Quick/Nimble, would be welcome to many people including our team. This code is not hitting production (it's part of the unit test target) so even if the library has a few extra functions that cater only to XCTest users, it should be fine.

Looking forward to using them!

ekscrypto avatar May 10 '22 03:05 ekscrypto

@nalexn sorry, that I didn't get back earlier.

I totally agree with you and also think the XCTestCase is coupling us too much and should be up to the user of the library to create it. I removed the commit and added examples to the guide. Should be good to go now.

FrauR avatar May 10 '22 19:05 FrauR

@nalexn can we merge this?

FrauR avatar Jun 09 '22 15:06 FrauR

Hey, sure, I'll merge this for the next version. I just wanted to wrap up work on a couple more tickets before making a release. No ETA unfortunately due to lack of free time 😔

nalexn avatar Jun 09 '22 15:06 nalexn

I've used this code below myself. Not necessary better. Agree that accessibility identifiers are very nice to use in the tests.

extension InspectableView {
    func find<T>(_ viewType: T.Type,
                 accessibilityIdentifier: String) throws -> InspectableView<T> where T: KnownViewType {
        return try find(viewType) { view in
            (try? view.accessibilityIdentifier()) == accessibilityIdentifier
        }
    }
    
    func findButton(_ accessibilityIdentifier: String) throws -> InspectableView<ViewType.Button> {
        try find(ViewType.Button.self, accessibilityIdentifier: accessibilityIdentifier)
    }
}

martinwennerberg avatar Sep 19 '22 16:09 martinwennerberg