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

Fix SandboxClassLoader to obey parent-first contract #2905

Merged
merged 2 commits into from Feb 22, 2017

Conversation

Projects
None yet
4 participants
@xian
Member

xian commented Feb 11, 2017

Fixes #2208.
Fixes #2677.

@xian xian added this to the 3.3 milestone Feb 11, 2017

@xian xian added the defect label Feb 11, 2017

@xian xian requested a review from jongerrish Feb 22, 2017

Fix SandboxClassLoader to obey parent-first contract.
Supports PowerMock and experimental Mockito mode which enables mocking final classes/methods.
Supports Package.getPackage().
@jongerrish

Sweet, one of the longest running Robolectric issues fixed? :-D

// See https://github.com/robolectric/robolectric/issues/521
if (name.startsWith("android.R")) {
// android.R and com.android.internal.R classes must be loaded from the framework jar
if (name.matches("(android|com\\.android\\.internal)\\.R(\\$.+)?")) {

This comment has been minimized.

@jongerrish

jongerrish Feb 22, 2017

Contributor

name.startsWith("android.R) || name.startsWith("com.android.internal.R") may be more clear here.

@jongerrish

jongerrish Feb 22, 2017

Contributor

name.startsWith("android.R) || name.startsWith("com.android.internal.R") may be more clear here.

This comment has been minimized.

@xian

xian Feb 22, 2017

Member

We're pretty confident there'll never be an android.Riboflavin or com.android.internal.Roustabout or something? :-)

@xian

xian Feb 22, 2017

Member

We're pretty confident there'll never be an android.Riboflavin or com.android.internal.Roustabout or something? :-)

This comment has been minimized.

@jongerrish

jongerrish Feb 22, 2017

Contributor

Yeah, I think so, although, you can never say never :-D I think this will go away when we check the jar as the source of truth for what to acquire though anyway, so I'm not fussed about this :-)

@jongerrish

jongerrish Feb 22, 2017

Contributor

Yeah, I think so, although, you can never say never :-D I think this will go away when we check the jar as the source of truth for what to acquire though anyway, so I'm not fussed about this :-)

ReflectionHelpers.setStaticField(androidBuildVersionClass, "SDK_INT", sdkConfig.getApiLevel());
ReflectionHelpers.setStaticField(androidBuildVersionClass, "RELEASE", sdkConfig.getAndroidVersion());
PackageResourceTable systemResourceTable = sdkEnvironment.getSystemResourceTable(getJarResolver());
PackageResourceTable systemResourceTable = (sdkEnvironment).getSystemResourceTable(getJarResolver());

This comment has been minimized.

@jongerrish

jongerrish Feb 22, 2017

Contributor

extraneous parens around (sdkEnvironment) ?

@jongerrish

jongerrish Feb 22, 2017

Contributor

extraneous parens around (sdkEnvironment) ?

This comment has been minimized.

@xian

xian Feb 22, 2017

Member

Oops, fixed!

@xian

xian Feb 22, 2017

Member

Oops, fixed!

@@ -107,6 +107,11 @@ public void testVersionConfiguration() {
.isEqualTo("4.4_r1");
}
@Test public void assertThatz() throws Exception {

This comment has been minimized.

@jongerrish

jongerrish Feb 22, 2017

Contributor

Want to do this more? I'm guessing that it is checking that things are loaded from the right classloader, but I'm a little confused as its an assert for a primitive rather than an object?

@jongerrish

jongerrish Feb 22, 2017

Contributor

Want to do this more? I'm guessing that it is checking that things are loaded from the right classloader, but I'm a little confused as its an assert for a primitive rather than an object?

This comment has been minimized.

@xian

xian Feb 22, 2017

Member

Fixed the test name. I came across a classloader conflict because org.junit.Assert loads some hamcrest things.

@xian

xian Feb 22, 2017

Member

Fixed the test name. I came across a classloader conflict because org.junit.Assert loads some hamcrest things.

@xian

This comment has been minimized.

Show comment
Hide comment
@xian

xian Feb 22, 2017

Member

Dogfood green! :-)

Member

xian commented Feb 22, 2017

Dogfood green! :-)

@xian xian merged commit c3a3b77 into master Feb 22, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@jongerrish

This comment has been minimized.

Show comment
Hide comment
@jongerrish

jongerrish Feb 24, 2017

Contributor
Contributor

jongerrish commented Feb 24, 2017

@YorkShen

This comment has been minimized.

Show comment
Hide comment
@YorkShen

YorkShen Mar 2, 2017

Well done.

YorkShen commented Mar 2, 2017

Well done.

@@ -75,8 +75,7 @@
private final SdkPicker sdkPicker;
private final ConfigMerger configMerger;
private TestLifecycle<Application> testLifecycle;
private DependencyResolver dependencyResolver;
private transient DependencyResolver dependencyResolver;

This comment has been minimized.

@bangarharshit

bangarharshit Nov 11, 2017

Why is dependencyResolver transient?

@bangarharshit

bangarharshit Nov 11, 2017

Why is dependencyResolver transient?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment