Conversation
| )) | ||
| } | ||
|
|
||
| val adbExecutablePath = provider { android.adbExecutable.path } |
There was a problem hiding this comment.
I'm pretty much at a loss to why most of the plugin logic is duplicated here, so let me know if I'm missing something.
There was a problem hiding this comment.
I'm pretty sure it's because there was a bootstrapping issue where the plugin automatically adds a Dropshots dependency, but we want to use the code under test here. There are also some minor differences in behavior IIRC. for instance, this always pulls screenshots but does not automatically overwrite the reference images when recording, since that would override the tests that are expected to fail.
There was a problem hiding this comment.
But, is it still in use anywhere? I didn't see any references to it.
|
|
||
| val deviceScreenshotDir = | ||
| androidTest.applicationId.map { "/storage/emulated/0/Download/screenshots/$it" } | ||
| val buildScreenshotDir = |
There was a problem hiding this comment.
This is possibly a breaking change if anyone is hardcoding the results directory.
We used to use (it as AndroidTestTask).resultsDir but this task is created very late in AGP now and a headache to get a reference to. It's also not a good practice to use another tasks output directory in this way.
dropshots-gradle-plugin/src/main/kotlin/com/dropbox/dropshots/DropshotsPlugin.kt
Show resolved
Hide resolved
dropshots-gradle-plugin/src/main/kotlin/com/dropbox/dropshots/ClearScreenshotsTask.kt
Show resolved
Hide resolved
| .withArguments("recordDebugAndroidTestScreenshots") | ||
| .withArguments("recordDebugAndroidTestScreenshots", "--stacktrace") | ||
| .build() | ||
| println(result.output) |
There was a problem hiding this comment.
Yes, but I left it intentionally as it helps debugging when these tests fail.
| )) | ||
| } | ||
|
|
||
| val adbExecutablePath = provider { android.adbExecutable.path } |
There was a problem hiding this comment.
I'm pretty sure it's because there was a bootstrapping issue where the plugin automatically adds a Dropshots dependency, but we want to use the code under test here. There are also some minor differences in behavior IIRC. for instance, this always pulls screenshots but does not automatically overwrite the reference images when recording, since that would override the tests that are expected to fail.
| } | ||
| } | ||
|
|
||
| private fun NamedDomainObjectContainer<out AndroidSourceSet>.addDropshotsAssetsDir( |
There was a problem hiding this comment.
Is there a reason to extract this part of the dropshots configuration out of the configureDropshots function? Both ApplicationExtension and LibraryExtension still implement TestedExtension, so splitting the configuration in two doesn't seem entirely necessary (or desirable).
There was a problem hiding this comment.
TestedExtension does define sourceSets
| import org.gradle.work.DisableCachingByDefault | ||
|
|
||
| @DisableCachingByDefault( | ||
| because = "The task interacts with the connected test device, so caching is not applicable." |
There was a problem hiding this comment.
Is this true? These tasks still have input, so if the input hasn't changed or dependent tasks aren't executed then there should be no work to do here.
There was a problem hiding this comment.
Clean tasks generally aren't cached
but, this task should probably also have a
outputs.upToDateWhen { false }
since it interacts with adb and input tracking will not be accurate
Reworks plugin to use the new AGP apis like
androidComponentscloses #111