[POC] Switch to writing to additional test output and device provider#82
[POC] Switch to writing to additional test output and device provider#82
Conversation
…s into rharter/device-provider
| .named { | ||
| it == "${deviceProviderName}${variant.name.capitalize()}${ComponentType.ANDROID_TEST_SUFFIX}" | ||
| } | ||
| .all { testTask -> |
There was a problem hiding this comment.
should be configureEach to avoid task creation
| else -> return@all | ||
| } | ||
| val defaultTestOutputDirectory = layout.buildDirectory | ||
| .dir("outputs/androidTest-results/$deviceProviderName/${variant.name}") |
There was a problem hiding this comment.
I was having trouble figuring our additionalTestOutputDir and I don't think it's actually used because of the return in testDataProvider evaluation above.
I think collpasing additionalTestOutputEnabled and additionalTestOutputDir would be a lot more readable.
val additionalTestOutputDir = when (testTask) {
is DeviceProviderInstrumentTestTask -> testTask.additionalTestOutputEnabled.flatMap {
if(it) { testTask.additionalTestOutputDir } else { provider { null } }
}
is ManagedDeviceInstrumentationTestTask -> testTask.getAdditionalTestOutputEnabled().flatMap {
if(it) { testTask.getAdditionalTestOutputDir() } else { provider { null } }
}
is ManagedDeviceTestTask -> testTask.getAdditionalTestOutputEnabled().flatMap {
if(it) { testTask.getAdditionalTestOutputDir() } else { provider { null } }
}
else -> return@configureEach
}
I would also consider combining it with the testDataProvider so that you only have a single when to read, but you'd need a data class of a pair return type so it's a little more subjective.
| public abstract class DeviceProviderFactory { | ||
| @Suppress("DEPRECATION") | ||
| @get:Internal | ||
| internal var deviceProvider: DeviceProvider? = null |
There was a problem hiding this comment.
Is this assigned anywhere? What's the reason for using a Nested Bean instead of just some helper method for getDeviceProvider?
There was a problem hiding this comment.
This is modeled after AGP, and I left this as a provider to enable tests that don't require connecting to actual devices. Those tests aren't implemented, and may not be, but I left this to leave that possibility.
|
Just a heads-up; additional test output might be broken when using multiple devices https://issuetracker.google.com/389673587 It'd be great if you could confirm this issue. |
This is a proof of concept implementation to explore the changes discussed in issue #31 (this comment specifically) to allow support for writing reference and diff images to the
additional_test_outputdirectory and leverage the AGP DeviceProvider to accessing devices.This is a fairly substantial change, so I'm creating a draft for feedback. There are several user-facing changes, most importantly the reference screenshot directory structure changes from
src/androidTest/screenshotstosrc/androidTest/screenshots/connectedfor the existing tests that are supported, andsrc/androidTest/screenshots/{device_name}for any additional devices (not validated yet).So far this change is tested with connected tests, with the intention that this will support Gradle managed devices, but I'm still testing and validating that. I wanted to share this draft for feedback and testing before it gets too big.
There are still some unknowns with this approach. For instance,
DeviceProvideris deprecated, but it's unclear what the accessible replacement is. I also don't love the way thatTestRunConfigis written to the device and would prefer to be able to add instrumentation arguments to the text execution instead, for reading at runtime.