Code review comment for lp:~tpeeters/ubuntu-ui-toolkit/ap-toolbar-open

Revision history for this message
Tim Peeters (tpeeters) wrote :

> 95 + # ensure the toolbar is open
> 96 + self.open()
>
> I find it weird that the click_button method opens the toolbar if it's not
> open. I think I would prefer to raise an exception if when called click_button
> the toolbar is not opened.

You cannot really know whether the toolbar is open when clicking.
When you call toolbar_open(), the toolbar opens, but things can happen between that and the actual click on the button so that the toolbar closes again. For example, a timeout can occur. If the toolbar was opened initially, it will close after 5s. But in a test it is possible that only after 4.5s toolbar.open() is called (which does nothing because it was open already), and then it takes 1s to move the cursor to the button to click it, but in the meantime the toolbar closed already.

I think it is a matter of choice whether we want click_button() to open the toolbar for you, or if we want always toolbar_open() to be called before click_button(). Note that you would *always* call toolbar_open() (or toolbar.open()) before click_button, so why not integrate it in that function?

« Back to merge proposal