wait and shut down the device after trying erase device's contents and setting #368
Conversation
thanhcs
force-pushed
the
improve-eraseContentsetting
branch
from
8177eec to
e6f1a19
Compare
thanhcs
force-pushed
the
improve-eraseContentsetting
branch
from
e6f1a19 to
8a64491
Compare
| simctlArgs.add(deviceUUID); | ||
| Command simctlCmd = new Command(simctlArgs, true); | ||
| List<String> simctlArgs; | ||
| simctlArgs = Arrays.asList("xcrun", "simctl", "erase", deviceUUID); |
There was a problem hiding this comment.
looks like most of the changes to eraseSimulator() are formatting changes. I think they are generally for the better although I would go ahead and combine the above 2 lines into a single line:
List<String> simctlArgs = Arrays.asList("xcrun", "simctl", "erase", deviceUUID);
fda9790 to
8a64491
Compare
|
Hi @freynaud, I addressed all of @mmccartney's comments. I think it's ready to merge. Can you take a look? Thanks, |
wait and shut down the device after trying erase device's contents and setting
|
@thanhcs One problem I have with this commit is the use of |
|
@mach6 There is a line 318. It should be:
I'm not sure about I don't think I can modify this PR anymore. |
|
this may be causing a travis-ci issue, the whole suite is taking too long and causing the 'build to fail'. Maybe because it can't shutdown the simulator and is causing it to take longer? @thanhcs you can open a new PR for that other change (and thus spin up another travis-ci job to check if it was just a one-time failure, or an on going issue) |
|
@lukeis I think this code is necessary. It only runs if the simulator needs more time to shut down (1 over 10 times when I ran the tests) You're right about failing to shut down the simulator will cause travis-CI take longer time to finish the build (even causing "build to fail" if the building time reaches 50 minutes). However, I think the problem is that there are a lot of tests needs to be fixed. Those failing tests, in my view, cause the build take longer time to finish. What I suggest is if we can disable all of the failing tests and only enable it after we fix it. I'm using travis-CI on my fork right now. Yes, sometimes it finishes the build, sometimes it doesn't. What I see is the failing tests run randomly (the order), and it will cause the "sometimes". |
|
@thanhcs sounds good to me :) |
|
@lukeis @mach6 👍 I think we really need to fix the tests. Around 300 tests in Note that apps' behaviors are different on different Xcode versions. You can see that if you run the tests in |
|
we should always target the latest version of xcode... even though travis-ci is behind (we need to wait for saucelabs to deploy the latest version of xcode). If we can identify the version of xcode running and mark the test skipped that fails on the earlier versions... although that sounds like a lot of busy work :) |
|
That is what @mmccartney suggested when I asked him about this issue. However, if we always use the lastest version of xcode, it is not important right now. Thanks @lukeis for pointing that out. |
|
@thanhcs line 347 |
|
@mach6 Yes. Sorry for my English. To be clear, that is an reasonable assumption after waiting a certain time for the simulator to shut down. If we send a shutdown command to the simulator which is already shut down, the system will return error code |
Old code: Erase right after shutting down the device may fail because the device at that time is still in
bootedstate. After that the old code tries to shut down the device while the device is already inshutdownstate.Output example: https://gist.github.com/thanhcs/f590b45726d84f47c5ea
new: Improve the code by wait 1 second after making an attempt at erasing device's contents and settings (3 times)
after that if the code still fails, shut down the device. If the shutdown is successful, erase the device's again.
Throw an exception if all attempts are failed.