Opened 5 years ago

Closed 5 years ago

#30974 closed Bug (duplicate)

Selenium asserts should be wait statements

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

Description

I just took a deep dive into our selenium tests, because ... well... they are "not deterministic sometimes" ;)

I realized that we use a lot of assertions in our tests, where I believe we should be using wait statements. Mainly because selenium tests are asynchronous and don't really when an assertion is true. It seems more reasonable to be, to simply say, what we expect and give it a reasonable amount of time before we fail.

So instead of:

self.assertEqual(
    self.selenium.switch_to.active_element,
    self.selenium.find_element_by_id('id_name')
)

We do:

self.wait_until(
    lambda selenium:
        selenium.switch_to.active_element == selenium.find_element_by_id('id_name')
)

I know this is not a foreign concept if you have been doing a lot of Selenium testing. Still, there are many cases where we do it "wrong" and the test suite fails "sometimes".

I believe the main difference is that in one case you raise an AssertionError in the other a TimeoutError. However, we could cast those to have the tests fail not error.

Change History (5)

comment:1 by Johannes Maron, 5 years ago

Haha on my first patch push a test failed like this:

Traceback (most recent call last):
  File "/home/jenkins/workspace/pull-requests-selenium/database/postgres/label/bionic-pr/python/python3.7/tests/admin_views/test_autocomplete_view.py", line 256, in test_select_multiple
    self.assertEqual(len(select.all_selected_options), 2)
AssertionError: 1 != 2

I believe there might be a lot of places that would need to use wait over assert ;)

comment:2 by Johannes Maron, 5 years ago

Patch needs improvement: set

comment:3 by Carlton Gibson, 5 years ago

Triage Stage: UnreviewedAccepted

Yep, thanks again. Very happy to review these :) (If we could get selenium running reliably on more than just Chrome that would be a win...)

comment:4 by Johannes Maron, 5 years ago

Owner: changed from nobody to Johannes Maron
Patch needs improvement: unset

comment:5 by Carlton Gibson, 5 years ago

Resolution: duplicate
Status: assignedclosed

So let's call this a duplicate of #29892, where this is being addressed.

Note: See TracTickets for help on using tickets.
Back to Top