Selenium asserts should be wait statements

[Johannes Maron]

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.

[Johannes Maron]

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 ;)

[Carlton Gibson]

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...)

[Carlton Gibson]

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