Code changes for Collecting Device Logs - Killing the Emulator Process - Setting Permissions in iOS

[sarangan12]

The following functionalities have been added to Paramedic code.

1. To collect device logs
2. To kill the emulator processes
3. To set permissions to contacts for the test app

@omefire @dblotsky @riknoll @nikhilkh @sgrebnov @vladimir-kotikov Can you please review and merge this PR?

[riknoll]

Why split this variable into three parts like this?

[riknoll]

Can paramedicPermissions be renamed to make it clear that it's only for iOS? The other naming convention seems like it applies to the generic files.

[dblotsky]

Please add defaults for these properties, or set them to null instead of undefined.

[sarangan12]

@riknoll - Response for your comments

1. Why split this variable into three parts like this?
   Changed the logic to use util.format
2. Q is being required twice
   Removed it
3. Can paramedicPermissions be renamed to make it clear that it's only for iOS? The other naming convention seems like it applies to the generic files.
   Renamed it to ParamediciOSPermissions

@dblotsky - Response for your comments

1. Please add defaults for these properties, or set them to null instead of undefined.
   Changed the defaults to null.
2. Please add "iOS Simulator" to this array.
   Added
3. Nitpick: spacing. (in ParamedicPermissions.js (now ParamediciOSPermissions.js))
   Fixed it
4. If a regular for-loop is used here, the self pattern can be avoided.
   Removed it
5. Please use 0644 instead of 0777 here. Also, please factor out the permissions setting into a variable.
   Refactored the permission. But 0644 is not working. So keeping it as 0777.
6. Does this function only support iOS?
   Yes.

@nikhilkh - Response for your comments

1. Please use "adb logcat -d -v time"
   Done.
2. Consider renaming this to testResultsPath or testResultsDir or outputDir
   Done
3. Consider re-using outputDir
   Done
4. Either remove this config parameter & use a default value that you can assume to be true or create the app with the name specified here.
   Using a default value.
5. Consider using what we do for getting simulator logs. Ideally we should have one way of doing this and not configurable externally.
   Changed the logic and refactored the code between logs and permissions.
6. consider factoring it out into util module
   Done
7. Instead of copying a tcc db with permissions - consider copying empty and insert permissions into it - that helps in configuring permissions only in one place.
   Done
8. Consider using util.format
   Done.
9. Consider writing a comment of why you are not handling errors here.
   Done
10. Ideally you want to log return code and stderr/stdout
    Done
11. Consider just using "cordova" inline instead of this method.
    Done
12. spaces missing
    Fixed
13. emulator process?
    Fixed

@rakatyal - Response for your comments

1. Consider renaming it srcPath.
   Specific code has been removed

@omefire - Response for your comments

1. consider returning an empty array if none of the platforms match this list.
   Done.
2. in this case, we want to stop processing and bail out of the kill function. Please, add a return statement here.
   Done
3. this break statement will never be reached.
   Fixed it
4. nitpick: 'dbPath' -> 'tccDbPath'
   Fixed it.

@sgrebnov - Response for your comments

1. Readme.md should be updated as well
   Done
2. Permission logic could be implemented via separate plugin with hooks
   Hooks will be handled as a seperate user story in the following sprinits. I will create a task for this.

[dblotsky]

Why doesn't 0644 work? If execution permissions are required on the folder, then please use 0755. Using 0777 is a security hazard.

[sarangan12]

Done

[nikhilkh]

👍

[omefire]

So, running this code, I face an issue where the simId returned is null.
Maybe, there's something going on with the regex ?
Here's the output of various bash commands executed here:

1- instruments -s devices | grep ^iPhone

iPhone 4s (9.3) [93EC91E3-2679-49DC-XXXX-CAB279E462AF] (Simulator)
iPhone 5 (9.3) [830AC555-1986-4B48-82C7-898849A50541] (Simulator)
iPhone 5s (9.3) [17726EFF-F3B9-XXXX-8882-CC6ED589C213] (Simulator)
iPhone 6 (9.3) [3BDE39CD-7998-4959-B2D8-7C08C33B20B1] (Simulator)
iPhone 6 Plus (9.3) [7235ADB5-8730-4D0A-XXXX-5469D1ED47D7] (Simulator)
iPhone 6s (9.3) [32745196-XXXX-4927-ADA0-16BACE15FC99] (Simulator)
iPhone 6s (9.3) + Apple Watch - 38mm (2.2) [6A868DF2-XXXX-4AEE-9EC1-B956375934B9] (Simulator)
iPhone 6s Plus (9.3) [A949CB17-350F-4B28-AD64-XXXXXXXXXXXX] (Simulator)
iPhone 6s Plus (9.3) + Apple Watch - 42mm (2.2) [E0C9BC62-83AA-45C5-B411-XXXXXXXXXXXX] (Simulator)

2- cordova run --list --emulator | grep ^iPhone | tail -n1

iPhone-6s-Plus, 9.3

[omefire]

So, turns out this regex breaks in Xcode 7.3, which leads to no simId being returned, which leads to a crash.
It's because of the (Simulator) added at the end, I still think this code is too fragile as it depends on specific Xcode output, which might change in the future. Maybe, we should go back to searching through all folders like before ?

In the meantime, reajusting the regex to var simIdRegex = /^([a-zA-Z\d ]+) \(([\d.]+)\) \[([a-zA-Z\d\-]*)\].*$/; fixes this issue.

[sarangan12]

Done

[sarangan12]

Merge complete. Closing the PR