fyne-io / fyne

Cross platform GUI toolkit in Go inspired by Material Design
https://fyne.io/
Other
24.17k stars 1.35k forks source link

Testing: widget.List #2776

Open e1fueg0 opened 2 years ago

e1fueg0 commented 2 years ago

Is your feature request related to a problem? Please describe:

I create a list with the content as follows:

    list := widget.NewList(
        func() fyne.CanvasObject {
            return container.NewBorder(nil, nil, widget.NewCheck("", func(bool) {}),
                nil, canvas.NewText("", color.White))
        },
        func(id widget.ListItemID, item fyne.CanvasObject) {
            cnt := item.(*fyne.Container)
            text := cnt.Objects[0].(*canvas.Text)
            // some code
            check := cnt.Objects[1].(*widget.Check)
            check.OnChanged = func(b bool) {
            // some code
            }
            // some code

I am not sure, whether it is the best solution (if not, please, give me a hint), but it works: I can assign a text to the text widget, I can assign a OnChanged handler to the check widget. The problem is I can't test in an elegant way a click on the check widget, because I can't externally obtain a content for a list entry.

Is it possible to construct a solution with the existing API?

Nope. Actually, yes, I could store somewhere the contents generated by createItem, but it would not be elegant enough for me to wish to include that in my code =)

Describe the solution you'd like to see:

Maybe, a kinda GetEntryContent(int) fyne.CanvasObject, which would expose the content. Also, mb, EntryCount() int, which tells how many entries are created, and EntryToItem(int) int and/or ItemToEntry(int) int, which can map list logical items to physical entries and vise-versa. But to start GetEntryContent would be enough: at least I can do this:

cnt := list:GetEntryContent(0)
assert.NotNil(t, cnt)
cnt := item.(*fyne.Container)
check := cnt.Objects[1].(*widget.Check)
check.SetChecked(!check.Checked)
// further checks of what OnChanged should do
andydotxyz commented 2 years ago

This is an interesting point. I suppose the easiest way to go about this is to test list cells directly rather than once placed in the List. However I agree that we should provide a way to do so when packed into the UI. I guess we could have something in the test package that does clever things with the cache internally... like:

test.ListItemContent(widget.ListItemID)
stuartmscott commented 2 years ago

I think this boils down to 'what are you trying to test?' - do you need to test list's internals, or are you trying to test a UI element that you created behaves in a particular way?

Your test could just call the list callbacks to create an item, update it to show test data, and then assert expected behaviour;

item := list. CreateItem()
list.UpdateItem(5, item)
container := item.(*fyne.Container)
check := container.Objects[1].(*widget.Check)
check.SetChecked(!check.Checked)
// further checks of what OnChanged should do

You could go one step further and separate the list from the callbacks entirely;

    // some code
    list := widget.NewList(DataLength, CreateItem, UpdateItem)
    // some more code
}

func DataLength() int {
    return 4
}

func CreateItem() fyne.CanvasObject {
    return container.NewBorder(nil, nil, widget.NewCheck("", func(bool) {}), nil, canvas.NewText("", color.White))
}

func UpdateItem(id widget.ListItemID, item fyne.CanvasObject) {
    cnt := item.(*fyne.Container)
    text := cnt.Objects[0].(*canvas.Text)
    // some code
    check := cnt.Objects[1].(*widget.Check)
    check.OnChanged = func(b bool) {
    // some code
}

So now your tests don't even need to know about list;

func TestDataLength(t *testing.T) {
    l := DataLength()
    // assert things
}

func TestCreateItem(t *testing.T) {
    i := CreateItem()
    // assert things
}

func TestUpdateItem(t *testing.T) {
    i := CreateItem()
    UpdateItem(4, i)
    // assert things
}
e1fueg0 commented 2 years ago

@stuartmscott

I think this boils down to 'what are you trying to test?' - do you need to test list's internals, or are you trying to test a UI element that you created behaves in a particular way?

Usually I try to test the complete user workflows. As the practice shows us, when you test the parts only, missing the whole picture, the revenge is usually terrible =) That's why I'm talking about a possibility to test everything assembled in the whole.

stuartmscott commented 2 years ago

You could use test.TapAt to tap the UI at a given position like a user would, and then assert that the Check's OnChange fires and performs the right action.

e1fueg0 commented 2 years ago

Yes, if I have an object to pass to test.TapAt().

andydotxyz commented 2 years ago

Perhaps @stuartmscott meant test.TapCanvas which takes a position parameter as well?

e1fueg0 commented 2 years ago

@andydotxyz but how should I calculate the position to aim the target check box?

andydotxyz commented 2 years ago

You can use a.Driver().AbsolutePositionForObject(obj)

e1fueg0 commented 2 years ago

Yes, but you still need an object to pass to the method. And that's why I created the issue.

andydotxyz commented 2 years ago

I guess you would pass the list to get that position and you can add to that the position of check inside the cell template? Just guessing... tapping a position in canvas isn't really an exact science inside a Scroller due to many possible complications. It does sound like a helper to reach into an item of a list could be helpful :)

stuartmscott commented 2 years ago

Don't forget tree and table too

e1fueg0 commented 2 years ago

btw, it works, guys, thank you @andydotxyz @stuartmscott!

for me it worked as follows:

        listPos := suite.app.Driver().AbsolutePositionForObject(list)
        pos := listPos.Add(fyne.NewSize(1, 1))
        test.TapCanvas(w.Canvas(), pos)