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

Enable integration_tests:sparsearray tests #7006

Merged

Conversation

utzcoz
Copy link
Member

@utzcoz utzcoz commented Jan 25, 2022

After included, Android Studio will mark directories for source code correctly.

@utzcoz utzcoz requested a review from hoisie January 25, 2022 17:43
@utzcoz
Copy link
Member Author

utzcoz commented Jan 25, 2022

Oh, it also fixes "sparsearray" not found when executing ./gradlew :integration_tests:sparsearray:test.

@utzcoz
Copy link
Member Author

utzcoz commented Jan 25, 2022

Oh, looks like ./gradlew :integration_tests:sparsearray:test don't run SparseArrayTest. I will update this PR to try to fix it.

@utzcoz utzcoz changed the title Add integration_tests multidex and sparsearray to settings.gradle Enable integration_tests:sparsearray tests Jan 25, 2022
@utzcoz
Copy link
Member Author

utzcoz commented Jan 25, 2022

Oh, looks like ./gradlew :integration_tests:sparsearray:test don't run SparseArrayTest. I will update this PR to try to fix it.

After enabling those tests, looks like they can't find set method when running tests under 31.

@hoisie
Copy link
Contributor

hoisie commented Jan 25, 2022

cc @JuliaSullivanGoogle

I believe as-is the integration_tests:sparsearray test will fail, because Robolectric is always configured to use the preinstrumented jars, and because the sparsearray.set support is a change in the instrumentation, it won't be used.

@JuliaSullivanGoogle is working on a change that adds a system property to disable preinstrumented jars, which will be used in the GitHub CI. It might slow things down a small amount, but not significantly. When we do another release and release a new set of preinstrumented jars, we can switch back to using them in GitHub CI.

@utzcoz
Copy link
Member Author

utzcoz commented Jan 25, 2022

because the sparsearray.set support is a change in the instrumentation, it won't be used.

Oh, I forget preinstrumented jar. It actually failed on my local machine, and I just push related commit to this PR for testing and reviewing.

@JuliaSullivanGoogle is working on a change that adds a system property to disable preinstrumented jars, which will be used in the GitHub CI.

That's cool. It's very useful for GitHub CI and local testing/debugging. I will keep this PR to draft state, and wait @JuliaSullivanGoogle 's system property to disable preinstrumented jar.

@utzcoz utzcoz marked this pull request as draft January 25, 2022 18:27
copybara-service bot pushed a commit that referenced this pull request Feb 14, 2022
…fault.

Some changes only occur when a new version of the jars are released. This will ensure all changes are tested when copied over.

This will allow integration_tests:sparsearray to run in Gradle before
a new version of preinstrumented jars are released.

Related to #7006

PiperOrigin-RevId: 428547156
copybara-service bot pushed a commit that referenced this pull request Feb 14, 2022
…fault.

Some changes only occur when a new version of the jars are released. This will ensure all changes are tested when copied over.

This will allow integration_tests:sparsearray to run in Gradle before
a new version of preinstrumented jars are released.

Related to #7006

PiperOrigin-RevId: 428547156
hoisie pushed a commit that referenced this pull request Mar 8, 2022
…fault.

Some changes only occur when a new version of the jars are released. This will ensure all changes are tested when copied over.

This will allow integration_tests:sparsearray to run in Gradle before
a new version of preinstrumented jars are released.

Related to #7006

PiperOrigin-RevId: 428547156
hoisie pushed a commit that referenced this pull request Mar 8, 2022
…fault.

Some changes only occur when a new version of the jars are released. This will ensure all changes are tested when copied over.

This will allow integration_tests:sparsearray to run in Gradle before
a new version of preinstrumented jars are released.

Related to #7006

PiperOrigin-RevId: 428547156
@utzcoz
Copy link
Member Author

utzcoz commented Mar 9, 2022

I will rebase and check it again, today.

@utzcoz utzcoz force-pushed the include-new-projects-in-settings.gradle branch from 622c375 to 5d49802 Compare March 9, 2022 13:29
@utzcoz
Copy link
Member Author

utzcoz commented Mar 9, 2022

Very strange that some tests are skipped:

org.robolectric.integrationtests.sparsearray.SparseArraySetTest > testSparseArrayBracketOperator_callsSetMethodPreApi31[29] SKIPPED

org.robolectric.integrationtests.sparsearray.SparseArraySetTest > testSparseArrayBracketOperator_callsSetMethodPreApi31[30] SKIPPED

org.robolectric.integrationtests.sparsearray.SparseArraySetTest > testSparseArrayBracketOperator_callsSetMethodPreApi31 SKIPPED

org.robolectric.integrationtests.sparsearray.SparseArraySetTest > testSparseArraySetFunction_callsSetMethodPreApi31[29] SKIPPED

org.robolectric.integrationtests.sparsearray.SparseArraySetTest > testSparseArraySetFunction_callsSetMethodPreApi31[30] SKIPPED

org.robolectric.integrationtests.sparsearray.SparseArraySetTest > testSparseArraySetFunction_callsSetMethodPreApi31 SKIPPED

@utzcoz utzcoz marked this pull request as ready for review March 9, 2022 14:09
@utzcoz
Copy link
Member Author

utzcoz commented Mar 9, 2022

Okay, I found the reason: the current config uses legacy resource, and it is not supported after P with thrown exception from SandboxTestRunner#getSandbox, and tests targeted to those newer APIs will be skipped. We should update build.gradle to normal app template with includeAndroidResource enabled.

@utzcoz utzcoz marked this pull request as draft March 9, 2022 14:36
@utzcoz utzcoz force-pushed the include-new-projects-in-settings.gradle branch from 5d49802 to 33265c3 Compare March 9, 2022 14:58
@utzcoz utzcoz marked this pull request as ready for review March 9, 2022 14:58
@utzcoz
Copy link
Member Author

utzcoz commented Mar 9, 2022

Okay, I found the reason: the current config uses legacy resource, and it is not supported after P with thrown exception from SandboxTestRunner#getSandbox, and tests targeted to those newer APIs will be skipped. We should update build.gradle to normal app template with includeAndroidResource enabled.

Fixed by transferring to use Robolectric's Android module plugin to enable includeAndroidResource. When I trying to use robolectric.usePreinstrumentedJars locally, I found I should execute ./gradlew clean before running ./gradlew :integration_tests:sparsearray:test -Drobolectric.usePreinstrumentedJars=false. I also want to add it to sparsearray's build.gradle, but it doesn't work correctly with can't find android-all dependency error, although I execute ./gradlew clean firstly. I have confirmed that I have installed normal android-all locally. @JuliaSullivanGoogle do you have encountered similar problem maybe caused by cache?

Anyway, I think this PR is ready for reviewing. cc /@hoisie .

@utzcoz utzcoz force-pushed the include-new-projects-in-settings.gradle branch from 33265c3 to 6a25900 Compare March 13, 2022 15:14
@utzcoz
Copy link
Member Author

utzcoz commented Mar 14, 2022

It's strange: actions/checkout doesn't clone Robolectric with recursive although I have added config for it. The output shows that it force no-recursive-module: /usr/local/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +cdefd9fce1399da7cfdff61a384a38233b9f2072:refs/remotes/pull/7006/merge.

@utzcoz utzcoz force-pushed the include-new-projects-in-settings.gradle branch 4 times, most recently from f2a625f to e32adea Compare March 20, 2022 13:59
@utzcoz utzcoz marked this pull request as draft March 20, 2022 14:31
@utzcoz
Copy link
Member Author

utzcoz commented Mar 20, 2022

Looks like this PR triggers ICU building for instrumentation-tests on Emulator, and it failed with some reasons.

@utzcoz utzcoz force-pushed the include-new-projects-in-settings.gradle branch from 3223729 to 03bbc7e Compare March 20, 2022 14:42
@utzcoz
Copy link
Member Author

utzcoz commented Mar 20, 2022

ERROR:D8: Cannot fit requested classes in a single dex file (# methods: 69779 > 65536)

multidex error raised for sparsearray.

@utzcoz utzcoz force-pushed the include-new-projects-in-settings.gradle branch 2 times, most recently from 13109ff to b813d91 Compare March 21, 2022 01:43
@utzcoz utzcoz marked this pull request as ready for review March 21, 2022 04:40
@utzcoz
Copy link
Member Author

utzcoz commented Mar 21, 2022

The api project(":robolectric") adds nativeruntime to sparsearray module, and it triggers building for ICU. This PR also reduce full robolectric dependency with shadowapi only. I think it can be review again @hoisie @JuliaSullivanGoogle .

BTW, will i4 version of preinstrumented jars published recent days?

@hoisie
Copy link
Contributor

hoisie commented Mar 21, 2022

@utzcoz made #7156 to bump preinstrumented jars version

@hoisie
Copy link
Contributor

hoisie commented Mar 21, 2022

@utzcoz preinstrumented jars got bumped, also 4.8-alpha-1 is released, so this should be unblocked?

@utzcoz
Copy link
Member Author

utzcoz commented Mar 21, 2022

@utzcoz preinstrumented jars got bumped, also 4.8-alpha-1 is released, so this should be unblocked?

Yup. I will rebase and rerun checking today.

After included, Android Studio will mark directories for source code
correctly.

Signed-off-by: utzcoz <utzcoz@outlook.com>
Signed-off-by: utzcoz <utzcoz@outlook.com>
Signed-off-by: utzcoz <utzcoz@outlook.com>
@utzcoz utzcoz force-pushed the include-new-projects-in-settings.gradle branch from b813d91 to e823a12 Compare March 21, 2022 10:12
@utzcoz utzcoz merged commit c278347 into robolectric:master Mar 21, 2022
@utzcoz utzcoz deleted the include-new-projects-in-settings.gradle branch March 21, 2022 11:20
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.

None yet

2 participants