Skip to content

[External] Fix OfflineCustomerInfoCalculatorTest Unresolved reference: ProductType#994

Merged
vegaro merged 1 commit into
RevenueCat:merge-external-994from
pablo-guardiola:pg-offlineentitlements-test-fix
May 4, 2023
Merged

[External] Fix OfflineCustomerInfoCalculatorTest Unresolved reference: ProductType#994
vegaro merged 1 commit into
RevenueCat:merge-external-994from
pablo-guardiola:pg-offlineentitlements-test-fix

Conversation

@pablo-guardiola

@pablo-guardiola pablo-guardiola commented May 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Checklist

  • If applicable, unit tests
  • If applicable, create follow-up issues for purchases-ios and hybrids

Motivation

Why is this change required? What problem does it solve?

Description

Describe your changes in detail

This was due to auto-merge #959 without updating from #983 which landed first

Please describe in detail how you tested your changes

Run OfflineCustomerInfoCalculatorTest before the changes ❌
Run OfflineCustomerInfoCalculatorTest after the changes ✅

cc @vegaro @tonidero

@pablo-guardiola pablo-guardiola force-pushed the pg-offlineentitlements-test-fix branch from 2f0d50d to b7b9171 Compare May 4, 2023 19:57
@vegaro vegaro changed the base branch from main to merge-external-994 May 4, 2023 20:58
vegaro
vegaro approved these changes May 4, 2023
View reviewed changes

@vegaro vegaro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh that was my fault. Thanks for fixing this

@vegaro vegaro merged commit e50c1d0 into RevenueCat:merge-external-994 May 4, 2023
@vegaro vegaro mentioned this pull request May 4, 2023
Merged
@vegaro vegaro changed the title Fix OfflineCustomerInfoCalculatorTest Unresolved reference: ProductType [External] Fix OfflineCustomerInfoCalculatorTest Unresolved reference: ProductType May 4, 2023
@pablo-guardiola

pablo-guardiola commented May 4, 2023

Copy link
Copy Markdown
Contributor Author

@vegaro Thanks for ✅ and merging 🙇‍♂️

I'd like to note that product with different entitlement per base plan failed once in CI https://app.circleci.com/pipelines/github/RevenueCat/purchases-android/5582/workflows/f4967c17-5302-4f65-a944-25d2b53806e0/jobs/15935/tests

org.opentest4j.AssertionFailedError: 
Expecting value to be true but was false
	at java.base@17.0.6/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base@17.0.6/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base@17.0.6/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base@17.0.6/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at com.revenuecat.purchases.common.offlineentitlements.OfflineCustomerInfoCalculatorTest.verifyEntitlement(OfflineCustomerInfoCalculatorTest.kt:483)
	at com.revenuecat.purchases.common.offlineentitlements.OfflineCustomerInfoCalculatorTest.product with different entitlement per base plan(OfflineCustomerInfoCalculatorTest.kt:207)
	at java.base@17.0.6/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base@17.0.6/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base@17.0.6/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base@17.0.6/java.lang.reflect.Method.invoke(Method.java:568)
	at app//org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at app//org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at app//org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at app//org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at app//org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at app//org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at app//org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:591)
	at app//org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$0(SandboxTestRunner.java:274)
	at app//org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:88)
	at java.base@17.0.6/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base@17.0.6/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base@17.0.6/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base@17.0.6/java.lang.Thread.run(Thread.java:833)

purchases-android/common/src/test/java/com/revenuecat/purchases/common/offlineentitlements/OfflineCustomerInfoCalculatorTest.kt

Lines 205 to 208 in b7b9171

// This entitlement will have the original purchase date of the second purchase
// instead of the first one until the backend starts returning original purchase date per entitlement
verifyEntitlement(receivedCustomerInfo, entitlementID, p1mProduct, expirationDate = oneHourAgo,
purchaseDate = twoHoursAgo, originalPurchaseDate = oneHourAgo)

purchases-android/common/src/test/java/com/revenuecat/purchases/common/offlineentitlements/OfflineCustomerInfoCalculatorTest.kt

Line 483 in b7b9171

assertThat(receivedEntitlement?.isActive).isTrue

Debugged it a little bit and it seems that the issue may be related to the dates provided in the test which could set EntitlementInfo#isActive as false for entitlementID "pro_1" / p1mProduct. Without knowing the underneath logic intrinsics, could it be because expiresDate for p1mProduct and purchaseTime for notBwProduct happen at the same exact instant (oneHourAgo.time)? Could that clash? Wondering if purchaseTime for notBwProduct should be 1ms after oneHourAgo.time to avoid that potential failing scenario 🤔

In any case, I'll defer to you as experts, I only wanted to flag ✌️

Thanks again!

@vegaro

vegaro commented May 4, 2023

Copy link
Copy Markdown
Member

Thanks for flagging that. Yes I've seen that test being a bit flaky but I thought it was fixed, maybe with your comment I can figure it out. Let me take a look

@vegaro vegaro mentioned this pull request May 4, 2023
Merged
@vegaro

vegaro commented May 4, 2023

Copy link
Copy Markdown
Member

@pablo-guardiola I figured it out in #997 . Thanks for pointing it out!

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

Reviewers

@vegaro vegaro vegaro approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pablo-guardiola @vegaro