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.

Sign up for GitHub

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

Jump to bottom

Support partial AppComponentFactory in Robolectric #8004

Merged
merged 1 commit into from Apr 8, 2023
Merged

Support partial AppComponentFactory in Robolectric #8004

merged 1 commit into from Apr 8, 2023

Conversation

utzcoz
Copy link
Member

@utzcoz utzcoz commented Mar 4, 2023 •
edited

Support to use AppComponentFactory in Robolectric from SDK 28. This PR only supports BroadcastReceiver with custom AppComponentFactory. In Robolectric, developers use Robolectric#setupService or Robolectric#buildService to initialize Service instance, and these methods are static, so it's difficult to leverage custom AppComponentFactory to initialize Service instance with custom constructor. And I don't find proper usage scenarios to use custom AppComponentFactory to initialize ContentProvider, Application, Activity and ClassLoader. So I leave them as not implemented state.

Fixes: #7581.

@utzcoz utzcoz mentioned this pull request Mar 4, 2023
Closed
@utzcoz utzcoz force-pushed the support-AppComponentFactory branch from 775b658 to d80410a Compare March 4, 2023 09:12
@utzcoz utzcoz changed the title Support AppComponentFactory in Robolectric Support partial AppComponentFactory in Robolectric Mar 4, 2023
@utzcoz utzcoz marked this pull request as ready for review March 4, 2023 11:31
@utzcoz
Copy link
Member Author

utzcoz commented Mar 4, 2023

@hoisie Could you help to review this PR? Thanks.
cc @ZacSweers for extra reviewing and testing.

@utzcoz utzcoz force-pushed the support-AppComponentFactory branch from d80410a to 2ba53a4 Compare March 4, 2023 12:07
@utzcoz
Copy link
Member Author

utzcoz commented Mar 10, 2023

@hoisie Could you help to give a look at this PR? Thanks.

@utzcoz utzcoz force-pushed the support-AppComponentFactory branch from 2ba53a4 to db9104f Compare March 11, 2023 09:39
@utzcoz
Copy link
Member Author

utzcoz commented Mar 15, 2023

@hoisie What about this PR? I also want to know whether it can be merged into 4.10.

@hoisie
Copy link
Contributor

hoisie commented Mar 15, 2023

Yeah it'll definitely be merged before 4.10. Let me run some validation tests for it right now.

@hoisie
Copy link
Contributor

hoisie commented Mar 16, 2023

When I ran this against some tests internally at Google I saw a non-trivial amount of failures. I'll take a closer look at a random sample and see what's going on.

@hoisie
Copy link
Contributor

hoisie commented Mar 16, 2023

Just as an FYI, there will have to be some kind of way to enable or disable this feature (e.g. a system property, like robolectric.enableAppComponentFactory). I can see situations where BroadcastReceiver are initialized in a different way, and maybe that way causes tests to throw an Exception.

@utzcoz
Copy link
Member Author

utzcoz commented Mar 16, 2023

@hoisie Could you help to provide examples for these edge cases? I can improve the current implementation based on these examples.

@utzcoz utzcoz force-pushed the support-AppComponentFactory branch from db9104f to 7e506e7 Compare March 19, 2023 03:48
@utzcoz
Copy link
Member Author

utzcoz commented Mar 19, 2023

@hoisie I applied above suggestions, PTAL.

@utzcoz utzcoz force-pushed the support-AppComponentFactory branch from 7e506e7 to b431f46 Compare April 5, 2023 03:26
@utzcoz
Copy link
Member Author

utzcoz commented Apr 6, 2023

@hoisie I applied above suggestions, PTAL.

@hoisie
Copy link
Contributor

hoisie commented Apr 7, 2023

@utzcoz doing another round of tests on this, hopefully we can merge it tomorrow and then release 4.10.

@utzcoz
Copy link
Member Author

utzcoz commented Apr 7, 2023

@hoisie If you can wait, I can fix some issues of this PR tomorrow(Satuarday, East Eighth District TZ). Otherwise you can release 4.10, and we can port it to 4.10.1.

@utzcoz utzcoz force-pushed the support-AppComponentFactory branch from b431f46 to 0cbd98f Compare April 8, 2023 02:46
@utzcoz
Support partial AppComponentFactory in Robolectric
531507e 
Support to use AppComponentFactory in Robolectric from SDK 28.
This CL only supports BroadcastReceiver with custom AppComponentFactory.
In Robolectric, developers use Robolectric#setupService or
Robolectric#buildService to initialize Service instance,
and these methods are static, so it's difficult to leverage
custom AppComponentFactory to initialize Service instance
with custom constructor. And I don't find proper usage
scenarios to use custom AppComponentFactory to initialize
ContentProvider, Application, Activity and ClassLoader.
So I leave them as not implemented state.

Signed-off-by: utzcoz <utzcoz@outlook.com>
@utzcoz utzcoz force-pushed the support-AppComponentFactory branch from 0cbd98f to 531507e Compare April 8, 2023 02:53
@utzcoz
Copy link
Member Author

utzcoz commented Apr 8, 2023

@hoisie Updated, PTAL.

@utzcoz utzcoz merged commit d2340ee into robolectric:master Apr 8, 2023
13 checks passed
@utzcoz utzcoz deleted the support-AppComponentFactory branch April 8, 2023 03:50
@ZacSweers ZacSweers mentioned this pull request Jun 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoisie hoisie hoisie approved these changes

@brettchabot brettchabot Awaiting requested review from brettchabot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support AppComponentFactory or ignore BroadcastReceivers with custom constructors
2 participants
@utzcoz @hoisie