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

feat: update various dependencies #1172

Merged
merged 12 commits into from
Jun 10, 2022
Merged

Conversation

rommansabbir
Copy link
Member

@rommansabbir rommansabbir commented Jun 3, 2022

New Pull Request Checklist

Issue Description

Related issue: #1163

Approach

According to #1163 (comment):

  • Set upper range for FB Login SDK to >=13.1.0 <14.0.0 in this PR
  • Merge this PR with a major version increase, as it may be a breaking change. The FB Login SDK is currently added as api, so it can transitively accessed if the developer adds the Parse SDK also as api to their app. Since the FB SDK has some braking changes with 13.x, we need to consider that also a breaking change for the Parse SDK.

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Jun 3, 2022

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@rommansabbir
Copy link
Member Author

@mtrezza CI is failing.

com.parse.ParseCorePluginsTest > testQueryControllerDefaultImpl FAILED
    java.lang.AssertionError at ParseCorePluginsTest.java:36
[Tests passed: 14 of 14 tests - my local machine - CI Fail]

com.parse.ParseDecoderTest > testCompleteness FAILED
    java.lang.AssertionError at ParseDecoderTest.java:270
[Tests passed: 23 of 23 tests - my local machine - CI Fail]

com.parse.ParseDecoderTest > testCompletenessOfIncludedParseObject FAILED
    java.lang.AssertionError at ParseDecoderTest.java:294
[Tests passed: 23 of 23 tests -  local machine - CI Fail]

Any idea on this failing?

@mtrezza
Copy link
Member

mtrezza commented Jun 3, 2022

Does it pass locally? If so, it may be the java version or another env version maybe.

@rommansabbir rommansabbir force-pushed the fix/fb branch 2 times, most recently from 97a11fe to d55fa89 Compare June 4, 2022 07:43
@rommansabbir
Copy link
Member Author

rommansabbir commented Jun 4, 2022

Does it pass locally? If so, it may be the java version or another env version maybe.

@mtrezza @mman any idea on how to access this file file:///home/runner/work/Parse-SDK-Android/Parse-SDK-Android/parse/build/reports/tests/testDebugUnitTest/index.html, so that I can check the stacktrace.

I ran gradle clean testDebugUnitTest command and checked the test reports. 3 test cases failing on both CI, Local but when testing through Run no errors at all.

Tried to findout the root cause but failed.

Any idea how to fix this?

@mtrezza
Copy link
Member

mtrezza commented Jun 4, 2022

If you want to inspect that in the CI, you would have to modify the workflow file and output the file in the console. But since you can reproduce this locally, that may not be necessary. Why can't you access Parse-SDK-Android/parse/build/reports/tests/testDebugUnitTest/index.html locally? Is there no file?

@rommansabbir
Copy link
Member Author

If you want to inspect that in the CI, you would have to modify the workflow file and output the file in the console. But since you can reproduce this locally, that may not be necessary. Why can't you access Parse-SDK-Android/parse/build/reports/tests/testDebugUnitTest/index.html locally? Is there no file?

Actually I did founds those issues and solved too but when tested by "Run" it doesn’t work but working fine in gradle clean testDebugUnitTest.

Need more time to investigate the issues, it would be more helpful if you or @mman could look at the issue too alongside with me.

@rommansabbir
Copy link
Member Author

@mtrezza The issues arise when we are performing a gradle clean testDebugUnitTest, which remove the buildDir folder.
So there is a possibilites that some of the component's build files get removed duing the clean operations, as all of the components is inter-connected to each others.

Here is the stacktraces for all failed tests.

  • ParseCorePluginsTest
testQueryControllerDefaultImpl

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at com.parse.ParseCorePluginsTest.testQueryControllerDefaultImpl(ParseCorePluginsTest.java:37)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:591)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$0(SandboxTestRunner.java:274)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:88)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
  • ParseDecoderTest
testCompleteness

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertFalse(Assert.java:65)
	at org.junit.Assert.assertFalse(Assert.java:75)
	at com.parse.ParseDecoderTest.testCompleteness(ParseDecoderTest.java:270)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:258)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:591)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$0(SandboxTestRunner.java:274)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:88)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

testCompletenessOfIncludedParseObject

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertFalse(Assert.java:65)
	at org.junit.Assert.assertFalse(Assert.java:75)
	at com.parse.ParseDecoderTest.testCompletenessOfIncludedParseObject(ParseDecoderTest.java:294)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:258)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:591)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$0(SandboxTestRunner.java:274)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:88)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

@mtrezza
Copy link
Member

mtrezza commented Jun 4, 2022

It could also be helpful to go back and see with which commit this issue started to occur. If I recall correctly, we had all tests passing not so long ago.

@rommansabbir
Copy link
Member Author

Okay. I will go through the commit changes.

@rommansabbir
Copy link
Member Author

rommansabbir commented Jun 7, 2022

AGPBI: {"kind":"warning","text":"Software Components will not be created automatically for Maven publishing from Android Gradle Plugin 8.0. To opt-in to the future behavior, set the Gradle property android.disableAutomaticComponentCreation=true in the gradle.properties file or use the new publishing DSL.","sources":[{}]}

@mtrezza

…pdated, Junit-Jupiter dependency added, new Test task added to use JUnit Platform
@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #1172 (dab2045) into master (ec7bd03) will decrease coverage by 67.31%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #1172       +/-   ##
============================================
- Coverage     67.31%   0.00%   -67.32%     
============================================
  Files           122     122               
  Lines          9962    9962               
  Branches       1343    1343               
============================================
- Hits           6706       0     -6706     
- Misses         2735    9962     +7227     
+ Partials        521       0      -521     
Impacted Files Coverage Δ
parse/src/main/java/com/parse/ParseRole.java 0.00% <0.00%> (-100.00%) ⬇️
parse/src/main/java/com/parse/ParseCloud.java 0.00% <0.00%> (-100.00%) ⬇️
parse/src/main/java/com/parse/ParseException.java 0.00% <0.00%> (-100.00%) ⬇️
parse/src/main/java/com/parse/ParseTextUtils.java 0.00% <0.00%> (-100.00%) ⬇️
parse/src/main/java/com/parse/PointerEncoder.java 0.00% <0.00%> (-100.00%) ⬇️
parse/src/main/java/com/parse/ParseDateFormat.java 0.00% <0.00%> (-100.00%) ⬇️
...rse/src/main/java/com/parse/ParseFileHttpBody.java 0.00% <0.00%> (-100.00%) ⬇️
...rse/src/main/java/com/parse/ParseSetOperation.java 0.00% <0.00%> (-100.00%) ⬇️
...se/src/main/java/com/parse/OfflineObjectStore.java 0.00% <0.00%> (-100.00%) ⬇️
...se/src/main/java/com/parse/http/ParseHttpBody.java 0.00% <0.00%> (-100.00%) ⬇️
... and 104 more

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 c658995...dab2045. Read the comment docs.

@rommansabbir
Copy link
Member Author

rommansabbir commented Jun 7, 2022

@mtrezza can you take a look at the codecov/project — 0.00% (target 65.00%) fail?

@mtrezza
Copy link
Member

mtrezza commented Jun 7, 2022

The codecov seems to be a temporary issue:

Github API rate limit error: Forbidden Check on Codecov's status

Not sure whether we (Parse Platform) are hitting the API limit, or GitHub in general. We'll ignore this check for merge in this PR.

I see all other checks pass! That's amazing, so did you find what made it fail previously? This?

@rommansabbir
Copy link
Member Author

I see all other checks pass! That's amazing, so did you find what made it fail previously? This?

I tried a lot of solutions but ended up with dependency issue solving. testImplementation of JUnit Jupiter & Test Task to support jUnitPlatform, that's it. Easy fix but took time 😅

@rommansabbir rommansabbir requested a review from mtrezza June 8, 2022 04:39
@mtrezza
Copy link
Member

mtrezza commented Jun 8, 2022

A PR can only address and describe a single issue, so we need to consolidate the PR title.

I suggest:

feat: update various dependencies

BREAKING CHANGE: The Facebook Login SDK is upgraded to 13.x; this is a transitive dependency, so if you are directly calling the Facebook Login SDK in your app then this may be a braking change; this release of the Parse Android SDK adds a version range to the Facebook Login SDK dependency for correct gradle dependency resolving

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good, great work @rommansabbir! Let's get just another review before merge.

@mtrezza mtrezza requested a review from a team June 8, 2022 09:59
@mtrezza mtrezza changed the title fix: fb login fail from embedded browser (FB SDK), feat: update gradle dependencies feat: update various dependencies; add Facebook Login SDK dependency version range for correct gradle dependency resolving Jun 8, 2022
@mtrezza mtrezza changed the title feat: update various dependencies; add Facebook Login SDK dependency version range for correct gradle dependency resolving feat: update various dependencies Jun 8, 2022
@rommansabbir
Copy link
Member Author

Any update on the review @parse-community/android-sdk-review?

Copy link
Contributor

@azlekov azlekov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the idea behind adding jupiter?

I remember that Android is compatible with junit but not jupiter. If that's not the case, may I ask for a reference? And why having both, the tests should be using either one of them...

@rommansabbir
Copy link
Member Author

What was the idea behind adding jupiter?

It was required to execute gradle task.

I remember that Android is compatible with junit but not jupiter

In short, I went though the commits to find out which commit cause the test cases failure. But didn't found any suspicious commits. So I started focus on the dependencies, as many of the were updated to latest version.

Then in DEBUG mode, I found some of the classes were not found during the test which might causes the Illegal Access. One of them was Boolean class. So it was confirmed that test cases were failing becuase of those mentioned classes was not found.

So, I added tasks.withType(Test) {useJUnitPlatform()} since we are using Gradle 7.x. But now Gradle Task can't execute without Jupiter dependency. After adding those, test's passes without any issue, as we expected.

@azlekov
Copy link
Contributor

azlekov commented Jun 9, 2022

@rommansabbir do you think we can migrate then to jupiter instead using junit. It's preferable to use only one library, where junit and jupiter are almost same, different versions.

@rommansabbir
Copy link
Member Author

@rommansabbir do you think we can migrate then to jupiter instead using junit. It's preferable to use only one library, where junit and jupiter are almost same, different versions.

I think we can keep this as it is (If no issue arise). In future we have to migrate to JUnit 5.x and Jupiter is required for that.

So, we have to decide, what we want.

@mtrezza
Copy link
Member

mtrezza commented Jun 9, 2022

@L3K0V What do you think about merging as is with both jupiter and junit? Any concerns? I understand that we should decide on a single one, but are there also any possible (technical) side effects if we merge both?

@azlekov
Copy link
Contributor

azlekov commented Jun 10, 2022

@mtrezza, I think we can proceed merging as it is.

@mtrezza mtrezza merged commit 779dc0b into parse-community:master Jun 10, 2022
@mtrezza
Copy link
Member

mtrezza commented Jun 10, 2022

Nice job everyone and thanks for reviving this PR @rommansabbir!

parseplatformorg pushed a commit that referenced this pull request Jun 10, 2022
# [4.0.0](3.0.1...4.0.0) (2022-06-10)

### Features

* update various dependencies ([#1172](#1172)) ([779dc0b](779dc0b))

### BREAKING CHANGES

* The Facebook Login SDK is upgraded to 13.x; this is a transitive dependency, so if you are directly calling the Facebook Login SDK in your app then this may be a braking change; this release of the Parse Android SDK adds a version range to the Facebook Login SDK dependency for correct gradle dependency resolving ([779dc0b](779dc0b))
@parseplatformorg
Copy link

🎉 This change has been released in version 4.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Jun 10, 2022
@rommansabbir rommansabbir deleted the fix/fb branch June 10, 2022 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Facebook login fails logging in to Facebook from an embedded browser Gradle dependencies out of date
5 participants