-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Remove testDebugUnitTest from the dependencies of the simulator task
#10413
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully removes the dependency on the testDebugUnitTest task from the simulator task, ensuring that only the assembleDebug task is executed to provide the necessary APK for the simulator. This change aligns with the stated objective and improves the efficiency of the simulation task.
| mainClass.set(MAIN_CLASS) | ||
| args = listOf(resourceApkFile.absolutePath) | ||
| dependsOn(testTaskName, "assembleDebug") | ||
| dependsOn("assembleDebug") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| mainClass.set(MAIN_CLASS) | ||
| args = listOf(resourceApkFile.absolutePath) | ||
| dependsOn(testTaskName, "assembleDebug") | ||
| dependsOn("assembleDebug") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, this simulate task requires debutTest task finished?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I know is that "apk" in test environment is liek "xxx._ap", and it is generated by test task. If you we remove this task dependency, how can we ensure the "apk" file is generated ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I thought it was looking for the app APK, not the test APK.
So maybe, dependsOn should include the following. What do you think?
Lines 45 to 46 in 7a3b64d
| // Find the 'apk-for-local-test.ap_' file | |
| val targetTask = project.tasks.getByName("packageDebugUnitTestForUnitTest") |
dependsOn(targetTask, "assembleDebug"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to run the packageDebugUnitTestForUnitTest locally, but it was not found (using AGP 8.10.1). Maybe we need to find a better way to generate the apk-for-local-test.ap_ file (as suggested in #10382, suggestion 7)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, the issue was that I didn't set unitTests.isIncludeAndroidResources = true.
I'll add a tip here if the task is not found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I just upgraded to AGP 8.10.1 and the task packageDebugUnitTestForUnitTest exists for me in a very simple project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MGaetan89 that sounds great, thanks. We should also consider failing Robolectric tests if the ap_ is not supplied as well, but that can be a separate thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hoisie I've pushed the changes. Can you try again?
The build is already failed when the .ap_ is not find:
Lines 47 to 51 in 7a3b64d
| val resourceApkFile = | |
| targetTask.outputs.files.find { it.name.endsWith(".ap_") } | |
| ?: throw GradleException( | |
| "Could not find an .ap_ file in the outputs of task '${targetTask.name}'. " | |
| ) |
Or do you have something else in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was thinking that RobolectricTestRunner could check for the ap_ file to be supplied, to force people to set unitTests.isIncludeAndroidResources = true for tests as well (not just the simulator). But that can be a separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey this PR works for me in my simple projects. I think it is a step in the right direction.
70261d0 to
1a64763
Compare
This commit removes the dependency on the test task when running the `simulator` task. We only need the `assembleDebug` task, to have an APK to run in the simulator. Fixes robolectric#10412
|
Thank you @MGaetan89 for quickly resolving this issue, I'll CP these fixes and push out 4.15. |
This commit removes the dependency on the test task when running the
simulatortask.We only need the
assembleDebugtask, to have an APK to run in the simulator.Fixes #10412