widgetastic.core icon indicating copy to clipboard operation
widgetastic.core copied to clipboard

Get wrong browser object for non top view in hierarchy

Open oshtaier opened this issue 6 years ago • 7 comments

What we do:

class AirgunBrowser(Browser):
    def wait_for_element(...):
        return super(AirgunBrowser, self).wait_for_element(...)

Then we do:

class MyView(View):
    def is_displayed(self):
        return self.browser.wait_for_element(...)

class MyOtherView(View):
    class MyTab(Tab):
        view = MyView()

In result when we call view we have wrong browser object type in its is_displayed method: self.browser == BrowserParentWrapper not AirgunBrowser that will get us to super(type, obj): obj must be an instance or subtype of type exception in wait_for_element method

oshtaier avatar Dec 18 '18 14:12 oshtaier

So basically we want to extend wait_for_element with one extra argument. To avoid copy pasting entire method body we just call super() and extend result with our stuff.

For top-level views calls to wait_for_element() via self.browser.wait_for_element() work ok, but for nested structure like the one @oshtaier provided - it fails on super() call (obj must be an instance or subtype of type), as self.browser for all nested views is BrowserParentWrapper, not the Browser itself.

Using it through self.root_browser.wait_for_element() works, but that doesn't sound like a right option to use root browser for every overwritten method to me. Is that expected behavior and we aren't supposed to extend any browser's method or that's an issue with BrowserParentWrapper proxy implementation?

abalakh avatar Dec 18 '18 15:12 abalakh

https://github.com/RedHatQE/widgetastic.core/blob/f39a87f2fc18d8a88b5c25884c3ebedd594577fd/src/widgetastic/browser.py#L833-L853 handles the locator mapping, it seems its not yet correctly supporting subclassing and needs to be extended in some way

RonnyPfannschmidt avatar Dec 18 '18 15:12 RonnyPfannschmidt

this change may necessitate a structural shift for wrapping browsers and locator using objects to sort out, its possible we cant get around it before the new year

right now i don't have a idea for a solution

RonnyPfannschmidt avatar Dec 18 '18 16:12 RonnyPfannschmidt

please also show the use-case you want to implement, due the nature of the issue (structural) it may be easier in the short term to find a different way to implement the use-case simply to avoid having to sort out the underlying issue in a timely manner

RonnyPfannschmidt avatar Dec 18 '18 16:12 RonnyPfannschmidt

@RonnyPfannschmidt actually that was real use case.

I did it here: https://github.com/SatelliteQE/airgun/pull/263/files#diff-b5de464875aa71a973dee2e139a855c9R130

but teammates said proper comment that it will be necessary to fix all (>50) places where we have not top level view.

oshtaier avatar Dec 18 '18 16:12 oshtaier

@RonnyPfannschmidt Perhaps a custom super wrapper that "unwraps" the underlying browser from its wrapping would do. Instead of super(MyBrowser, self) one would call something like self.super(MyBrowser) and it would:

  1. Figure the unwrapped object
  2. call super(...) with it
  3. Rewrap it again with the current wrapper.

And then the use would be like with normal super.

mfalesni avatar Dec 21 '18 15:12 mfalesni

Actually not. The base Browser was designed as an immutable set of operations which other components rely on.

If specific things are needed, you can:

  • add new methods that implement specific things in your browser subclass
  • request a backwards-compatible change in WT browser method (add a parameter or whatever).
  • use an existing hook in plugin class
  • add a new hook into the Plugin class and add the hook calls in the particular browser methods, that way extensions altering the behaviour of the browser can be done without harming vanilla browser's behaviour.

mfalesni avatar Jan 03 '19 15:01 mfalesni