-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Increase timeouts on XCUITest existence checks #70631
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
Conversation
jmagman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we start with 5.0 and file an issue if it's taking longer than that?
The code LGTM to green out the build though.
|
I'd really rather just remove the timeout altogether. This should never take more than 60s, but really we should just use the whole test timeout to find out exactly what's wrong. I just don't want to keep incremently moving this up all week as it continues to flake :) |
gaaclarke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this truly fixes your issue that would mean that the bots are super slow for some reason. Maybe we could make a timeout mechanism that is tied to to a metric that would scale with a slow bot like waitForRenderedFrames or waitForAllEventsToExecute. The latter wouldn't be hard to implement, just pop an event on the main queue. Run the main event queue for a couple of milliseconds then check if your event has been executed yet.
You mean make make the wait
I think that's essentially what |
|
The bots are slow - sometimes. The bot that failed this actually passed it again on a retry with the 1s timeout. I'm just going with a generous value that should avoid the need to revisit this - really we try to avoid timeouts in general except for the main test/bot timeout. |
|
It's not a huge deal but I don't think my point was conveyed. Let me try saying it another way. When making a game you have 2 different ways to progress time for an event. You can base it off of the clock or you can base it off the update rate. Tying your event to the real clock means that a drop in framerate will make sure your event still lasts the same amount of real time. Tying your event to the update event means that even when the framerate drops your event will progress with the same steps with time dilation. We are effectively performing our wait like an event based on real time, that's how Apple has implemented it. In our case, with bots that are sometimes slow, time dilation is a good thing. The proposed wait mechanism would be more like basing our wait on the update calls and would experience time dilation with a slow execution. Not a huge deal, I just want to make sure my point was communicated and understood. |
|
How would we achieve what you're suggesting with an XCUITest? |
|
My first 2 thoughts were tying the duration of the waiting to the framerate of the device (which should slow down on slow devices) or basing the waiting on flushing the whole event loop, which would also slow down on slow devices. |
We're currently waiting 1s, which may not be enough time on slower CI machines. Increases timeouts to 60s. Ideally we'd just have no timeout at all here, but if it takes more than a minute for these conditions something else must be wrong.
@jmagman
This will be landed on red to help make the tree green - current redness is due to #70630
Fixes #70630