search

Shopify / android-testify

Add screenshots to your Android tests
https://testify.dev
MIT License
231 stars 23 forks source link

Apply annotations to task properties for up-to-date checks #231

Closed jaredh closed 3 years ago

jaredh commented 3 years ago

What does this change accomplish?

Resolves https://github.com/Shopify/android-testify/issues/225

Adds missing annotations on public properties in Tasks to assist with up-to-date checks. Missing annotations are now errors in Gradle 7.0+.

As suggested in https://github.com/Shopify/android-testify/issues/225#issuecomment-879917888, I've confirmed this specific issue doesn't affect our ABI at all, and remains interopable with existing Gradle versions, at least with regards to the Plugin.

How have you achieved it?

Applied Internal to our public properties which were public for task subclasses.

Tophat instructions

  1. Bump your Gradle to 7.0.2
  2. Run ./gradlew --warning-mode all clean Sample:screenshotTest --console=plain
  3. Tasks should not fail with something like the following
❯ ./gradlew screenshotRecord
> Task :app:deviceLocale FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Some problems were found with the configuration of task ':app:deviceLocale' (type 'LocaleTask').
  - Type 'LocaleTask' property 'description' is missing an input or output annotation.
    Reason: A property without annotation isn't considered during up-to-date checking.
    Possible solutions:
      1. Add an input or output annotation.
      2. Mark it as @Internal.
    Please refer to https://docs.gradle.org/7.0/userguide/validation_problems.html#missing_annotation for more details about this problem.
DanielJette commented 3 years ago

Nitpick but your descriptions says Applied Ignored to our public properties which were public for task subclasses., but I think you meant Internal

jaredh commented 3 years ago

Nitpick but your descriptions says Applied Ignored to our public properties which were public for task subclasses., but I think you meant Internal

edited. Thank you contrubutor.

sav007 commented 3 years ago

@jaredh @DanielJette 👋 hey folks are you planing to do a new release any time soon with this fix?

jaredh commented 3 years ago

@jaredh @DanielJette 👋 hey folks are you planing to do a new release any time soon with this fix?

@sav007 long time no see! 😃 I'd like to cut a release very soon (now?). There was a bug introduced between the last RC and final AGP 7.0 release that we've been trying to resolve beforehand but it looks to be a legitimate bug upstream that is getting addressed in 7.0.1. It can be worked around though by manually specifying the testPackageId + applicationPackageId.