Update selenium tests #3412
Update selenium tests #3412
Conversation
Bug found during pair programming with @mpacer
Originally written by @takluyver while pair programming
| current_items = get_list_items(authenticated_browser) | ||
| current_items_links = [item["link"] for item in current_items] | ||
| stored_items = visited_dict.pop(authenticated_browser.current_url) | ||
| stored_items_links = [item["link"] for item in stored_items] |
There was a problem hiding this comment.
Neat, I like the comparison of what's here now vs what was here before.
| items = get_list_items(authenticated_browser) | ||
| visited_dict[authenticated_browser.current_url] = items | ||
| print(authenticated_browser.current_url, len(items)) | ||
| if len(items)>1: |
There was a problem hiding this comment.
Why does it not work with 1 item? Does it catch the link to go up a directory?
I think it would be good to filter out which items are directories, so that this doesn't accidentally load a notebook or another file.
There was a problem hiding this comment.
For the first, I was using that to filter out .. but I see now that that actually introduces weird behaviour at the root.
For the second, I gather that I can distinguish that based on the link target being on the /tree vs. /notebook or /file endpoints?
There was a problem hiding this comment.
Oops! i forgot about this simple fix and have been focused on getting the notebook navigation stuff working on the markdown stuff. I'll fix this quickly
| raise("You currently ") | ||
| wait_for_selector(browser, '.item_link') | ||
| items = get_list_items(browser) | ||
| return [i for i in items if 'tree' in i['link'] and i['label'] != '..'] |
There was a problem hiding this comment.
This will go wrong for any file that has tree in the name. Maybe that's unlikely, but I think it's worth doing it a bit more thoroughly.
There was a problem hiding this comment.
ok addressed by using an explicit check for whether urls point to something in the tree (with url_in_tree)
| """ | ||
| try: | ||
| assert 'tree' in browser.current_url | ||
| except PageError: |
There was a problem hiding this comment.
This is a convoluted way of writing if 'tree' not in browser.current_url: , even if it was catching the right error. Let's stick to the simple if statement rather than using assert to achieve the same thing.
|
🎉 |
This PR comes from the same pair programming session with @takluyver that #3411 came from.
This also depends upon #3411.