EarlGrey
EarlGrey copied to clipboard
google
Refactor: Reimplement 6 matchers and remove dependency on OCHamcrest
Refactoring. It seems that EarlGrey doesn't really need OCHamcrest framework and the dependency could be removed after reimplementing only 6 matchers.
- Reimplement 6 matchers
- Remove OCHamcrest-related code
Is this something you want to do? I realize some of these changes may not be compatible with the internal version of EarlGrey. If you want to accept some parts of this PR, but not everything, I can undo whatever changes you don't want to merge.
Positive impact:
- Once fishhook is removed, EarlGrey has 0 dependencies on non-Apple code
- Net removal of 200+ lines of code
- Once fishhook is removed, no need to run Scripts/setup-earlgrey.sh (only required for OCMock for unit tests)
- No need to read OCHamcrest code to understand how 5 GREYMatchers work and what they do
- Mismatch description is now consistent with other GREYMatchers
Thanks for the corrections here!! While having an external dependency can be painful, OCHamcrest does have a very rich set of matchers that would simply be duplicated in our code if we remove the dependency.
OCHamcrest does have a very rich set of matchers that would simply be duplicated in our code if we remove the dependency.
It looks like we're only using 6 of them though?
It looks like we're only using 6 of them though?
That's only in our test project. Currently, with GREYHCMatcher, a user can use all of the matchers provided by OCHamcrest - https://github.com/hamcrest/OCHamcrest#predefined-matchers
Currently, with GREYHCMatcher, a user can use all of the matchers provided by OCHamcrest
Oh. I didn't realize that. Very cool.
@tirodkar Read the code in this PR. These 6 matchers are not in the test project. It's inside EarlGrey, and see just how much duplication there is when you rewrite them, and how much code you can remove.
Actually, EarlGrey users cannot use them. OCHamcrest is not exposed by EarlGrey, and neither is GREYHCMatcher header.
@tirodkar Read the code in this PR. These 6 matchers are not in the test project. It's inside EarlGrey, and see just how much duplication there is when you rewrite them, and how much code you can remove.
We added these particular matchers and bridged them in because we were using them internally in our tests. That's what I meant by test project :)
OCHamcrest is not exposed by EarlGrey, and neither is GREYHCMatcher header.
Ah, we could actually expose it. Thanks for bringing this to our attention.
I don't think exposing them is a good idea. We should remove it. That's why I made this PR. Anything we need is easily reimplemented, will be more consistent with other EarlGrey matchers and gives us the ultimate flexibility for fixing and changing things.
I think Espresso allows using arbitrary hamcrest matchers. If removing them prevents end users from matching with OCHamcrest then that doesn't seem great.
I don't see a lot of value in reimplementing all the OCHamcrest matchers.
@tirodkar Can we find out how many users will impacted by this change? If there's not a lot, I am in favor of removing this dependency.
