Add android run --app #389

Macarse · 2018-11-13T20:06:39Z

Macarse · 2018-11-13T21:56:16Z

test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt

@@ -45,7 +47,7 @@ class AndroidArgs(
    override val resultsHistoryName = gcloud.resultsHistoryName

    private val androidGcloud = androidGcloudYml.gcloud
-    val appApk = androidGcloud.app
+    val appApk = if (cmd.app.isEmpty()) androidGcloud.app else cmd.app


Basically, AndroidArgs will be created with cmdline class + yml information.

If we mark absent command values as null we could shorten this to:

cmd.app ?: androidGcloud.app

what do you think?

Macarse · 2018-11-13T21:57:10Z

test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt

+    @Test
+    fun androidArgs_overrideAppFromCmdLine() {
+        val cmd = AndroidRunCommand()
+        CommandLine(cmd).parse("--app", testApk)


I used testApk to avoid having a separate apk just to have a working test.

codecov-io · 2018-11-13T22:33:43Z

Codecov Report

Merging #389 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #389      +/-   ##
============================================
+ Coverage     79.88%   79.92%   +0.03%     
- Complexity      476      478       +2     
============================================
  Files            66       66              
  Lines          1546     1549       +3     
  Branches        234      234              
============================================
+ Hits           1235     1238       +3     
  Misses          173      173              
  Partials        138      138

bootstraponline · 2018-11-13T23:25:39Z

test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt


-        fun load(data: String): AndroidArgs {
+        fun load(data: String, cmd: AndroidRunCommand): AndroidArgs {


maybe default to an empty cmd:

fun load(data: String, cmd: AndroidRunCommand = AndroidRunCommand()): AndroidArgs {

then we don't need a bunch of emptyCmdLine in the tests

bootstraponline

provided some feedback, I like the idea of passing in AndroidRunCommand to AndroidArgs.

bootstraponline · 2018-11-13T23:30:28Z

test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidRunCommand.kt

@@ -35,4 +36,7 @@ class AndroidRunCommand : Runnable {

    @Option(names = ["-h", "--help"], usageHelp = true, description = ["Prints this help message"])
    var usageHelpRequested: Boolean = false
+
+    @Option(names = ["--app"], description = ["The path to the application binary file."])


we should borrow the descriptions from Firebase:

The path to the application binary file. The path may be in the local filesystem or in Google Cloud Storage using gs:// notation.

https://cloud.google.com/sdk/gcloud/reference/firebase/test/android/run

I think Flank supports gs:// notation for remote apks.

bootstraponline · 2018-11-13T23:57:12Z

test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt

@@ -45,7 +47,7 @@ class AndroidArgs(
    override val resultsHistoryName = gcloud.resultsHistoryName

    private val androidGcloud = androidGcloudYml.gcloud
-    val appApk = androidGcloud.app
+    val appApk = cmd.app ?: androidGcloud.app


thoughts on cli.app?

sure! done!

bootstraponline · 2018-11-14T00:43:45Z

test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt

@@ -223,8 +225,6 @@ AndroidArgs
            assert(projectId, "mockProjectId")

            // AndroidGcloudYml
-            assert(appApk, appApk)
-            assert(testApk, testApk)


I reverted these deletions.

oh, I forgot to comment about this diff.
That assert doesn't make sense to me, what are you testing?

Thanks for commenting, I fixed that here: 29670fe

The intent is to test the expected values are present on androidArgs. The name was shadowed. I capture the values above the with statement to resolve the conflict.

bootstraponline · 2018-11-14T00:48:10Z

test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt

+    @Test
+    fun androidRunCommandApp() {
+        val cmd = AndroidRunCommand()
+        CommandLine(cmd).parse("--app", "myApp.apk")


Added a test to verify null is the default value when --app is omitted.

bootstraponline · 2018-11-14T03:18:29Z

test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt

+          app: $appApk
+          test: $testApk
+
+        flank:


flank: may be omitted entirely from the YAML. I updated this test.

Macarse force-pushed the macarse/issue365 branch from 525a03d to f37602a Compare November 13, 2018 21:54

Macarse commented Nov 13, 2018

View reviewed changes

Macarse force-pushed the macarse/issue365 branch 3 times, most recently from 683779a to cce436e Compare November 13, 2018 22:33

bootstraponline reviewed Nov 13, 2018

View reviewed changes

bootstraponline suggested changes Nov 13, 2018

View reviewed changes

bootstraponline reviewed Nov 13, 2018

View reviewed changes

Macarse force-pushed the macarse/issue365 branch from cce436e to e20c32e Compare November 13, 2018 23:52

-app param

ef47288

bootstraponline changed the title ~~For review only~~ Add android run --app Nov 14, 2018

bootstraponline previously approved these changes Nov 14, 2018

View reviewed changes

Macarse dismissed bootstraponline’s stale review via ef47288 November 14, 2018 00:06

Macarse force-pushed the macarse/issue365 branch from e20c32e to ef47288 Compare November 14, 2018 00:06

bootstraponline previously approved these changes Nov 14, 2018

View reviewed changes

bootstraponline reviewed Nov 14, 2018

View reviewed changes

Restore test

8ca62b7

bootstraponline dismissed their stale review via 8ca62b7 November 14, 2018 00:44

bootstraponline reviewed Nov 14, 2018

View reviewed changes

Add @test app_parse_null

a45244c

bootstraponline mentioned this pull request Nov 14, 2018

CLI support for all args #365

Closed

34 tasks

bootstraponline previously approved these changes Nov 14, 2018

View reviewed changes

Fix androidArgs Test

29670fe

bootstraponline dismissed their stale review via 29670fe November 14, 2018 03:13

Clean up androidArgs_overrideAppFromCmdLine

db6b3c8

bootstraponline reviewed Nov 14, 2018

View reviewed changes

bootstraponline merged commit 38f100d into Flank:master Nov 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add android run --app #389

Add android run --app #389

Macarse commented Nov 13, 2018 •

edited by bootstraponline

Loading

Macarse Nov 13, 2018

bootstraponline Nov 13, 2018

Macarse Nov 13, 2018

codecov-io commented Nov 13, 2018 •

edited

Loading

bootstraponline Nov 13, 2018

Macarse Nov 13, 2018

bootstraponline left a comment

bootstraponline Nov 13, 2018

Macarse Nov 14, 2018

bootstraponline Nov 13, 2018

Macarse Nov 14, 2018

bootstraponline Nov 14, 2018

Macarse Nov 14, 2018

bootstraponline Nov 14, 2018

bootstraponline Nov 14, 2018

bootstraponline Nov 14, 2018


		fun load(data: String): AndroidArgs {
		fun load(data: String, cmd: AndroidRunCommand): AndroidArgs {

Add android run --app #389

Add android run --app #389

Conversation

Macarse commented Nov 13, 2018 • edited by bootstraponline Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Nov 13, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bootstraponline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Macarse commented Nov 13, 2018 •

edited by bootstraponline

Loading

codecov-io commented Nov 13, 2018 •

edited

Loading