Code

Opened 10 months ago

Closed 2 weeks ago

#21158 closed Cleanup/optimization (wontfix)

Set an implicit wait for selenium.

Reported by: apollo13 Owned by: bsilverberg
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: timo Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Selenium often tries to access elements before the page is loaded; it offers an implicitly_wait method which we could utilize and it might fix a few of our timing issues; I'd suggest a relatively high timeout of 10 seconds to cover our bases. See https://github.com/SeleniumHQ/selenium/blob/master/py/selenium/webdriver/remote/webdriver.py#L626 for this method.

Attachments (0)

Change History (9)

comment:1 Changed 9 months ago by timo

  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 months ago by bsilverberg

  • Owner changed from nobody to bsilverberg
  • Status changed from new to assigned

comment:3 Changed 3 months ago by bsilverberg

I have opened pull request https://github.com/django/django/pull/2543 for this. This is my first django pull request so any advice/feedback would be appreciated.

comment:4 Changed 3 months ago by bsilverberg

  • Has patch set

comment:5 Changed 3 months ago by jmbowman

I'd advise a little caution on this approach. I've worked on projects which have assertions that just want to verify that an element is not on the page. If every such assertion takes 10 seconds of redundantly verifying that the element is still not on the page, that can make the tests take a lot longer to run. It's a pain to correctly do it consistently, but I've had better results fixing tests with implicit timing dependencies to wait explicitly when appropriate.

comment:6 Changed 3 months ago by bsilverberg

Thanks jmbowman. That certainly can be an issue. I ran the current test suite and it doesn't seem to suffer from this issue. In the suites that we use at work we do have a default implicit wait set up, and then we use helper methods when we want to check that an element is not present which switch off the implicit wait.

I think the approach used here is appropriate for the current test suite, but it is something to be careful about in the future, if this approach is adopted.

comment:7 Changed 2 weeks ago by timo

  • Cc timo added
  • Patch needs improvement set

I left an idea for improvement on the pull request.

@apollo13, any thoughts on the comments in the thread? Was the idea to commit this and see if it helps with the random failures on Jenkins?

comment:8 Changed 2 weeks ago by apollo13

We currently don't have any random failures (or at least not really often), so I think we can close this for now.

comment:9 Changed 2 weeks ago by timo

  • Resolution set to wontfix
  • Status changed from assigned to closed

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.