Skip to content

Add suffix to menu items locator #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 22, 2021

Conversation

lugi0
Copy link
Contributor

@lugi0 lugi0 commented Dec 21, 2021

Signed-off-by: Luca Giorgi [email protected]

To address issue #45

@lugi0
Copy link
Contributor Author

lugi0 commented Dec 21, 2021

Not sure why https://github.com/robots-from-jupyter/robotframework-jupyterlibrary/blob/master/atest/lab/00_shell.robot#L11 seems to be failing. Click JupyterLab Menu hasn't been modified, and Click JupyterLab Menu Item with the new locator should be able to find the item About JupyterLab.

@bollwyvl do you happen to have access to the screenshots of the test run or more in depth logs?

@bollwyvl
Copy link
Collaborator

do you happen to have access to the screenshots

If you are logged in, you can pop up to the "summary" of the run, and scroll ALL the way down. All the artifacts should be visible there.

Having a bit of a fire drill on some other stuff, but can hopefully take a look at this later today.

@lugi0
Copy link
Contributor Author

lugi0 commented Dec 21, 2021

6-selenium-screenshot-1

Somehow the test fails when trying to click the menu item

14:56:58.820 | INFO | Clicking element '//div[contains(@class, 'p-Menu-itemLabel')][text() = 'About JupyterLab']/..[not(contains(@class,'p-mod-disabled'))]'. |  

14:56:58.884 | FAIL | ElementNotInteractableException: Message: Element <li class="p-Menu-item p-mod-active"> could not be scrolled into view

@bollwyvl What do you make of this?
I believe <li class="p-Menu-item p-mod-active"> is the list item containing the element we are trying to click on, and from the screenshot it's clearly in the view. Any idea what might be the cause of the failure? Should I add a small sleep after clicking on the Menu name in the navbar?

@bollwyvl
Copy link
Collaborator

what might be the cause of the failure

I would i need to get into the browser and take a look at those selectors with some representative DOM... sometimes the parent selectors don't always do what one would expect. It can also be a timing thing: we might need to first wait until that item is visible and then click it.

@lugi0
Copy link
Contributor Author

lugi0 commented Dec 22, 2021

I'm stumped, I cannot reproduce the error locally because RobotFramework complains about //div[contains(@class, 'p-Menu-itemLabel')][text() = '${label}']/..[not(contains(@class,'p-mod-disabled'))] not being a valid xpath (for any ${label}), even though the xpath works via dev tools.

I could work around the issue by implementing a check on the label, and in the case of Log Out hardcoding a couple of locators for both scenarios (jupyterhub/single user). What do you think?

@lugi0
Copy link
Contributor Author

lugi0 commented Dec 22, 2021

As an update, my local issue with the xpath is due to Chrome, while the xpath is valid in Firefox it does not return any elements when using Chrome.

@bollwyvl bollwyvl closed this Dec 22, 2021
@bollwyvl bollwyvl reopened this Dec 22, 2021
@bollwyvl
Copy link
Collaborator

I went ahead and refreshed all the CI etc on #49, and am now taking a look at this locally...

@bollwyvl
Copy link
Collaborator

Huzzah, looking good here. I'll merge this as-is and we can follow-up later, if need be. No promises due to the holidays, but I'll go ahead and start up an 0.4.0 release checklist so we can start dialing in what needs doing.

@bollwyvl bollwyvl merged commit 3ef47f7 into robots-from-jupyter:master Dec 22, 2021
@bollwyvl bollwyvl mentioned this pull request Dec 22, 2021
22 tasks
@lugi0
Copy link
Contributor Author

lugi0 commented Dec 23, 2021

Great, thanks for going ahead and fixing it up!
For posterity, what we could've done to make the XPath valid in Chrome as well was to use either:
//div[contains(@class, 'p-Menu-itemLabel')][text() = 'Log Out']/parent::*[not(contains(@class,'p-mod-hidden'))]
or
//div[contains(@class, 'p-Menu-itemLabel')][text() = 'Log Out']/../self::*[not(contains(@class,'p-mod-hidden'))]

Essentially Chrome doesn't like when you apply attribute selectors to the shorthand of the parent, so we need to substitute /.. for either /parent::* or /../self::*

@bollwyvl
Copy link
Collaborator

p-mod-hidden

Eek! Merged with p-mod-disabled, not p-mod-hidden, on the strength of it not breaking the existing tests... did it need that? Both? I guess if you get a chance to try out one of the 0.4.0 distributions, and what's already in is wrong, let me know.

Chrome doesn't like when

Yeah, I've forgotten more than I'd like to admit ever having known about XPath and XSLT. It's definitely a lowest-common-denominator thing, where possible, before one even starts considering performance, which can be a very real concern. Luckily, in testing, it usually doesn't matter, though... so it's really whatever's shortest/most reliable.

@lugi0
Copy link
Contributor Author

lugi0 commented Dec 23, 2021

The problem with p-mod-disabled is that there are elements which might not be hidden but only disabled in certain scenarios (e.g. under the File menu most submenus are disabled if you have no notebook open).
Keywords might not outright fail but "clicking" the item will not produce any result, which might be even trickier to spot and debug (not sure if Selenium's Click Element checks that the element is actually clickable beforehand).

Sorry, should've spotted it earlier! As soon as I have some time I'll send a new PR fixing it.

@lugi0
Copy link
Contributor Author

lugi0 commented Dec 23, 2021

Opened #52 to update the locator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants