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

Remove integration test flavor #1143

Merged
merged 11 commits into from
Jul 19, 2023

Conversation

tonidero
Copy link
Contributor

Description

After we joined all modules into one, we can remove the integration test flavor. This will simplify things

purchases/build.gradle Outdated Show resolved Hide resolved
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools">
<!-- Ignoring errors in API <21 since mockk-android is only supported starting in API 21 -->
<uses-sdk tools:overrideLibrary="io.mockk,io.mockk.proxy.android"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need this since mockk-android is only android api 21+. If we try to run integration test in less than that it would fail at runtime. Before, it wouldn't even run (since the integration test flavor was 21+). But I think this is ok?


class MainActivity : AppCompatActivity() {
class MainActivity : FragmentActivity() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was running into a weird issue of a missing function when using AppCompatActivity... I tried to research a bit and didn't find anything. Going to the superclass fixed things, but we probably want to take another look when we have more time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it maybe due to the differences in the AndroidManifest?

Copy link
Contributor Author

@tonidero tonidero Jul 19, 2023

Choose a reason for hiding this comment

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

Shouldn't be, the activity that is defined is MainActivity, not the superclass... As in, why would it matter what's the superclass of the MainActivity in the AndroidManifest? It seemed to be as if a dependency was missing, but it didn't make much sense... We probably want to revisit this at some other time, but this works for now.

var delayToApply = (delay.minDelay.inWholeMilliseconds..delay.maxDelay.inWholeMilliseconds).random()
if (runningIntegrationTests) {
delayToApply = (delayToApply * INTEGRATION_TEST_DELAY_PERCENTAGE).toLong()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not ideal... Honestly it was cleaner before, but I think it's worth this for the tradeoff of removing the extra flavor... Lmk what you think!

Copy link
Contributor

Choose a reason for hiding this comment

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

I honestly think the other way was a bit "magic" in the sense that it was hard to know the override was happening, I think this way it's more explicit

@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #1143 (9fef071) into main (32ee171) will increase coverage by 0.06%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##             main    #1143      +/-   ##
==========================================
+ Coverage   85.37%   85.43%   +0.06%     
==========================================
  Files         175      176       +1     
  Lines        6194     6200       +6     
  Branches      916      917       +1     
==========================================
+ Hits         5288     5297       +9     
+ Misses        560      558       -2     
+ Partials      346      345       -1     
Impacted Files Coverage Δ
...revenuecat/purchases/common/DispatcherConstants.kt 100.00% <ø> (ø)
...otlin/com/revenuecat/purchases/PurchasesFactory.kt 86.58% <75.00%> (ø)
...tlin/com/revenuecat/purchases/common/Dispatcher.kt 86.04% <100.00%> (+9.12%) ⬆️

@tonidero tonidero marked this pull request as ready for review July 19, 2023 12:25
@tonidero tonidero requested a review from a team July 19, 2023 12:25
purchases/build.gradle Outdated Show resolved Hide resolved
var delayToApply = (delay.minDelay.inWholeMilliseconds..delay.maxDelay.inWholeMilliseconds).random()
if (runningIntegrationTests) {
delayToApply = (delayToApply * INTEGRATION_TEST_DELAY_PERCENTAGE).toLong()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I honestly think the other way was a bit "magic" in the sense that it was hard to know the override was happening, I think this way it's more explicit

@@ -56,7 +60,10 @@ internal open class Dispatcher(
synchronized(this.executorService) {
if (!executorService.isShutdown) {
val future = if (delay != Delay.NONE && executorService is ScheduledExecutorService) {
val delayToApply = (delay.minDelay.inWholeMilliseconds..delay.maxDelay.inWholeMilliseconds).random()
var delayToApply = (delay.minDelay.inWholeMilliseconds..delay.maxDelay.inWholeMilliseconds).random()
if (runningIntegrationTests) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only place where we use runningIntegrationTests? If so maybe it's cleaner to have the delay as parameter of the Dispatcher constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm how do you mean? There can be 3 different types of delay: NONE, DEFAULT or LONG, with different durations so the delay needs to be a parameter of the enqueue method. Not sure if you meant something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

nevermind, this looks good


class MainActivity : AppCompatActivity() {
class MainActivity : FragmentActivity() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it maybe due to the differences in the AndroidManifest?

@tonidero tonidero requested a review from vegaro July 19, 2023 13:46
@tonidero tonidero merged commit 492e97c into main Jul 19, 2023
2 checks passed
@tonidero tonidero deleted the toniricodiez/sdk-3234-remove-integrationtest-flavor branch July 19, 2023 14:52
tonidero added a commit that referenced this pull request Jul 20, 2023
### Description
After we joined all modules into one, we can remove the integration test
flavor. This will simplify things
This was referenced Jul 21, 2023
aboedo added a commit that referenced this pull request Jul 24, 2023
**This is an automatic release.**

### New Features

Introduced Custom Entitlements Computation mode. 

This is new library intended for apps that will do their own entitlement
computation separate from RevenueCat. It's distributed as a separate
artifact in Maven.

Apps using this mode rely on webhooks to signal their backends to
refresh entitlements with RevenueCat.

See the [demo app for an example and usage
instructions](https://github.com/RevenueCat/purchases-android/tree/main/examples/CustomEntitlementComputationSample).

* Custom entitlements: add README and other improvements (#1167) via
Andy Boedo (@aboedo)
* Update Custom Entitlements Sample app (#1166) via Andy Boedo (@aboedo)
* purchase coroutine (#1142) via Andy Boedo (@aboedo)
* Add switchUser (#1156) via Cesar de la Vega (@vegaro)
* CustomEntitlementsComputation: disable first listener callback when
set (#1152) via Andy Boedo (@aboedo)
* CustomEntitlementsComputation: Prevent posting subscriber attributes
in post receipt (#1151) via Andy Boedo (@aboedo)
* Fix `customEntitlementComputation` library deployment (#1169) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Configure method for
customEntitlementComputation mode (#1168) via Toni Rico (@tonidero)
* Add publish system for customEntitlementComputation package (#1149)
via Cesar de la Vega (@vegaro)
* Use purchase coroutine in CustomEntitlementComputationSample (#1162)
via Cesar de la Vega (@vegaro)
* Adds CustomEntitlementComputationSample (#1160) via Cesar de la Vega
(@vegaro)
* Fix tests in customEntitlementComputation after merges (#1161) via
Toni Rico (@tonidero)
* CustomEntitlementComputation: Remove custom entitlement computation
flavor for amazon module (#1158) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Generate dokka docs only for defaults
flavor (#1159) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Create different PurchasesConfiguration
that requires an appUserId parameter (#1154) via Toni Rico (@tonidero)
* CustomEntitlementComputation: New Purchases class (#1153) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Disable automatic cache refresh (#1157)
via Toni Rico (@tonidero)
* Add `customEntitlementComputation` flavor (#1147) via Toni Rico
(@tonidero)
* Make `customEntitlementComputation` singular (#1148) via Toni Rico
(@tonidero)
* Disable offline entitlements in custom entitlements computation mode
(#1146) via Toni Rico (@tonidero)
* Remove integration test flavor (#1143) via Toni Rico (@tonidero)
* Add header to requests when in custom entitlement computation mode
(#1145) via Toni Rico (@tonidero)
* Add internal customEntitlementsComputation mode to app config (#1141)
via Toni Rico (@tonidero)

### New Coroutines
* `awaitPurchase` is available as a coroutine-friendly alternative to
`purchase()`. (#1142) via Andy Boedo (@aboedo)

### Dependency Updates
* Bump fastlane from 2.213.0 to 2.214.0 (#1140) via dependabot[bot]
(@dependabot[bot])

### Other changes
* CI: make all Codecov jobs informational (#1155) via Cesar de la Vega
(@vegaro)
* Creates PurchasesOrchestrator (#1144) via Cesar de la Vega (@vegaro)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Andy Boedo <andresboedo@gmail.com>
tonidero pushed a commit that referenced this pull request Jul 25, 2023
### New Features

Introduced Custom Entitlements Computation mode. 

This is new library intended for apps that will do their own entitlement
computation separate from RevenueCat. It's distributed as a separate
artifact in Maven.

Apps using this mode rely on webhooks to signal their backends to
refresh entitlements with RevenueCat.

See the [demo app for an example and usage
instructions](https://github.com/RevenueCat/purchases-android/tree/main/examples/CustomEntitlementComputationSample).

* Custom entitlements: add README and other improvements (#1167) via
Andy Boedo (@aboedo)
* Update Custom Entitlements Sample app (#1166) via Andy Boedo (@aboedo)
* purchase coroutine (#1142) via Andy Boedo (@aboedo)
* Add switchUser (#1156) via Cesar de la Vega (@vegaro)
* CustomEntitlementsComputation: disable first listener callback when
set (#1152) via Andy Boedo (@aboedo)
* CustomEntitlementsComputation: Prevent posting subscriber attributes
in post receipt (#1151) via Andy Boedo (@aboedo)
* Fix `customEntitlementComputation` library deployment (#1169) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Configure method for
customEntitlementComputation mode (#1168) via Toni Rico (@tonidero)
* Add publish system for customEntitlementComputation package (#1149)
via Cesar de la Vega (@vegaro)
* Use purchase coroutine in CustomEntitlementComputationSample (#1162)
via Cesar de la Vega (@vegaro)
* Adds CustomEntitlementComputationSample (#1160) via Cesar de la Vega
(@vegaro)
* Fix tests in customEntitlementComputation after merges (#1161) via
Toni Rico (@tonidero)
* CustomEntitlementComputation: Remove custom entitlement computation
flavor for amazon module (#1158) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Generate dokka docs only for defaults
flavor (#1159) via Toni Rico (@tonidero)
* CustomEntitlementComputation: Create different PurchasesConfiguration
that requires an appUserId parameter (#1154) via Toni Rico (@tonidero)
* CustomEntitlementComputation: New Purchases class (#1153) via Toni
Rico (@tonidero)
* CustomEntitlementComputation: Disable automatic cache refresh (#1157)
via Toni Rico (@tonidero)
* Add `customEntitlementComputation` flavor (#1147) via Toni Rico
(@tonidero)
* Make `customEntitlementComputation` singular (#1148) via Toni Rico
(@tonidero)
* Disable offline entitlements in custom entitlements computation mode
(#1146) via Toni Rico (@tonidero)
* Remove integration test flavor (#1143) via Toni Rico (@tonidero)
* Add header to requests when in custom entitlement computation mode
(#1145) via Toni Rico (@tonidero)
* Add internal customEntitlementsComputation mode to app config (#1141)
via Toni Rico (@tonidero)

### New Coroutines
* `awaitPurchase` is available as a coroutine-friendly alternative to
`purchase()`. (#1142) via Andy Boedo (@aboedo)

### Dependency Updates
* Bump fastlane from 2.213.0 to 2.214.0 (#1140) via dependabot[bot]
(@dependabot[bot])

### Other changes
* CI: make all Codecov jobs informational (#1155) via Cesar de la Vega
(@vegaro)
* Creates PurchasesOrchestrator (#1144) via Cesar de la Vega (@vegaro)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Andy Boedo <andresboedo@gmail.com>
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.

2 participants