Add coverage report #148
Add coverage report #148
Conversation
| names = arrayOf("--coverage"), | ||
| required = false, | ||
| arity = 1, | ||
| description = "Either `true` or `false` to enable/disable test coverage collection. `false` by default.", |
There was a problem hiding this comment.
Should we add a note that it doesn't turn on code coverage in the tests but just collects it?
README needs to be updated too btw
| @@ -111,15 +113,25 @@ private fun runAllTests(args: Args, testPackage: TestPackage.Valid, testRunner: | |||
| val installTimeout = Pair(args.installTimeoutSeconds, TimeUnit.SECONDS) | |||
| val installAppApk = device.installApk(pathToApk = args.appApkPath, timeout = installTimeout) | |||
| val installTestApk = device.installApk(pathToApk = args.testApkPath, timeout = installTimeout) | |||
| val coverageDir = "/storage/emulated/0/app_coverage/${testPackage.value}" | |||
There was a problem hiding this comment.
Shouldn't we use EXTERNAL_STORAGE environment variable instead of hardcoding external storage directory?
| @@ -0,0 +1,5 @@ | |||
| package com.gojuno.composer | |||
|
|
|||
| val EXTERNAL_STORAGE = "/storage/emulated/0" | |||
There was a problem hiding this comment.
I meant retrieving the actual value from environment on device using something like adb shell printenv EXTERNAL_STORAGE.
AFAIR it is mandatory: https://source.android.com/devices/storage/config-example
There was a problem hiding this comment.
Btw I guess we can have this code in commander
There was a problem hiding this comment.
Do you need me to try and do this or will you add something more generic to commander ?
There was a problem hiding this comment.
I'll add it to commander now
|
Related: #151 |
| private fun buildCoverageArguments(coverage: Boolean, coverageDir: String): List<Pair<String, String>> = | ||
| if (coverage) listOf( | ||
| "coverage" to "true", | ||
| "coverageFile" to "$coverageDir/coverage.ec" |
There was a problem hiding this comment.
This needs to be coverageFilePath when running with testOrchestrator and then we will have to pull down .ec for each test case.
|
@tokou Any reason why this wasn't merged yet? |
Fixes #123