mvysny / karibu-testing

Vaadin Server-Side Browserless Containerless Unit Testing
Apache License 2.0
105 stars 14 forks source link

All grid operations seem to return wrong data if DataCommunicator has pagingEnabled #99

Closed Artur- closed 2 years ago

Artur- commented 2 years ago

Tested by downloading a Flow app with a master detail view from https://start.vaadin.com/

The master detail view looks like this in the browser: image

But when doing

@SpringBootTest
public class GridTest {

    @Autowired
    private SamplePersonService samplePersonService;

    @Test
    public void gridData() {
        Routes routes = new Routes().autoDiscoverViews(Application.class.getPackageName());
        MockVaadin.setup(routes);

        MasterDetailView view = new MasterDetailView(samplePersonService);
        System.out.println(GridKt._dump(view.grid));

    }
}

the output is

--[First Name]-[Last Name]-[Email]-[Phone]-[Date Of Birth]-[Occupation]-[Important]--
0: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
1: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
2: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
3: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
4: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
5: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
6: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
7: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
8: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null
9: Eula, Lane, eula.lane@jigrormo.ye, (762) 526-5961, 1951-06-07, Insurance Clerk, null

Reproduction case: https://github.com/Artur-/start-flow-testing Remove the grid.getDataCommunicator().setPagingEnabled(false); in MasterDetailViewTest.java

simasch commented 2 years ago

I can reproduce the problem with your project but mine works fine: https://github.com/72services/jtaf4

Expect that I use jOOQ for the data access I can't find the difference.

mvysny commented 2 years ago

The problem is as follows:

  1. Karibu-testing calls DataCommunicator.fetchFromProvider(1, 1) which in turn calls the CallbackDataProvider.FetchCallback at MasterDetailView:92 and passes in a Query with offset 1 and limit 50 (since that's the default page size).
  2. The MasterDetailView.java:93 however doesn't use offset but uses query.getPage() instead, which returns 0. The query will therefore return first 50 items.
  3. Karibu-Testing then takes the 0th item and returns it.

Now I wonder where the problem lies:

  1. Is the culprit the MasterDetailView.java, incorrectly taking the page number and incorrectly fetching the data?
  2. Is it the fault of Vaadin's Query, providing an incorrect page? Maybe... If the DataCommunicator is paged (pagingEnabled indeed returns true) then maybe Vaadin should make sure to do some offset recalculation. Or the fetchFromProvider() function should require that offset is a multiple of the page size.
  3. Maybe it's the fault of Karibu-Testing and it should call DataCommunicator.fetchFromProvider() with the offset parameter rounded to the page start when DataCommunicator.pagingEnabled is true. The fetchFromProvider() function is private to Vaadin after all... However it would be good if its javadoc would mention this fact.

Setting the setPagingEnabled to false sets the page size to 1 which causes the page to be calculated correctly (it will be equal to the offset).

I think that Vaadin Grid is always passing in offset rounded to a page, that's why the bug wasn't noticed in Vaadin apps.

@Artur- any thoughts? I think there are two options:

  1. either fetchFromProvider() function will start requiring that offset is a multiple of the page size (when in paged mode), or
  2. It would expect any offset and then somehow hotfix the data returned by the Query.

I think the first option is the sanest one. In such case, Karibu-Testing can simply set the pagingEnabled to false, to set the page size to 1.