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

Just interact with visible things #51

Merged
merged 16 commits into from
May 10, 2017

Conversation

rocboronat
Copy link
Member

At #48, @despinola highlited an issue related with Recyclers inside ViewPagers. But #17 is related with the same issue. Actually, all BaristaActions* should only be applied to the visible item, to avoid issues with Fragments inside ViewPagers again and again.

This PR just adds the isDisplayed() matcher to all calls.

When this PR is applied, we will avoid having lots of MultipleMatchingExceptions when working with items inside ViewPagers.

  • Actually, scrollTo needs to work on invisible widgets, too :·)

BEWARE THE ALIEN! This PR depends on #50

@rocboronat rocboronat changed the title Just insteract with visible things Just interact with visible things May 9, 2017
Copy link
Member

@Sloy Sloy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me

import static android.support.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.core.AllOf.allOf;

public class DisplayedMatchers {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some javadocs to the class or methods might be helpful for users, to understand the magic of these matchers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wo... Actually, I think that the code is autoexplained. In addition, in this great example, the really nice method name also explains the implementation 😂

displayedWithId(int id) = allOf(withId(id), isDisplayed());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me change the order of the matchers, to make them more readable if possible :·D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-explaining code doesn't work for public APIs, IMHO ^^

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this code is not part of the public API, isn't it? It's code that we call from Barista. I know that someone can call it cos it's public, but... that's not what Barista aims to do.

In the other hand, the method does exactly what it explains. I mean, if I know how Espresso works, if I see a matcher called displayedWithId(@ResId int id), it sounds that it matches with displayed views that had the given ID.

Let me skip adding the Javadoc. If it's a issue, we will add in as soon as it adds value to Barista users.

@rocboronat
Copy link
Member Author

Espresso tests passed! 🎉

@rocboronat rocboronat merged commit 9cd61d0 into master May 10, 2017
@rocboronat rocboronat deleted the just-insteract-with-visible-things branch May 10, 2017 11:22
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.

2 participants