-
-
Notifications
You must be signed in to change notification settings - Fork 454
Possible solution for multiple issues about idevicedate usage #326
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
jlipps
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 you explain what this is doing in prose a bit? also @imurchie should probably review
|
Sure. The task here is to prevent preemptive WDA startup by filtering out outdated log entries. The current implementation users idevicedate utility for this purpose with fallback to local machine date. The problem there is, that not all Appium users have idevice library properly updated, which causes idevicedate execution to fail. Appium cannot detect driver startup as a result, because it cannot distinguish old log entries from new old entries. My approach tries to solve the issue without affecting idevicedate utility. It just executes logs minitoring asynchronously before xcodebuild execution. Then catches all the output collected by log reader utility so far and treats this as old logs. Then we set shared flag, which indicates xcodebuild has already started and continue monitoring logs. This monitoring blocks main thread until there is a new log record about WDA initialisation or timeout happens (hardcoded as 2 minutes). This helps to avoid endless loop in case something happened to WDA and it cannot start properly. |
|
I haven't been able to get this working locally, with sims. It also fails in Travis (ignoring the linting problems). |
|
That is strange :( |
|
Locally it hangs and for some reason I cannot even stop the process. I ended up having to kill the terminal and then use |
|
@imurchie Hell yeah, it works on Travis now! |
jlipps
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.
code LGTM, @imurchie can you retest on your end?
|
@mykola-mokhnach I'm going to give this a shot locally, and look over what is going on. Can you have a look, when you have time, at #327 ? It is solving the problem from another angle. |
|
Closed in favour of #327 |
@imurchie please take a look. I have tested this code with our automated framework and it seems there are no major issues there