first() is fundamentally broken

[mvysny]

The expression $(ButtonWrapper.class).first() should fundamentally not be used in a test: it promotes creation of flaky tests. If there are multiple Buttons on the web page, TestBench selects an arbitrary one and then the test may (or may not) fail later on, after the button is clicked and the outcome is something else than the test intended.

Instead, there should be a get() function which asserts that there's exactly one Button, failing if there are multiple matching Buttons. That's exactly what Playwright is doing: at https://playwright.dev/java/docs/writing-tests the page.getByRole(xyz) will fail if there are multiple matching elements.

first() should be deprecated and replaced by get() (which selects one), or get(int) which returns a list of elements and asserts that it's of that particular size.

[joelpop]

@mvysny Is this resolved by the new single() method added in PR #1774? I don't see the need to deprecate first() as there may still be some use cases for it, but I agree that use of single() is preferable when there should only be one possible.

[mvysny]

Yup, single() is great. I'd still deprecate first() - if you have more than one element matching, you should really refine your search to return exactly what you need. If there are multiple matching, that's usually the case for repeated elements and there you would use iterator to iterate over those. I've created Karibu Testing and have written tests for 6 years, and I never needed first() once; I've only seen it to be used for evil :-D

[joelpop]

I have used first() on occasion when there was not any other good way to uniquely identify a component with the selectors provided other than by position. What do you think of instead of deprecating first() (and last()) we instead put a strong Javadoc "warning" on first() discouraging its use and encouraging they instead use single() along with better selection criteria to make it unique?

[knoobie]

Deprecation for removal is the only warning developer understand 🙂

[joelpop]

However, I don't believe it is appropriate to deprecate a convenience method just because its use has been abused due to previous lack of an appropriate method. For instance, although Lists have a .get(int) method, that does not preclude them from having getFirst() and getLast() methods.

I would categorize the situation more as "Legacy use of first() is fundamentally wrong."

BTW, there is also a companion last() method that would also need to be deprecated if first() is deprecated.

[joelpop]

Proposed change:

    /**
     * Executes the search and returns the first result.
     * <p>
     * Usage note: This selector is a convenience method for
     * {@code get(0)}. It is intended to be used when multiple
     * elements satisfy your selection criteria, and you are
     * explicitly targeting the first one of them.
     * <p>
     * If you are expecting only one element to satisfy your selection criteria,
     * use {@link #single()} instead of {@code first()}.
     *
     * @return The element of the type specified in the constructor
     * @throws NoSuchElementException
     *             if no element is found
     */
    public T first() {
        return get(0);
    }