mvysny / karibu-testing

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

Find components inside virtual children #86

Closed hfazai closed 2 years ago

hfazai commented 2 years ago

Fixes #85

hfazai commented 2 years ago

Duplicated elements has appeared from virtual children. Let's see the tree for this example:

Before discovering virtual children: (ModuleListMenu is a MenuBar) image

After (the new element appeared is marked in red) ss

for menubar and grid, It seems that items are added to children and virtual children.

I did a change to keep only components discovered in virtual children. This way, some PrettyPrintTree tests are failing because the tree graph has changed. (in the example above we keep only the red component) If you agree with these changes, I will change the expected trees to get all tests passing.

mvysny commented 2 years ago

Gotcha, thanks! The problem is that when I try to comment out the component is MenuItemBase<*, *, *> -> { rule, the sub menu items are no longer discovered properly.

I propose to "revert" the behavior: exclude the virtual children lookup for MenuItemBase and for MenuBar and for Grid.

I've tested with this getAllChildren() implementation and it seems to be working properly:

    public fun getAllChildren(component: Component): List<Component> = when {
        component is Grid<*> -> {
            // don't attach the header/footer components as a child of the Column component:
            // that would make components in merged cells appear more than once.
            // see https://github.com/mvysny/karibu-testing/issues/52
            val headerComponents: List<Component> = component.headerRows
                    .flatMap { it.cells.map { it.component } }
                    .filterNotNull()
            val footerComponents: List<Component> = component.footerRows
                    .flatMap { it.cells.map { it.component } }
                    .filterNotNull()
            val editorComponents: List<Component> = component.columns
                    .mapNotNull { it.editorComponent }
            val children = component.children.toList()
            (headerComponents + footerComponents + editorComponents + children).distinct()
        }
        component is MenuItemBase<*, *, *> -> {
            // also include component.children: https://github.com/mvysny/karibu-testing/issues/76
            (component.children.toList() + component.subMenu.items).distinct()
        }
        component is MenuBar -> {
            // don't include virtual children since that would make the MenuItems appear two times.
            component.children.toList()
        }
        component is PolymerTemplate<*> -> {
            // don't include virtual children since those will include nested components.
            // however, those components are only they are only "shallow shells" of components constructed
            // server-side - almost none of their properties are transferred to the server-side.
            // Listing those components with null captions and other properties would only be confusing.
            // Therefore, let's leave the virtual children out for now.
            // See https://github.com/mvysny/karibu-testing/tree/master/karibu-testing-v10#polymer-templates--lit-templates
            component.children.toList()
        }
        component.javaClass.name == "com.vaadin.flow.component.grid.ColumnGroup" -> {
            // don't include virtual children since that would include the header/footer components
            // which would clash with Grid.Column later on
            component.children.toList()
        }
        component is Grid.Column<*> -> {
            // don't include virtual children since that would include the header/footer components
            // which would clash with Grid.Column later on
            component.children.toList()
        }
        // Also include virtual children.
        // Issue: https://github.com/mvysny/karibu-testing/issues/85
        else -> component._allChildren
    }

what do you think?

If you agree, can you please incorporate the code above into your PR? I'll then merge the code.

hfazai commented 2 years ago

Oh thanks! I think this is the best way to do it. I hesitated about the change I did because even components tree structure doesn't look correct. Now after excluding visrtual components everything looks OK.

I would like to add unit tests for EnhancedDialog. If want to merge this PR now, I'll do it in another PR.

mvysny commented 2 years ago

Please go ahead and add the tests in this PR, no problem

hfazai commented 2 years ago

Please go ahead and add the tests in this PR, no problem

Done :+1:

mvysny commented 2 years ago

I was planning to release next Karibu-Testing after Vaadin 21 is released, but if you need an earlier release just let me know :+1:

hfazai commented 2 years ago

I was planning to release next Karibu-Testing after Vaadin 21 is released, but if you need an earlier release just let me know

Ok, Thank you! For now I'm overriding getAllChildren() and adding needed code untill I upgrade to the next release.

I'm building also a module based on Karibu-Testing that provides hight-level functions to test applications made with Galite (https://github.com/kopiLeft/Galite/tree/master/galite-testing). As galite is a framework to build applications made of forms, pages, reports, charts, triggers and commands...etc Galite-testing provides a functions to simulate user actions like (Form.open(), Command.trigger(), login(),..etc). The framework still under construction :smile: but it is now in an advanced steps.

mvysny commented 2 years ago

Nice project, thanks for sharing :+1: I'll definitely take a look