Fixed visibility assertions

[nickmi11er]

This PR relates to #294

Issue:
assertDisplayed(@idres viewId: Int, text: String)
assertDisplayed(@idres viewId: Int, @StringRes stringId: Int)
assertNotDisplayed(@idres viewId: Int, text: String)
assertNotDisplayed(@idres viewId: Int, @StringRes stringId: Int)

check only if there is a view in the hierarchy with provided text. But it does not check that view is really (not)displayed for user

[rocboronat]

I'm afraid this PR would change how Barista behaves right now, so it could break everyone's tests. Chaos! Destruction! 💣🔥💀

What would you think about creating a new method, let's say assertVisible that does this new check?

@Sloy @alorma any thoughts there? Please share! 👯‍♂

[glureau-betclic]

The method is in BaristaVisibilityAssertions, but doesn't handle visibility.

Also Displayed is pretty clear on what is expected, no one would think that a view displayed is not visible, that's weird.

The only reason to have another method name could be for retrocompatibility like @rocboronat suggested, and in this case we should deprecate the assertDisplayed and push developers to use assertVisible instead. (Even add a assertExists if it's needed for some people.)

[glureau-betclic]

@nickmi11er maybe add tests to confirm the broken behaviour.

[rocboronat]

Yes, I totally agree with you. The problem here is just a naming issue. Native Espresso names the current behavior "display". I mean, if we understand Barista as a list of shortcut recipes, displayed was a good name. That's why I would suggest creating a new method to avoid breaking everything.

But honestly, I know this is a mess. I'm wondering what's better for the current Barista users and I think that creating a new method to know if a View is actually "displayed" is the way to be. But, as visible in Android means that it is displayed in Espresso, it seem that the best way to handle it right now is to rename the current behavior to talk about visibility and accept the PR using the old displayed names.

Food for thought! And for the rest of Barista contributors... Guys, naming issue here! 📛

[Sloy]

After writing a long paragraph about how I don't understand the issue... I understood it 😂

Yes, you're right. The assertDisplayed(text) is not having the same behaviour as assertDisplayed(id). It feels like a bug to me. But @rocboronat is right, changing it might break existing tests out there.

I'm more declined towards "let's fix it and warn about it in the changelog".

[josinaldobarbosa]

I always thought it worked like that, I don't see why it would "break" something

[Sloy]

Hey @nickmi11er! I'm so sorry for not checking this sooner, it's been a long time. Since there has been no further discussion, I think the best choice is to merge your PR fixing the issues.

However, I'm missing some tests to make sure this doesn't break again (it should have been tested already but it wasn't). Let me add them myself and then I'll merge the PR :)