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 , 5 years ago
comment:2 by , 5 years ago
Patch needs improvement: | set |
---|
comment:3 by , 5 years ago
Triage Stage: | Unreviewed → Accepted |
---|
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 , 5 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | unset |
comment:5 by , 5 years ago
Resolution: | → duplicate |
---|---|
Status: | assigned → closed |
So let's call this a duplicate of #29892, where this is being addressed.
Haha on my first patch push a test failed like this:
I believe there might be a lot of places that would need to use wait over assert ;)