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

Replace testharness with DeviceConfigurationOverride #1221

Conversation

sanao1006
Copy link
Contributor

What I have done and why
Replace testharness with DeviceConfigurationOverride.
This is because accompanist/testharness is deprecated.

ref: https://google.github.io/accompanist/testharness/

**This library is deprecated, with a superseding version in androidx.compose.ui.test.

Fixes #1201

@sanao1006 sanao1006 marked this pull request as ready for review February 21, 2024 02:18
@sanao1006 sanao1006 changed the title [Refactor]: Replace testharness with DeviceConfigurationOverride Replace testharness with DeviceConfigurationOverride Feb 21, 2024
@alexvanyo alexvanyo self-requested a review February 21, 2024 10:32
Copy link
Contributor

@alexvanyo alexvanyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment on the version, and it looks like CI is failing. Do the tests pass locally for you?

gradle/libs.versions.toml Outdated Show resolved Hide resolved
- rename `androidxUiTest` to `androidxComposeUiTest`
- update version
@sanao1006
Copy link
Contributor Author

I have no idea what causes the java.lang.NoSuchMethodError: No static method ... error on the Instrument test.
https://github.com/android/nowinandroid/actions/runs/8005993030/job/21873328665?pr=1221

java.lang.NoSuchMethodError: No static method getAllSemanticsNodes(Landroidx/compose/ui/semantics/SemanticsOwner;ZZ)Ljava/util/List; in class Landroidx/compose/ui/semantics/SemanticsOwnerKt; or its super classes (declaration of 'androidx.compose.ui.semantics.SemanticsOwnerKt' appears in /data/app/~~e8B0qmOuMalVgsWU1kGVrg==/com.google.samples.apps.nowinandroid.demo.debug-9YEXqTHbjJ2K4mFxOY4hUw==/base.apk!classes11.dex)
java.lang.NoSuchMethodError: No static method CompositionLocalProvider(Landroidx/compose/runtime/ProvidedValue;Lkotlin/jvm/functions/Function2;Landroidx/compose/runtime/Composer;I)V in class Landroidx/compose/runtime/CompositionLocalKt; or its super classes (declaration of 'androidx.compose.runtime.CompositionLocalKt' appears in /data/app/~~e8B0qmOuMalVgsWU1kGVrg==/com.google.samples.apps.nowinandroid.demo.debug-9YEXqTHbjJ2K4mFxOY4hUw==/base.apk)

Is it due to dependence?
Do you have any idea what it is?

@alexvanyo
Copy link
Contributor

It seems like the 1.7.x version of the ui-test is expecting methods from a newer version of the main ui library. Due to how main and test dependencies resolve, sometimes adding a new dependency in the test library won't transitively bump the main ui library to the needed version.

#1220 will update our Compose dependencies from 1.5.x to 1.6.x which may resolve the issue. If it doesn't, then the other option is to update to the 1.7.x version of Compose throughout the project so that the version is aligned between the main dependencies and the test dependencies.

@dturner
Copy link
Collaborator

dturner commented Feb 26, 2024

Hi @sanao1006, many thanks for submitting this PR. Unfortunately I already had a branch with this change in it ready to go (plus some slight cosmetic differences e.g. version catalog naming) so I will merge this PR instead when we're ready.

@dturner
Copy link
Collaborator

dturner commented Feb 26, 2024

I do have some questions/comments on @alexvanyo's comment though:

Due to how main and test dependencies resolve, sometimes adding a new dependency in the test library won't transitively bump the main ui library to the needed version

Could you elaborate more on this? Why doesn't ui-test:1.7.x have a strict dependency on ui:1.6.x minimum?

#1220 will update our Compose dependencies from 1.5.x to 1.6.x which may resolve the issue.

Yes, it does resolve the issue.

Would you consider this behaviour a bug? It certainly seems like one if you can add a library and have compilation fail in an non-obvious way.

@alexvanyo
Copy link
Contributor

ui-test:1.7.x does have a strict dependency on at least ui:1.6.x, but that transitive update to ui is being ignored in favor of the dependency from main configuration, since we are only declaring the ui-test dependency in test.

This has come up before such as in android/android-test#1315 (comment)

This definitely feels buggy - it's not clear at all why the version is "fixed" to the one in the main source set instead of being bumped by the one in the test source set.

We should follow-up with AGP folks to use this as an example of whether this behavior can be improved.

@sanao1006
Copy link
Contributor Author

sanao1006 commented Feb 27, 2024

Hi @dturner

Hi @sanao1006, many thanks for submitting this PR. Unfortunately I already had a branch with this change in it ready to go (plus some slight cosmetic differences e.g. version catalog naming) so I will merge this PR instead when we're ready.

I understand. Thanks for contacting me!

It is OK to close this PR when your discussion is complete!

@alexvanyo alexvanyo mentioned this pull request Mar 5, 2024
@alexvanyo
Copy link
Contributor

Closing this PR - #1140 has more info about the resolution restrictions as reference.

@alexvanyo alexvanyo closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace testharness with DeviceConfigurationOverride
3 participants