Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify dash timeout #429

Closed
wants to merge 1 commit into from

Conversation

kwinkunks
Copy link
Member

Issue
Timeouts were set in a few different places, and we're trying to change them and add others to cope with Azure performance... but it's getting out of hand. Does not resolve but is connected to the perma-issue #428.

Approach

  • Make a way to set the explicit and implicit timeouts in one place: conftest.py
  • Make a way to set the timeouts from the command-line invocation of pytest
  • Double the timeouts when the testkomodo.sh script detects the $CI_RUNNER_LABEL env var has the valueazure
  • Remove all the custom timeout overrides
  • Remove a slightly hacky wait_a_bit function which added custom sleeps
  • Remove a bunch of commented-out code

Pre review checklist

  • Added appropriate labels

@kwinkunks kwinkunks added the improvement Something nice to have, that will make life easier for developers or users or both. label Apr 14, 2023
@ertomatic
Copy link
Collaborator

Can one of the admins verify this patch?

@kwinkunks
Copy link
Member Author

Three tests timed out, so will increase the timeout in the workflow and see if that fixes it.

Also Selenium seems to be throwing a lot of warnings, non-fatal.

@kwinkunks kwinkunks force-pushed the simplify-dash-timeout branch 2 times, most recently from 3b24cf9 to 16ffdaa Compare April 14, 2023 14:43
We are experiencing flaky test timeouts when running tests on Azure.
We have been addressing this by increasing timeouts in affected calls
but this seems unsustainable. This commit instead allows us to change
the implicit and explicit waits (see Selenium and dash.testing docs)
for all tests. Connected to equinor#428.

It also adds optional arguments to pytest to change these timeouts at
runtime, and allows testkomodo.sh to detect the environment (azure vs
onprem) from an environment variable.

It also removes a hacky `wait_a_bit` function, replacing it in one
case with an undocumented wait_for_no_elements call from dash.testing.
@kwinkunks
Copy link
Member Author

Doubled both timeouts (implicit and explicit) and two tests are failing. I don't tihnk the yare flaky / performance related, and they failed on the last PR before this one too, but I can't see what the issue is right now. I cannot reproduce it locally.

@kwinkunks kwinkunks marked this pull request as ready for review April 14, 2023 14:59
@hnformentin
Copy link
Contributor

As tests are failing, I would think it is not ready for review. I will move to todo, sorry if it was a mistake from my side.

@kwinkunks kwinkunks removed their assignment Sep 6, 2023
@kwinkunks
Copy link
Member Author

Closing this because we are no longer experiencing these flakinesses.

@kwinkunks kwinkunks closed this Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Something nice to have, that will make life easier for developers or users or both.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants