[iOS] Added Support for XCResult#686
[iOS] Added Support for XCResult#686abhishekbedi1432 wants to merge 15 commits intoMarathonLabs:developfrom
Conversation
abhishekbedi1432
requested review from
Malinskiy,
Vacxe and
tagantroy
as code owners
…tag-xcxresult-testplan-from-anton * ios-uitest-runner-via-tags: Update README.md Added Support to run iOS Test Runners Tag # Conflicts: # configuration/src/main/kotlin/com/malinskiy/marathon/config/vendor/VendorConfiguration.kt # sample/ios-app/Marathonfile
… into develop/talabat/final-tag-xcxresult-testplan-from-anton * develop/ios-support-for-test-plan-from-xcresult-branch: Modified XCTestRun to adapt to new format of xctestrun ( supporting test plan)
Malinskiy
left a comment
Malinskiy
left a comment
There was a problem hiding this comment.
Logic wise we're missing pulling back the xcresult back to the marathon's side
|
|
||
| Please check the official [documentation](https://marathonlabs.github.io/marathon/) for installation, configuration and more | ||
|
|
||
| ## [iOS Only] Added Support to consume tags for test functions under XCTestCase Subclasses |
There was a problem hiding this comment.
Please move the docs info to the https://github.com/MarathonLabs/marathon/blob/develop/docs/_posts/2018-11-19-ios.md. They're automatically published at https://marathonlabs.github.io/marathon/
| ?: fileListProvider | ||
| val resolvedResultBundlePath = marathonfileDir.resolve(configuration.vendorConfiguration.xcResultBundlePath) | ||
|
|
||
| // Adding support for Test Plan |
There was a problem hiding this comment.
Please remove the comment. It doesn't make sense after we merge this.
| .fileList(resolvedDerivedDataDir) | ||
| .firstOrNull { it.extension == "xctestrun" } | ||
| ?: throw ConfigurationException("Unable to find an xctestrun file in derived data folder") | ||
| .firstOrNull { it.extension == "xctestrun" && it.name.contains("$testPlanName") } ?: throw ConfigurationException("Unable to find matching TestPlan. Please recheck if testplan is enabled") |
There was a problem hiding this comment.
You can skip creating an additional string here:
it.name.contains("$testPlanName") -> it.name.contains(testPlanName)
It might also be a good idea to use startsWith or equals instead of contains. I'm not sure if there is a contract on the file name here
| val tests = applyTestFilters(parsedTests) | ||
| val shard = prepareTestShard(tests, analytics) | ||
|
|
||
| log.info("\n\n\n **** Marathon File Params **** \n") |
There was a problem hiding this comment.
This is a no-no: configuration might contain sensitive credentials, and we can't print them to the stdout
There was a problem hiding this comment.
If you really want to do this then we need to implement some kind of masking for sensitive fields
| val method: String, | ||
| val metaProperties: Collection<MetaProperty> | ||
| val metaProperties: Collection<MetaProperty>, | ||
| val tags: Collection<String> = emptyList() |
There was a problem hiding this comment.
MetaProprety is an abstraction over tags, annotations and so on. We should just reuse MetaProperty here
There was a problem hiding this comment.
Refer:
[iOS]Added Support for Cucumberish Tags / Function Annotations #702
| return tests | ||
| val iosConfig = configuration.vendorConfiguration as? VendorConfiguration.IOSConfiguration | ||
|
|
||
| return tests.filter { it.tags.contains(iosConfig?.xcTestRunnerTag) } |
There was a problem hiding this comment.
This should just be an annotation filter, no vendor-specific logic in the core
| super.tearDown() | ||
| } | ||
|
|
||
| // @Flowers |
There was a problem hiding this comment.
Instead of modifying existing tests we should just add more tests with these tags
| private fun TestBatch.toXcodebuildArguments(): String = | ||
| tests.joinToString(separator = " ") { "-only-testing:\"${it.pkg}/${it.clazz}/${it.method}\"" } | ||
|
|
||
| private fun getTimezone(): String { |
There was a problem hiding this comment.
This method sounds like getTimestamp rather than getTimezone
| override suspend fun extract(): List<Test> { | ||
|
|
||
| if (vendorConfiguration.xcTestRunnerTag.isNullOrEmpty()) { | ||
| logger.warn { "[iOS] Did not find `xcTestRunnerTag` in iOS Vendor Config. This will result in running all tests" } |
There was a problem hiding this comment.
The tag filtering doesn't have anything to do with the responsibility of the parser, this probably doesn't belong here
| */ | ||
|
|
||
| private val targetMap: Map<String, *> = ( | ||
| ((((((propertyList.valueForKeypath("TestConfigurations") as? Array<Any>)?.first()) as Map<*, *>) |
There was a problem hiding this comment.
first() will throw NullPointerException here when TestConfigurations is missing. Reproduced on the old sample
|
Closes #620 |
…latform-Test-Runner-Marathon into develop/talabat/final-tag-xcxresult-testplan-from-anton
…final-tag-xcxresult-testplan-from-anton
2 participants
Motivation
Marathon for iOS produces an amazing HTML report (in the
outputDir) but sometimes we need the finalxcresultbundle for many reasons (say integrating with the CI, uploading the result in repository, etc).The
xcresultbundles are stored at transient paths under Xcode'sderived-datafolder.In this PR, we have added an enhancement of providing
xcResultBundlePathparameter in the Marathon file (under iOS Vendor Config)As a positive side effect, this helps to avoid flooding Xcode's derived data directory.