Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verify does not take in to account resource changes #747

Closed
timrijckaert opened this issue Mar 31, 2023 · 6 comments · Fixed by #690
Closed

verify does not take in to account resource changes #747

timrijckaert opened this issue Mar 31, 2023 · 6 comments · Fixed by #690
Labels
bug Something isn't working

Comments

@timrijckaert
Copy link

timrijckaert commented Mar 31, 2023

Description
When a Composable is using a string resource the verify tasks does not take into account changes that could have happened to that resources.

Steps to Reproduce

Index: sample/src/test/java/app/cash/paparazzi/sample/HelloComposeTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sample/src/test/java/app/cash/paparazzi/sample/HelloComposeTest.kt b/sample/src/test/java/app/cash/paparazzi/sample/HelloComposeTest.kt
--- a/sample/src/test/java/app/cash/paparazzi/sample/HelloComposeTest.kt	(revision 9e4684a5a180a52cef543efbe0a6d26af3aaea1e)
+++ b/sample/src/test/java/app/cash/paparazzi/sample/HelloComposeTest.kt	(date 1680258820694)
@@ -8,6 +8,7 @@
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
+import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.TextStyle
 import androidx.compose.ui.text.font.FontFamily
 import androidx.compose.ui.text.font.FontWeight
@@ -37,7 +38,7 @@
       .wrapContentSize()
   ) {
     Text(text)
-    Text(text, style = TextStyle(fontFamily = FontFamily.Cursive))
+    Text(stringResource(id = R.string.test), style = TextStyle(fontFamily = FontFamily.Cursive))
     Text(
       text = text,
       style = TextStyle(textDecoration = TextDecoration.LineThrough)
Index: sample/src/main/res/values/strings.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sample/src/main/res/values/strings.xml b/sample/src/main/res/values/strings.xml
new file mode 100644
--- /dev/null	(date 1680258846345)
+++ b/sample/src/main/res/values/strings.xml	(date 1680258846345)
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+  <string name="test">Hello World</string>
+</resources>

Run ./gradlew recordPaparazziDebug
A screenshot is generated

app cash paparazzi sample_HelloComposeTest_compose

Change the string resource value

Index: sample/src/main/res/values/strings.xml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sample/src/main/res/values/strings.xml b/sample/src/main/res/values/strings.xml
--- a/sample/src/main/res/values/strings.xml	(revision 3ee2d41e6c3889c9cd4e1b8964735f484f8fa956)
+++ b/sample/src/main/res/values/strings.xml	(date 1680259023916)
@@ -1,4 +1,4 @@
 <?xml version="1.0" encoding="utf-8"?>
 <resources>
-  <string name="test">Hello World</string>
+  <string name="test">Hello Paparazzi</string>
 </resources>

run ./gradlew verifyPaparazziDebug

The build succeeds without any issue.

BUILD SUCCESSFUL in 9s
36 actionable tasks: 1 executed, 3 from cache, 32 up-to-date

Expected behavior
Build should fail.

Additional information:

  • Paparazzi Version: 1.2.0
  • OS: MacOS Ventura 13.1
  • Compile SDK: (Sample project) 33
  • Gradle Version: (Sample project) 7.6
  • Android Gradle Plugin Version: (Sample project) 7.4.0

Screenshots
n/a

Thanks in advance

@timrijckaert timrijckaert added the bug Something isn't working label Mar 31, 2023
@TWiStErRob
Copy link
Contributor

Can you please change the string again, and run gradlew verifyPaparazziDebug --console=verbose and post the output? Curious what the :testDebugUnitTest outcome is.

@timrijckaert
Copy link
Author

Type-safe project accessors is an incubating feature.
projectsLoaded

> Configure project :paparazzi:libs:layoutlib
No compatible plugin found in project layoutlib for publishing

> Configure project :paparazzi:libs:native-linux
No compatible plugin found in project native-linux for publishing

> Configure project :paparazzi:libs:native-macarm
No compatible plugin found in project native-macarm for publishing

> Configure project :paparazzi:libs:native-macosx
No compatible plugin found in project native-macosx for publishing

> Configure project :paparazzi:libs:native-win
No compatible plugin found in project native-win for publishing

> Task :paparazzi:paparazzi-gradle-plugin:pluginVersion UP-TO-DATE
> Task :paparazzi:paparazzi-gradle-plugin:compileKotlin UP-TO-DATE
> Task :paparazzi:paparazzi-gradle-plugin:compileJava NO-SOURCE
> Task :paparazzi:paparazzi-gradle-plugin:pluginDescriptors UP-TO-DATE
> Task :paparazzi:paparazzi-gradle-plugin:processResources UP-TO-DATE
> Task :paparazzi:paparazzi-gradle-plugin:classes UP-TO-DATE
> Task :paparazzi:paparazzi-gradle-plugin:jar UP-TO-DATE
> Task :paparazzi:paparazzi-gradle-plugin:inspectClassesForKotlinIC UP-TO-DATE

> Configure project :
projectsLoaded

> Configure project :sample
WARNING:Software Components will not be created automatically for Maven publishing from Android Gradle Plugin 8.0. To opt-in to the future behavior, set the Gradle property android.disableAutomaticComponentCreation=true in the `gradle.properties` file or use the new publishing DSL.

> Task :paparazzi:paparazzi:processResources UP-TO-DATE
> Task :sample:preBuild UP-TO-DATE
> Task :sample:preDebugBuild UP-TO-DATE
> Task :sample:compileDebugAidl NO-SOURCE
> Task :sample:compileDebugRenderscript NO-SOURCE
> Task :paparazzi:paparazzi-agent:compileKotlin UP-TO-DATE
> Task :paparazzi:paparazzi-agent:compileJava NO-SOURCE
> Task :paparazzi:paparazzi-agent:processResources NO-SOURCE
> Task :paparazzi:paparazzi-agent:classes UP-TO-DATE
> Task :sample:dataBindingMergeDependencyArtifactsDebug UP-TO-DATE
> Task :sample:generateDebugResValues UP-TO-DATE
> Task :sample:generateDebugResources UP-TO-DATE
> Task :paparazzi:paparazzi-agent:jar UP-TO-DATE
> Task :sample:packageDebugResources UP-TO-DATE
> Task :paparazzi:paparazzi-agent:inspectClassesForKotlinIC UP-TO-DATE
> Task :sample:dataBindingGenBaseClassesDebug UP-TO-DATE
> Task :sample:generateDebugBuildConfig UP-TO-DATE
> Task :sample:parseDebugLocalResources UP-TO-DATE
> Task :sample:processDebugManifest UP-TO-DATE
> Task :sample:generateDebugRFile UP-TO-DATE
> Task :sample:compileDebugKotlin NO-SOURCE
> Task :sample:processDebugJavaRes NO-SOURCE
> Task :sample:bundleLibResDebug NO-SOURCE
> Task :sample:javaPreCompileDebug UP-TO-DATE
> Task :sample:compileDebugJavaWithJavac UP-TO-DATE
> Task :sample:bundleLibRuntimeToJarDebug UP-TO-DATE
> Task :sample:bundleLibCompileToJarDebug UP-TO-DATE
> Task :sample:preDebugUnitTestBuild UP-TO-DATE
> Task :sample:generateDebugUnitTestStubRFile UP-TO-DATE
> Task :sample:mergeDebugShaders UP-TO-DATE
> Task :sample:compileDebugShaders NO-SOURCE
> Task :sample:generateDebugAssets UP-TO-DATE
> Task :sample:mergeDebugAssets UP-TO-DATE
> Task :paparazzi:paparazzi:kaptGenerateStubsKotlin UP-TO-DATE
> Task :sample:mergeDebugResources UP-TO-DATE
> Task :sample:preparePaparazziDebugResources UP-TO-DATE
> Task :sample:javaPreCompileDebugUnitTest UP-TO-DATE
> Task :sample:processDebugUnitTestJavaRes NO-SOURCE
> Task :paparazzi:paparazzi:kaptKotlin UP-TO-DATE
> Task :paparazzi:paparazzi:compileKotlin UP-TO-DATE
> Task :paparazzi:paparazzi:compileJava UP-TO-DATE
> Task :paparazzi:paparazzi:classes UP-TO-DATE
> Task :paparazzi:paparazzi:jar UP-TO-DATE
> Task :paparazzi:paparazzi:inspectClassesForKotlinIC UP-TO-DATE
> Task :sample:compileDebugUnitTestKotlin UP-TO-DATE
> Task :sample:compileDebugUnitTestJavaWithJavac NO-SOURCE
> Task :sample:testDebugUnitTest UP-TO-DATE
> Task :sample:verifyPaparazziDebug UP-TO-DATE

BUILD SUCCESSFUL in 917ms
36 actionable tasks: 36 up-to-date

@TWiStErRob
Copy link
Contributor

Strangely there's a rerunOnResourceChange that does a very similar test, but it tests record, not verify.

@TWiStErRob
Copy link
Contributor

I was able to repro in a test, but the weird thing is that test.inputs.dir(mergeResourcesOutputDir) is set up. Weirdly adding test.inputs.file("src/main/res/values/colors.xml") doesn't fix the problem either. After rebasing my test on #690, it seems to work.

TWiStErRob added a commit to TWiStErRob/paparazzi that referenced this issue Mar 31, 2023
@TWiStErRob
Copy link
Contributor

Can you please check if the new test in b864103 is indeed representative of this issue?

@timrijckaert
Copy link
Author

The test indeed looks representative.

TWiStErRob added a commit to TWiStErRob/paparazzi that referenced this issue Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants