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

Truth 1.1.3 to 1.1.4 upgrade broke Paparazzi #906

Closed
TWiStErRob opened this issue May 30, 2023 · 5 comments
Closed

Truth 1.1.3 to 1.1.4 upgrade broke Paparazzi #906

TWiStErRob opened this issue May 30, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@TWiStErRob
Copy link
Contributor

Description
Truth patch update caused missing method in Paparazzi. This is mostly an FYI, as I don't think there's much you can do about this bar adding a strict clause to the published metadata.

java.lang.NoSuchMethodError: 'java.util.stream.Collector com.google.common.collect.Sets.toImmutableEnumSet()'
	at com.android.resources.ResourceType.<clinit>(ResourceType.java:175)
	at app.cash.paparazzi.internal.DynamicResourceIdManager$IdProvider.<init>(DynamicResourceIdManager.kt:14)
	at app.cash.paparazzi.internal.DynamicResourceIdManager.<init>(DynamicResourceIdManager.kt:28)
	at app.cash.paparazzi.internal.PaparazziCallback.<init>(PaparazziCallback.kt:52)
	at app.cash.paparazzi.Paparazzi.prepare(Paparazzi.kt:145)
	at app.cash.paparazzi.Paparazzi$apply$statement$1.evaluate(Paparazzi.kt:116)
	at app.cash.paparazzi.agent.AgentTestRule$apply$1.evaluate(AgentTestRule.kt:17)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at com.google.testing.junit.testparameterinjector.PluggableTestRunner$ContextMethodRule$1.evaluate(PluggableTestRunner.java:429)
	... org.junit internals
	... gradle running `:test` task in a worker

Steps to Reproduce
See TWiStErRob/net.twisterrob.sun#256

Expected behavior
Just works.

Additional information:

  • Paparazzi Version: 1.2.0
  • OS: N/A
  • Compile SDK: 33
  • Gradle Version: 8.2
  • Android Gradle Plugin Version: 8.0.2

Screenshots
image

@TWiStErRob TWiStErRob added the bug Something isn't working label May 30, 2023
@TWiStErRob
Copy link
Contributor Author

TWiStErRob commented May 30, 2023

Investigation: gradlew :feature:configuration:dependencies (where :feature:configuration is one of the affected modules)

Here's a full dependency dump and diff, if anyone's interested.

before.txt
after.txt
image

Truth bumped Guava 31 to 32 and this somehow also caused Paparazzi 1.2.0's 30.1-jre to become 32.0.0-android in final dependency resolution.

According to docs the Android flavor is a subset of the full API, and it seems this Sets.toImmutableEnumSet() is on that didn't make the cut. See 32-jre vs 32-android.

I saw the recent Guava 31 to 32 bump for Paparazzi 1.3.0, I guess that will not solve this issue, because the problem is the jre vs android flavor selection.

Guava 32-android and 32-jre are exactly the same, so the truth must lie within Truth [self-amused chuckle ☺].

Diffing Truth 1.1.3 POM vs Truth 1.1.4 POM reveals

no significant changes. 🤨

image

@TWiStErRob
Copy link
Contributor Author

TWiStErRob commented May 30, 2023

More investigation: gradlew :feature:configuration:dependencyInsight --configuration=debugUnitTestRuntimeClasspath --dependency=com.google.guava:guava (using runtimeClasspath to see what artifacts exists on the classpath when running tests)

Diffing the dependency insight

insight-before.txt
insight-after.txt

image

Revealed that no-one depends on 32.0.0-jre, so it doesn't play in the dependency conflict resolution. According to Gradle rules (specifically "If both are non-numeric, the parts are compared alphabetically, in a case-sensitive manner: 1.A < 1.B < 1.a < 1.b") -jre is greater than -android, but since there are no two versions with same base version, simply the higher version wins.

This means that #900 will actually fix this issue for Paparazzi 1.3.0.

Workaround

Add -jre wherever paparazzi is used to emulate 1.3.0-compatible behavior:

dependencies {
	api(libs.test.paparazzi)
	api(libs.guava.jre) {
		because(
			"Help paparazzi transitively select the -jre flavor instead of -android. " +
			"See https://github.com/cashapp/paparazzi/issues/906."
		)
	}
}
Diffing the dependency insight with the broken version

image

shows that now both flavors exist with the same version, which helps select the "right" one for Paparazzi, and since Truth is built with the "lite" version, it'll also work with the full one.

TWiStErRob added a commit to TWiStErRob/net.twisterrob.sun that referenced this issue May 30, 2023
@TWiStErRob
Copy link
Contributor Author

This came back in Truth 1.1.5 because Paparazzi depends on 32.0.0-jre vs Truth uses 32.0.1-android, and the higher version number wins. Looks like the workaround has to stay.

@ZacSweers
Copy link
Contributor

We're seeing this even with Guava 32.1.2-jre, which supersedes the Truth version. We also exclude guava -android artifacts from our repo. I'm not sure that's the issue in this case?

@TWiStErRob
Copy link
Contributor Author

Interesting, thanks for the extra info! I'll have to take another look, because things have changed a bit since I investigated this issue. I think I still have some PRs which don't run because of this.

How do you exclude -android versioned artifacts? Dependency substitution with a condition on requested version?

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

No branches or pull requests

2 participants