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

add convenience finding methods and XCTestCase to test by accessibility #168

Merged
merged 3 commits into from
Sep 10, 2022

Conversation

FrauR
Copy link
Contributor

@FrauR FrauR commented Mar 23, 2022

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)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 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.

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

@FrauR
Copy link
Contributor Author

FrauR commented Mar 23, 2022

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.

@nalexn
Copy link
Owner

nalexn commented Mar 24, 2022

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!

@ekscrypto
Copy link

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!

@FrauR
Copy link
Contributor Author

FrauR commented May 10, 2022

@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 FrauR marked this pull request as ready for review May 10, 2022 19:24
@FrauR
Copy link
Contributor Author

FrauR commented Jun 9, 2022

@nalexn can we merge this?

@nalexn
Copy link
Owner

nalexn commented Jun 9, 2022

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 nalexn changed the base branch from master to 0.9.2 June 9, 2022 15:31
nalexn added a commit that referenced this pull request Sep 10, 2022
@nalexn nalexn merged commit 6b8989a into nalexn:0.9.2 Sep 10, 2022
@martinwennerberg
Copy link

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)
    }
}

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

Successfully merging this pull request may close these issues.

None yet

4 participants