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 into
EFForg:masterfrom
cschanaj:fix-selenum-test-options
May 17, 2018
Merged

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

Conversation

@cschanaj

Copy link
Copy Markdown
Collaborator

No description provided.

@Bisaloo

Bisaloo commented May 15, 2018

Copy link
Copy Markdown
Collaborator

@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

Hainish commented May 15, 2018

Copy link
Copy Markdown
Member

Hey @bcyphers can you look at this? Thanks.

@J0WI
J0WI requested a review from bcyphers May 15, 2018 21:53

@bcyphers bcyphers left a comment

Copy link
Copy Markdown
Contributor

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

bcyphers commented May 16, 2018

Copy link
Copy Markdown
Contributor

@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

bcyphers commented May 17, 2018

Copy link
Copy Markdown
Contributor

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