Skip to content
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

feat: iOS add directories-to-pull #1266

Merged
merged 9 commits into from
Nov 2, 2020

Conversation

adamfilipow92
Copy link
Contributor

Fixes #1198

Test Plan

How do we know the code works?

Can set directories-to-pull in iOS

Checklist

  • Unit tested

"/sdcard or /data/local/tmp (for example, --directories-to-pull /sdcard/tempDir1,/data/local/tmp/tempDir2). " +
"Path names are restricted to the characters a-zA-Z0-9_-./+. The paths /sdcard and /data will be made available " +
"and treated as implicit path substitutions. E.g. if /sdcard on a particular device does not map to external " +
"storage, the system will replace it with the external storage path prefix for that device."]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is description for Android, not for common. Try to make it more generic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a copy pasted description? if it is it should most likely be left as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a description from android. I moved this property from android to common so now this description should have information for android and ios.

@piotradamczyk5
Copy link
Contributor

please update docs/index.md and flank.ios.yml

@piotradamczyk5 piotradamczyk5 added iOS New Option Used to track PR with new configuration option (useful when updating fladle) labels Oct 23, 2020
@bootstraponline bootstraponline force-pushed the 1198-ios-directories-to-pull branch from 9ed1f1c to a3e0976 Compare October 26, 2020 05:07
@codecov-io
Copy link

codecov-io commented Oct 26, 2020

Codecov Report

Merging #1266 into master will increase coverage by 0.01%.
The diff coverage is 53.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1266      +/-   ##
============================================
+ Coverage     79.65%   79.67%   +0.01%     
  Complexity      724      724              
============================================
  Files           235      235              
  Lines          4498     4501       +3     
  Branches        773      773              
============================================
+ Hits           3583     3586       +3     
  Misses          515      515              
  Partials        400      400              

@pawelpasterz
Copy link
Contributor

Do we have any working example to verify if this feature is working?
If our current test iOS app is enough please add a detailed/descriptive test plan

@@ -62,6 +63,7 @@ object GcIosTestMatrix {

val iOSTestSetup = IosTestSetup()
.setNetworkProfile(args.networkProfile)
.setPullDirectories(args.directoriesToPull.toIosDeviceFiles())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, for android we pass a list of strings... and the field itself is named differently directoriesToPull 😸

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

google API is full of surprises

@pawelpasterz
Copy link
Contributor

I can see it's a rather simple change but I will feel more comfortable if I could test it.

@bootstraponline bootstraponline force-pushed the 1198-ios-directories-to-pull branch from a3e0976 to 8dbfcd2 Compare October 26, 2020 07:31
@adamfilipow92
Copy link
Contributor Author

adamfilipow92 commented Oct 26, 2020

please update docs/index.md and flank.ios.yml

Thanks, changed

@piotradamczyk5
Copy link
Contributor

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Oct 26, 2020

Command rebase: failure

Branch rebase failed
First, rewinding head to replay your work on top of it...
Applying: Moved directories to pull to CommonArgs, added tests
.git/rebase-apply/patch:72: trailing whitespace.

warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
M test_runner/src/main/kotlin/ftl/args/CommonArgs.kt
M test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt
M test_runner/src/main/kotlin/ftl/args/CreateCommonArgs.kt
M test_runner/src/main/kotlin/ftl/args/IArgs.kt
M test_runner/src/main/kotlin/ftl/args/IosArgs.kt
M test_runner/src/main/kotlin/ftl/config/android/AndroidGcloudConfig.kt
M test_runner/src/main/kotlin/ftl/config/common/CommonGcloudConfig.kt
M test_runner/src/main/kotlin/ftl/gc/GcIosTestMatrix.kt
M test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt
M test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosRunCommandTest.kt
M test_runner/src/test/kotlin/ftl/gc/GcIosTestMatrixTest.kt
Falling back to patching base and 3-way merge...
Auto-merging test_runner/src/test/kotlin/ftl/gc/GcIosTestMatrixTest.kt
CONFLICT (content): Merge conflict in test_runner/src/test/kotlin/ftl/gc/GcIosTestMatrixTest.kt
Auto-merging test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosRunCommandTest.kt
Auto-merging test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt
CONFLICT (content): Merge conflict in test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt
Auto-merging test_runner/src/main/kotlin/ftl/gc/GcIosTestMatrix.kt
CONFLICT (content): Merge conflict in test_runner/src/main/kotlin/ftl/gc/GcIosTestMatrix.kt
Auto-merging test_runner/src/main/kotlin/ftl/config/common/CommonGcloudConfig.kt
CONFLICT (content): Merge conflict in test_runner/src/main/kotlin/ftl/config/common/CommonGcloudConfig.kt
Auto-merging test_runner/src/main/kotlin/ftl/config/android/AndroidGcloudConfig.kt
CONFLICT (content): Merge conflict in test_runner/src/main/kotlin/ftl/config/android/AndroidGcloudConfig.kt
Auto-merging test_runner/src/main/kotlin/ftl/args/IosArgs.kt
CONFLICT (content): Merge conflict in test_runner/src/main/kotlin/ftl/args/IosArgs.kt
Auto-merging test_runner/src/main/kotlin/ftl/args/IArgs.kt
CONFLICT (content): Merge conflict in test_runner/src/main/kotlin/ftl/args/IArgs.kt
Auto-merging test_runner/src/main/kotlin/ftl/args/CreateCommonArgs.kt
CONFLICT (content): Merge conflict in test_runner/src/main/kotlin/ftl/args/CreateCommonArgs.kt
Auto-merging test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt
Auto-merging test_runner/src/main/kotlin/ftl/args/CommonArgs.kt
CONFLICT (content): Merge conflict in test_runner/src/main/kotlin/ftl/args/CommonArgs.kt
Auto-merging test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
error: Failed to merge in the changes.
Patch failed at 0001 Moved directories to pull to CommonArgs, added tests
Use 'git am --show-current-patch' to see the failed patch

Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

err-code: 53A23

@piotradamczyk5 piotradamczyk5 force-pushed the 1198-ios-directories-to-pull branch from 9c75613 to cb2df6f Compare October 27, 2020 13:23
@piotradamczyk5 piotradamczyk5 marked this pull request as draft October 27, 2020 13:25
@piotradamczyk5 piotradamczyk5 force-pushed the 1198-ios-directories-to-pull branch from e0abffa to ee8cc72 Compare October 29, 2020 14:53
@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2020

Timestamp: 2020-11-02 13:11:22
Buildscan url for ubuntu-workflow run 341714181
https://gradle.com/s/jz2tinz6fvmnk

@piotradamczyk5 piotradamczyk5 force-pushed the 1198-ios-directories-to-pull branch from 3b05bfa to c4cdaa2 Compare November 2, 2020 13:06
@piotradamczyk5 piotradamczyk5 marked this pull request as ready for review November 2, 2020 13:14
@piotradamczyk5 piotradamczyk5 merged commit 22da7bf into master Nov 2, 2020
@piotradamczyk5 piotradamczyk5 deleted the 1198-ios-directories-to-pull branch November 2, 2020 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS New Option Used to track PR with new configuration option (useful when updating fladle) Tiger 🐯
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS | Add --directories-to-pull
7 participants