Skip to content

Do not initialize when parse is already initialized #640

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

Merged
merged 3 commits into from
Apr 26, 2017

Conversation

hermanliang
Copy link
Contributor

If Parse is initialized in the application.
An IllegalStateException might be thrown when unit testing with Robolectric framework.

for example
MyApplication.java

public class MyApplication extends Application {
  @Override
  public void onCreate() {
    super.onCreate();
    Parse.initialize(this);
  }
}

SomeTest.java

@RunWith(RobolectricTestRunner.class)
@Config(constants = BuildConfig.class)
public class SomeTest {

  @Test
  public void test1() {
    ...
  }

  @Test
  public void test2() {
    ...
  }

}

Will get IllegalStateException: ParsePlugins is already initialized because of Robolectric setup application twice in the same testing session.

I think it would be better not to throw an exception when init parse more than once, just issue a warning log and do nothing.

It does not seems to be Robolectric's issue.
robolectric/robolectric#595
robolectric/robolectric#1981
robolectric/robolectric#2456

@codecov
Copy link

codecov bot commented Apr 20, 2017

Codecov Report

Merging #640 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #640      +/-   ##
============================================
- Coverage     52.85%   52.84%   -0.02%     
  Complexity     1675     1675              
============================================
  Files           131      131              
  Lines         10142    10145       +3     
  Branches       1408     1408              
============================================
  Hits           5361     5361              
- Misses         4338     4340       +2     
- Partials        443      444       +1
Impacted Files Coverage Δ Complexity Δ
Parse/src/main/java/com/parse/Parse.java 49.76% <0%> (-0.7%) 23 <1> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e82808e...d8485f8. Read the comment docs.

@Jawnnypoo
Copy link
Member

Are we positive that the error was not being throw intentionally there to prevent some other side effects of having it init twice?

Side note: if you extend ResetPluginsParseTest it will fix this problem too.

@hermanliang
Copy link
Contributor Author

hermanliang commented Apr 20, 2017

@Jawnnypoo I think the behavior is not changed because this PR is still not allowing parse to init more than once.
BTW, ResetPluginsParseTest is in the SDK unit tests cannot be extended in application test class, but thanks anyway.

@natario1
Copy link
Contributor

Since this is an application/developer issue, maybe it’s better to do this in Parse.initialize ? Something like if (isInitialized()) return.

ParsePlugins is private, I think the exception might come useful when writing internal tests for the SDK (ensures everything is tear down correctly, you know where you’re failing etc).

@hermanliang
Copy link
Contributor Author

Thanks @natario1
You're right, we should keep the behavior in ParsePlugin.
Checking in Parse.initialized is a good idea. 😄

@hermanliang hermanliang changed the title Do not throw an exception when init parse more than once Do not initialize when parse is already initialized Apr 24, 2017
@rogerhu rogerhu merged commit 2e15813 into parse-community:master Apr 26, 2017
@hermanliang hermanliang deleted the multiple_init_warn branch April 27, 2017 00:48
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.

4 participants