search

mvysny / karibu-testing

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

ConfirmDialog still there #102

Closed simasch closed 2 years ago

simasch commented 2 years ago

I have a test that worked with beta3 but now with 23.0.0 it fails

        GridKt._getCellComponent(categoryEventsGrid, 0, "edit-column").getChildren()
            .filter(component -> component instanceof Button).findFirst().map(component -> (Button) component)
            .ifPresent(Button::click);

        ConfirmDialog confirmDialog = _get(ConfirmDialog.class);
        assertThat(confirmDialog.isOpened()).isTrue();
        _fireConfirm(confirmDialog);

        // Check if event was removed
        assertThat(GridKt._size(categoryEventsGrid)).isZero();

        // Remove category
        GridKt._getCellComponent(categoriesGrid, 0, "edit-column").getChildren()
            .filter(component -> component instanceof Button).findFirst().map(component -> (Button) component)
            .ifPresent(Button::click);

        confirmDialog = _get(ConfirmDialog.class);

On the last line now I get 

java.lang.AssertionError: /series/1: Too many visible ConfirmDialogs (2) in UI[] matching ConfirmDialog: [ConfirmDialog[], ConfirmDialog[]]. Component tree:
└── UI[]
    ├── MainLayout[]
    │   ├── Header[@class='bg-base border-b border-contrast-10 box-border flex h-xl items-center w-full', @slot='navbar touch-optimized']
    │   │   ├── DrawerToggle[@aria-label='Menu toggle', @class='text-secondary', @theme='contrast']
    │   │   ├── H1[#view-title, text='Series', @class='m-0 text-l', @style='width:300px']
    │   │   └── HorizontalLayout[@style='width:100%;justify-content:flex-end;align-items:center;padding-right:20px', @theme='spacing']
    │   │       ├── Anchor[text='About', href='https://github.com/72services/jtaf4', @target='_blank']
    │   │       ├── Div[text='@project.version@']
    │   │       ├── RouterLink[INVIS, text='Register']
    │   │       ├── Button[INVIS, caption='Login']
    │   │       └── Button[#logout, caption='Logout (simon@martinelli.ch)']
    │   ├── Section[@class='flex flex-col items-stretch max-h-full min-h-full', @slot='drawer']
    │   │   ├── Image[@alt='JTAF', @src='icons/logo.png']
    │   │   ├── Nav[@aria-labelledby='views', @class='border-b border-contrast-10 flex-grow overflow-auto']
    │   │   │   ├── RouterLink[@class='flex mx-s p-s relative text-secondary']
    │   │   │   │   ├── Span[@class='me-s text-l la la-globe']
    │   │   │   │   └── Span[text='Dashboard', @class='font-medium text-s']
    │   │   │   ├── RouterLink[@class='flex mx-s p-s relative text-secondary']
    │   │   │   │   ├── Span[@class='me-s text-l la la-file']
    │   │   │   │   └── Span[text='My Organizations', @class='font-medium text-s']
    │   │   │   ├── RouterLink[#series-list-link, text='CIS', @class='flex mx-s p-s relative text-secondary']
    │   │   │   ├── RouterLink[@class='flex mx-s p-s relative text-secondary']
    │   │   │   │   ├── Span[@class='me-s text-l la la-file']
    │   │   │   │   └── Span[text='Events', @class='font-medium text-s']
    │   │   │   ├── RouterLink[@class='flex mx-s p-s relative text-secondary']
    │   │   │   │   ├── Span[@class='me-s text-l la la-file']
    │   │   │   │   └── Span[text='Clubs', @class='font-medium text-s']
    │   │   │   └── RouterLink[@class='flex mx-s p-s relative text-secondary']
    │   │   │       ├── Span[@class='me-s text-l la la-file']
    │   │   │       └── Span[text='Athletes', @class='font-medium text-s']
    │   │   └── Footer[@class='flex my-s px-m py-xs']
    │   │       └── Anchor[href='https://72.services', @style='width:300px;font-size:small', @target='_blank']
    │   │           └── Html[<p style="color:var(--lumo-primary-color)">Free and<br>Open Source<br>by 72© Services LLC</p>, @style='color:var(--lumo-primary-color)']
    │   └── SeriesView[@style='width:100%;height:100%', @theme='padding spacing']
    │       ├── FormLayout[]
    │       │   ├── TextField[label='Name', value='CIS 2018']
    │       │   └── Upload[#logo-upload, @target='VAADIN/dynamic/resource/1/197a9c35-cc3b-4a27-92e8-6ae9a4a78826/upload']
    │       │       ├── Button[caption='Upload logo...', @slot='add-button', @theme='primary']
    │       │       └── Span[text='Drop logo here', @slot='drop-label']
    │       ├── HorizontalLayout[@theme='spacing']
    │       │   ├── Checkbox[label='Hidden', value='false']
    │       │   └── Checkbox[label='Locked', value='false']
    │       ├── Button[#save-series, caption='Save', @theme='primary']
    │       ├── Tabs[@style='width:100%']
    │       │   ├── Tab[label='Competitions', Tab{Competitions}]
    │       │   ├── Tab[label='Categories', Tab{Categories}]
    │       │   └── Tab[label='Athletes', Tab{Athletes}]
    │       └── Div[@style='width:100%;height:100%']
    │           ├── Grid[#competitions-grid, INVIS, dataprovider='com.vaadin.flow.data.provider.ListDataProvider@6311898c', @style='height:100%']
    │           │   ├── Button[#add-button, caption='Add']
    │           │   ├── Column[header='Name', key='name']
    │           │   ├── Column[header='Date', key='competition_date']
    │           │   ├── Column[]
    │           │   └── Column[key='edit-column']
    │           ├── Grid[#categories-grid, dataprovider='com.vaadin.flow.data.provider.ListDataProvider@7bdbf06f', @style='height:100%']
    │           │   ├── Button[#add-button, caption='Add']
    │           │   ├── Column[header='Abbreviation', key='abbreviation']
    │           │   ├── Column[header='Name', key='name']
    │           │   ├── Column[header='Year from', key='year_from']
    │           │   ├── Column[header='Year to', key='year_to']
    │           │   ├── Column[]
    │           │   └── Column[key='edit-column']
    │           └── Grid[#athletes-grid, INVIS, dataprovider='com.vaadin.flow.data.provider.ListDataProvider@1c1bc904', @style='height:100%']
    │               ├── Button[#assign-athlete, caption='Assign Athlete']
    │               ├── Column[header='Last Name', key='last_name']
    │               ├── Column[header='First Name', key='first_name']
    │               ├── Column[header='Gender', key='gender']
    │               ├── Column[header='Year', key='year_of_birth']
    │               ├── Column[header='Club']
    │               └── Column[key='remove-column']
    └── CategoryDialog[@aria-labelledby='dialog-title', @theme='jtaf-dialog']
        ├── Header[@theme='light']
        │   ├── H2[text='Category', @class='dialog-title']
        │   ├── Button[#toggle, icon='vaadin:expand-square', @theme='icon']
        │   │   └── Icon[@icon='vaadin:expand-square', @slot='prefix']
        │   └── Button[icon='vaadin:close-small', @theme='icon']
        │       └── Icon[@icon='vaadin:close-small', @slot='prefix']
        ├── Div[@class='dialog-content']
        │   ├── FormLayout[]
        │   │   ├── TextField[label='Abbreviation', value='1']
        │   │   ├── TextField[label='Name', value='Test']
        │   │   ├── Select[label='Gender', value='M', dataprovider='com.vaadin.flow.data.provider.ListDataProvider@1aae50f9']
        │   │   ├── TextField[label='Year from', value='1999']
        │   │   └── TextField[label='Year to', value='2000']
        │   └── HorizontalLayout[@style='padding-top:20px', @theme='spacing']
        │       ├── Button[#edit-save, caption='Save', @theme='primary']
        │       └── Button[caption='Cancel']
        ├── Grid[#category-events-grid, dataprovider='com.vaadin.flow.data.provider.ListDataProvider@2b53d6fc']
        │   ├── Button[#add-event, caption='Add Event']
        │   ├── Column[header='Abbreviation']
        │   ├── Column[header='Name']
        │   ├── Column[header='Gender']
        │   ├── Column[header='Event Type']
        │   ├── Column[header='A']
        │   ├── Column[header='B']
        │   ├── Column[header='C']
        │   ├── Column[header='Position']
        │   └── Column[key='edit-column']
        ├── SearchEventDialog[#search-event-dialog, @aria-labelledby='dialog-title', @theme='fullscreen jtaf-dialog']
        │   ├── Header[@theme='light']
        │   │   ├── H2[text='Events', @class='dialog-title']
        │   │   ├── Button[#search-event-dialog-toggle, icon='vaadin:compress-square', @theme='icon']
        │   │   │   └── Icon[@icon='vaadin:compress-square', @slot='prefix']
        │   │   └── Button[icon='vaadin:close-small', @theme='icon']
        │   │       └── Icon[@icon='vaadin:close-small', @slot='prefix']
        │   ├── Div[@class='dialog-content']
        │   │   ├── TextField[#event-filter, label='Filter', value='']
        │   │   └── Grid[#events-grid, dataprovider='com.vaadin.flow.data.provider.DataProvider$2@3b30409', @style='height:calc(100% - 300px']
        │   │       ├── Column[header='Abbreviation', key='abbreviation']
        │   │       ├── Column[header='Name', key='name']
        │   │       ├── Column[header='Gender', key='gender']
        │   │       ├── Column[header='Event Type', key='event_type']
        │   │       ├── Column[header='A']
        │   │       ├── Column[header='B']
        │   │       ├── Column[header='C']
        │   │       └── Column[key='assign-column']
        │   └── Notification[]
        ├── ConfirmDialog[]
        └── ConfirmDialog[]

So this looks like the dialog is not closed with _fireConfirm()

mvysny commented 2 years ago

Hmm sounds like #34 . However, Karibu-Testing contains a test for that and it passes for Vaadin 23.0.0:

    test("confirm") {
        var confirmCalled = false
        ConfirmDialog("Foo", "Bar", "Yes") { confirmCalled = true }.open()
        _get<ConfirmDialog>()._fireConfirm()
        expect(true) { confirmCalled }
        _expectNone<ConfirmDialog>()  // make sure the ConfirmDialog is closed: https://github.com/mvysny/karibu-testing/issues/34
    }

I noticed something strange in your dump: the ConfirmDialog is nested within CategoryDialog. Usually dialogs should be placed as direct children straight under UI.

Do you have any special code, which would nest ConfirmDialog into CategoryDialog? If not, could it be that CategoryDialog is modal, causing any nested dialogs to be added as a children of ConfirmDialog rather than the UI (something akin the server-side modality curtain)? If that's true, then that's most probably the origin of the issue - Karibu-Testing only auto-cleans up dialogs nested directly under the UI...

simasch commented 2 years ago

I noticed something strange in your dump: the ConfirmDialog is nested within CategoryDialog. Usually dialogs should be placed as direct children straight under UI.

Yes that's very strange and I don't understand why this worked until beta3 and with newer versions this is broken.

mvysny commented 2 years ago

I think that a server-side "modality curtain" has been developed in beta4+, which directly affects dialogs. Perhaps this kind of dialog nesting is an after effect. Let me investigate.

mvysny commented 2 years ago

It was the case as I suspected. Not only for ConfirmDialog, but for the regular Dialog as well. Starting from Vaadin 23.0.0.beta4, when a modal dialog is opened while another modal dialog is active, the second dialog is nested inside of the first one, with respect to the server-side component hierarchy. I've changed Karibu-Testing to do a full sweep of all components attached to the current UI, to find and close such dialogs.

The fix will be present in Karibu-Testing 1.3.12

nubitic-admin commented 1 year ago

Well, it happens that if this first dialog is closed but have nested dialogs that still remain opened, this fella <cleanupDialogs()> ends up removing the first dialog along all nested dialogs within it, even if those were still opened.

I'd came out with this code to "recover" those still opened nested dialogs previous to the cleanupDialogs() call. It works well to my purposes, I share here in case someone finds it useful:

        List<Dialog> dialogs = DialogUtilsKt.getAllDialogs();
        if (dialogs != null) {
            List<Dialog> nestedDialogs = new ArrayList<>();
            dialogs.forEach(dialog -> DepthFirstTreeIteratorKt.walk(dialog).forEach(posibleNestedDialog -> {
                if (posibleNestedDialog != dialog && 
                    posibleNestedDialog instanceof Dialog && 
                    ((Dialog) posibleNestedDialog).isOpened()) {
                    nestedDialogs.add((Dialog) posibleNestedDialog);
                }
            }));
            dialogs.addAll(nestedDialogs);
        }
        return dialogs.stream().filter(Dialog::isOpened).collect(Collectors.toList());