-
Notifications
You must be signed in to change notification settings - Fork 81
fix: delay before starting maestro tests #1804
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
924a7ae to
a8cae4a
Compare
filip131311
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.
I have questions inline and another here:
We observed a problem with the permissions script only at the beginning of the test suite run. Is it because of our example tests or is it something normal? If it is only an issue with the setup could we detect that this part of the process is done and clean up the sim links then? this way we could minimize the risk of leaving symlinks behind
| } catch (e) {} | ||
| const id = this.device.id; | ||
| const devicePath = path.join(getOrCreateDeviceSet(id), id); | ||
| const coreSimulatorPath = path.join( |
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.
Is it always the same even for users with beta versions of the Xcode? shouldn't we really on xcode-select or something like that?
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.
It's not stored within XCode, it's in your Library path.
This path was taken from applesimutils implementation so even if the system decided to place the sims somewhere else, for applesimutils to work we need to link them here.
| this.maestroProcess = undefined; | ||
| } | ||
|
|
||
| public async stopMaestroTest(): Promise<void> { |
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.
why does it not cleanup simlinks?
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.
Why would it?
Stopping the process will make the maestroProcess "promise" reject, triggering the finally clause set up in start.
You should cleanup wherever you initialize the resource, where possible.
I wouldn't overthink this. If the symlink causes issues, it will cause them regardless if we keep it for the 15 seconds it takes maestro to set up, or the 30 seconds it takes it to run the tests to completion. |
filip131311
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.
My concerns were misplaced
Fixes a 30s delay after starting maestro tests and before the app is launched on the device.
The delay was caused by
maestrocallingapplesimutilsutility in order to set permissions on the device. The utility does not support passing simulator device sets (and even if it did,maestrowould not set it anyway), so it would repeatedly try to set permissions on a simulator device which does not exist, and timeout after 30s, after whichmaestrowould proceed with running the application to be tested.The solution allows
applesimutilsto modify the permissions on Radon simulator by symlinking it to theCoreSimulatordirectory -- sinceapplesimutilsaccesses the simulator's files directly (without checking the simulators database, likexcodebuild), a symlink is sufficient for it to function.How Has This Been Tested:
How Has This Change Been Documented:
Bugfix