Alternative Locator API that returns "testing" API instead of raw components

[mstahv]

The current API has an issue that its "queries" return the component instant. And most often the user interactions should be, instead of using the raw component API, simulated throught "testers". So essentially user of the library first queries the component and then does another special trick to convert that into "tester". As they also happen throught a same "gate", the code looks bit odd.

This is an experiment that takes a more inspiration from Playwright's API but keeps the well typed approach we currently have in the browserless-test. Check out the LocatorApiTest at this point and comment about the prototyped code end users would write after this change, the current implementation and API details probably have a lot to improve. The typed locators are here ATM written based on the current Testers.

[mstahv]

/format

[mcollovati]

Without digging too much in the code, overall the feature looks a great improvement for browserless testing.
A couple of points that can be addressed in the PoC, before a proper code review:

• The Locator only exposes a subset of ComponentQuery filter methods. We should perhaps expand it a bit (e.g. withAttribute(...)) or add a generic with(UnaryOperator<ComponentQuery<C>) to allow users more flexibility,
• The processor could be helpful also in end user projects that define their own Testers. However, as it is, it has a couple of problems: it seems to scan everything instead of being limited to the compilation unit, regenerating the GeneratedLocators interface in the application. I think this can be solved by limiting the generation scope to the compilation unit and also generating an entry point per application/jar, so basically one GeneratedLocators per run in different packages (package name can be given as a processor parameter). Or make it so that GeneratedLocators is created only for the browserless shared module. But the single generated locators could anyway be used.
• I'd rename getXXX methods to findXXX in the GeneratedLocators class; getChart() looks like you already have a chart component back, but this actually only starts the search process.
• The method skip algorithms should probably be refined a bit; for example, a tester might not implement clickable but provide custom overrides for click methods.
• Commercial components like Chart should not be added to the GeneratedLocators class, otherwise there will be compilation errors when the end project only uses Vaadin core components. This already happened with the TestWrappers interface that had to be split, moving Chart helpers in CommercialTesterWrappers
• We should consider adding locators’ stuff also to BrowserlessTest and the JUnit extension (can probably be done in a separated PR)

I'll go deeper into the processor implementation later on

[mstahv]

I'm not 100% sure if it is good to use the same verb for the component queries and now for the "locators", thus I went for the get verb. Also aligns to Playwright terminology. Changed anyways to find. Not a big issue if we end up deprecating the old component queries altogether (might make sense) and we can test how different find methods now feel in actual projects with snapshot build and revert if we end up regretting too much.

Other but the last poin tackled with "some" help from Kalle. Making this a non-draft as it looks so promising.

[mstahv]

/format

[mstahv]

/format

[mstahv]

/format

[StefanPenndorf]

Some thoughts from me:

The example requires a View as a starting point. If I want to develop a complex reusable component (e.g. a person form I want to use several times in my app or a shared component if I develop a framework for multiple applications) and I want to test this component independently from the view it will be finally integrated into, I need to provide a dummy view to navigate to. Is this intended as a final decision? If yes, this should be mentioned in the official Vaadin documentation, at least.

I like the idea of the PersonFormLocator and maybe this should be explored in more depth. This allows for more business language style tests (and even BDD tests)

window.find(DeliveryChooserLocator::new)
    .enterShippingAddress("MyName", "MyStreet","MyTown")
    .selectInvoiceAddressSameAsShippingAddress()
    .next();

that encapsulates the internals of locating individual components and fields.

Those locators might be a bit of boiler plate but they are useful to encapsulate internals of the view: In your example the test for the view needs to know the concrete id of the emailField of it's subcomponent PersonForm. In complex real world applications this will be different java files and maybe a deeper nesting of components. I don't like the idea that the test for the view knows the exact id of a component in a grand grand child of that view.

Even if the components are direct children to the view: The test and the production code are coupled via Strings (I refer to those ids). Does it make sense to propagate the use of constants like

@Route("locator-demo")
public class LocatorDemoView extends VerticalLayout {

    static final String SAVE_BUTTON_ID = "save";

}

that can be refered to in tests? This would make the coupling more explicit, I think. Would you agree or do you think differently?

Following this idea further something like

@Route("locator-demo")
public class LocatorDemoView extends VerticalLayout {

    final Button save = new Button("Save");

}

class LocatorApiTest {
   @Test
    void test() {
        window.navigate(LocatorDemoView.class).find(view -> view.save).click();
    }
}

came into my mind. But I'm not quite happy with this approach. It doesn't work for all usecases (dynamically added children etc. that can't be represented as fields in the view or it's children) and maybe couples test and implementation too much.

Thus using "lots of locators" seems the best compromise to me: There are no (child) component ids scattered in the tests and the locators provide a good layer of indirection.

Hope me random ideas and thoughts make some sense to you and might be of some help.

[mstahv]

Hi @StefanPenndorf, very good feedback! We need to work also with the docs to make things clearer.

The PersonFormLocator in the PR tests is indeed kind of a replica of "page object pattern", commonly used among Selenium/Playwright/TestBench, but adapted now for the browserless-test module user. That's the only way to keep larger tests maintainable, like having classes is on the UI code side on bigger projects.

I try to avoid adding ids. Same for Playwright tests. Usually there is some identifying feature of the component that can be used. In Playwright tests my most common selector is https://playwright.dev/java/docs/api/class-page#page-get-by-label. The counterpart in browserless-test is bit confusingly "withCaption". We should add a more specific withLabel as well for discoverability, #42, because I didn't find it 🤣.

I created #69 to help testing individual components 👍 Add your commnets there if you have some other insights.

In my own projects I tend to keep low barrier to create sub modules for components. Even if they would be only used in the app itself and in a single place, this often helps to create cleaner code. At least it feels more natural to create tests and it is then a tiny step to separate it to a standalone project if one things some other project (or community in general) could benefit of it. Here is one recent example: https://github.com/mstahv/lahtokaavio/tree/main The visual-lane-planner ther is probably never split out from here, but I have

• a test view for that component only (where it is easy to work with or manually test specific uses cases)
• browserless-test based tests
• Playwright based E2E tests

Then in the actual application module I can concentrate covering the basic use cases only.

[mcollovati]

window.navigate(LocatorDemoView.class).find(view -> view.save).click() cannot be achieved since the result of navigate is the view instance.
What we can do instead without changing the core API is

var view = window.navigate(LocatorDemoView.class)
window.use(view.save).click();

[mcollovati]

@mstahv inside(Locator) eagearly resolved the component. Is this the intended behavior, or should we lazily evaluate the locator?

[mstahv]

I haven't thought about that. Maybe it should be, but I guess it doesn't matter that much (or if we need to change it in upcoming version). The inside(Component) version is kind of eager anyway 🤷‍♂️ Your call.

[mcollovati]

Implemented inside(Locator) to lazyly compute parent component

[mstahv]

👍 Verified the latest changes against two external projects where I have been using these. All fine there.