-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Initial test to see if screenshot timing is the prob. #5932
Conversation
I know we don't want fragile sleep's in the tests but 4 seconds before each screenshot fixes the problems where the actual charts were blank. Examples here; |
As a less fragile alternative would a |
@jbudz I could, but I don't have one generic method to get chart data as it depends on the type of chart. So each test would have to tryForTime a call to get the chart data for that chart type and test that the size of the returned data > 0. And what I realize now, is that the tests for getting the chart data do not have a tryForTime, but they work because they come after these 'save, load, screenshot' tests and the charts are done loading by that time. |
If screenshots need to get working, and the sleeps aren't part of the pass/fail testing, I'm not going to be overly picky. A comment explaining why the sleep needs to happen might be useful. The changes without 9157cf7 LGTM |
… promise chains in visualizePage.
This reverts commit b6f6792.
I had to use a short sleep in a couple of cases. I added comments for them. |
… promise chains in visualizePage. Fixes #5932
… promise chains in visualizePage. Fixes #5932
Initial test to see if screenshot timing is the prob.