Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Increase selenium query_selector timeout#15396

Merged
bcyphers merged 1 commit intoEFForg:masterfrom
cschanaj:fix-selenum-test-options
May 17, 2018
Merged

Increase selenium query_selector timeout#15396
bcyphers merged 1 commit intoEFForg:masterfrom
cschanaj:fix-selenum-test-options

Conversation

@cschanaj
Copy link
Copy Markdown
Collaborator

No description provided.

@Bisaloo
Copy link
Copy Markdown
Collaborator

Bisaloo commented May 15, 2018

@Hainish, can you have a look at this? It's almost impossible to merge any PR because the tests are failing way too often.

@Hainish
Copy link
Copy Markdown
Member

Hainish commented May 15, 2018

Hey @bcyphers can you look at this? Thanks.

@J0WI J0WI requested a review from bcyphers May 15, 2018 21:53
Copy link
Copy Markdown
Contributor

@bcyphers bcyphers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@bcyphers
Copy link
Copy Markdown
Contributor

bcyphers commented May 16, 2018

@Giltyhub it's not clear to me how this will address #15239. I don't think SEL_DEFAULT_WAIT_TIMEOUT is relevant to anything in the driver.get(...); driver.current_url.startswith(...) sequence -- am I missing something?

@cschanaj
Copy link
Copy Markdown
Collaborator Author

@bcyphers I agree that this might not address #15239, as this PR was drafted in attempt to resolve the issues with the TestPopup.test_rule_shown (as shown in #15411)

@bcyphers
Copy link
Copy Markdown
Contributor

bcyphers commented May 17, 2018

Ah ok. I still think this is fine to merge, but it might help more to add some retries on failure for parts of the code that seem to break a lot. We have had similar issues with Privacy Badger; see e.g. EFForg/privacybadger#1894

@bcyphers bcyphers merged commit 439ec8d into EFForg:master May 17, 2018
@cschanaj cschanaj deleted the fix-selenum-test-options branch May 18, 2018 02:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants