-
Notifications
You must be signed in to change notification settings - Fork 172
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
(feat) ListViewMatchers: Add hasPlaceholder(Node placeholder). #252
Conversation
b73bb5a
to
bc1823c
Compare
After doing some exploration/investigate work on this PR, I came up with a general question. Does adding these types of methods make sense? Basically it is the difference between:
and
I am starting to think that the second way is much better (which is counter to the direction of this PR) and would lead to less maintenance over time, as the first method could get unwieldy (as I think you can see it already is started to become with the Tangentially, it would be really nice if there was a way to make assertions more "type-smart" in the sense of e.g. AssertJ, where once can type: List<String> myList = new ArrayList<>();
assertThat(myList). // <----- invoke completion here and it shows you type specific assertions based on the type of the object inside the The two concepts that I have talked about are not directly linked together, but I wanted to get my preliminary thoughts/musings out so that you could (hopefully) guide me in the best direction to take as we go forward. In summary, what do you think about the idea behind this PR and the other one?? Are these legitimate additions? Or should we instead only add methods to the XMatchers class (so in this case, since the placeholder property is a Node, it is already covered by NodeMatchers)? Thanks! |
FxAssert.verifyThat(lookup("#someListView"),
ListViewMatchers.hasPlaceholder(new Label("List is empty!"))); For me the "idiomatic way" is: FxAssert.verifyThat("#someListView",
ListViewMatchers.hasPlaceholder("List is empty!"));
The second way with
This is a good point. It would be great, if we find a reliable way to minimize the maintenance of matcher fixtures.
I thought about types for
Both PRs improve the cohesion and make the tests more readable, I think. |
|
The problem is that this only works for placeholders that are Labeled. Consider, for example, a placeholder that is an ImageView.
Would you like me to change the PR to to use ControlMatchers? |
The visible variant is used when one also wants to assert that the placeholder is currently visible (e.g. that the ListView is empty).
Ideal usage: