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
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -151,22 +151,20 @@ jobs:
- android/restore-build-cache
- run:
name: Run Tests
# We are running tests in the "defaults" flavor only. If we ever add/remove flavors, we will need to update
# this command.
command: ./gradlew lint testDefaultsDebugUnitTest testDefaultsReleaseUnitTest
command: ./gradlew lint test
- run:
name: Consolidate artifacts
command: |
mkdir -p build/test-results/
find . -type f -regex ".*/build/test-results/.*xml" -exec cp --parents {} build/test-results/ \;
- run:
name: Kover HTML
command: ./gradlew purchases:koverHtmlReportDefaultsRelease
command: ./gradlew purchases:koverHtmlReportRelease
- run:
name: Kover XML
command: ./gradlew purchases:koverXmlReportDefaultsRelease
command: ./gradlew purchases:koverXmlReportRelease
- codecov/upload:
file: purchases/build/reports/kover/reportDefaultsRelease.xml
file: purchases/build/reports/kover/reportRelease.xml
- android/save-build-cache
- store_artifacts:
path: build/reports
Expand Down
2 changes: 1 addition & 1 deletion .run/Debug tests.run.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</option>
<option name="taskNames">
<list>
<option value="testDefaultsDebugUnitTest" />
<option value="testDebugUnitTest" />
</list>
</option>
<option name="vmOptions" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<component name="ProjectRunConfigurationManager">
<configuration default="false" name="Purchases Integration Tests (set purchases module variant before executing)" type="AndroidTestRunConfigurationType" factoryName="Android Instrumented Tests" singleton="true">
<configuration default="false" name="Purchases Integration Tests" type="AndroidTestRunConfigurationType" factoryName="Android Instrumented Tests" singleton="true">
<module name="purchases-android.purchases.androidTest" />
<option name="TESTING_TYPE" value="0" />
<option name="METHOD_NAME" value="" />
Expand Down
2 changes: 1 addition & 1 deletion .run/Release tests.run.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</option>
<option name="taskNames">
<list>
<option value="testDefaultsReleaseUnitTest" />
<option value="testReleaseUnitTest" />
</list>
</option>
<option name="vmOptions" />
Expand Down
4 changes: 0 additions & 4 deletions base-application.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ android {
defaultConfig {
minSdkVersion minVersion
targetSdkVersion compileVersion
// Library modules have a new dimension used to add extra APIs for integration tests
// Our applications however don't need the extra flavor. This makes sure that we use the
// flavor without the extra APIs in our applications.
missingDimensionStrategy 'extraAPIs', 'defaults'

testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
}
Expand Down
16 changes: 8 additions & 8 deletions fastlane/Fastfile
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,11 @@ platform :android do
new_text: ENV['LOAD_SHEDDER_REVENUECAT_API_KEY'],
paths_of_files_to_update: [constants_path]
)
# Running integration test flavor in order to make backend tests run faster, since the jittering is too
# long in the `defaults` flavor.
gradle(
task: ':purchases:testIntegrationTestDebugUnitTest',
task: ':purchases:testDebugUnitTest',
properties: {
"RUN_INTEGRATION_TESTS" => true
"RUN_INTEGRATION_TESTS" => true,
"minSdkVersion" => 21,
}
)
end
Expand Down Expand Up @@ -221,7 +220,7 @@ platform :android do
def build_purchases_integration_tests(app_name:, api_key:, google_purchase_token:, product_id_to_purchase:,
base_plan_id_to_purchase:, active_entitlements_to_verify: '',
proxy_url: nil, build_type: 'release')
constants_path = './purchases/src/androidTestIntegrationTest/kotlin/com/revenuecat/purchases/Constants.kt'
constants_path = './purchases/src/androidTest/kotlin/com/revenuecat/purchases/Constants.kt'
replace_text_in_files(
previous_text: "REVENUECAT_API_KEY",
new_text: api_key,
Expand Down Expand Up @@ -472,11 +471,12 @@ def build_purchases_android_test_apk(package_name, apk_name, build_type)
task: ':purchases:assembleAndroidTest',
properties: {
"testApplicationId" => package_name,
"testBuildType" => build_type
"testBuildType" => build_type,
"minSdkVersion" => 21
}
)
apk_path = "../purchases/build/outputs/apk/androidTest/integrationTest/#{build_type}/purchases-integrationTest-#{build_type}-androidTest.apk"
new_apk_path = "../purchases/build/outputs/apk/androidTest/integrationTest/#{build_type}/#{apk_name}.apk"
apk_path = "../purchases/build/outputs/apk/androidTest/#{build_type}/purchases-#{build_type}-androidTest.apk"
new_apk_path = "../purchases/build/outputs/apk/androidTest/#{build_type}/#{apk_name}.apk"
sh "mv #{apk_path} #{new_apk_path}"
copy_artifacts(
target_path: 'purchases/test_artifacts',
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ purchaseTesterMinSdkVersion=21

#Do not sign releases. When calling uploadArchives pass -PRELEASE_SIGNING_ENABLED=true
RELEASE_SIGNING_ENABLED=false
ANDROID_VARIANT_TO_PUBLISH=defaultsRelease
ANDROID_VARIANT_TO_PUBLISH=release
SONATYPE_HOST=DEFAULT
SONATYPE_AUTOMATIC_RELEASE=true

Expand Down
23 changes: 11 additions & 12 deletions library.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,8 @@ android {
compileSdkVersion compileVersion
buildToolsVersion buildToolsVersion

flavorDimensions "extraAPIs"
productFlavors {
defaults {
dimension "extraAPIs"
}
integrationTest {
dimension "extraAPIs"
minSdkVersion 21 // Needed because mockk-android requires API 21
}
}

defaultConfig {
minSdkVersion minVersion
minSdkVersion obtainMinSdkVersion()
targetSdkVersion compileVersion
versionCode 1
versionName "6.8.0-SNAPSHOT"
Expand Down Expand Up @@ -42,3 +31,13 @@ project.afterEvaluate {
// after https://github.com/Kotlin/kotlinx-kover/issues/362 is fixed
project.pluginManager.apply("org.jetbrains.kotlinx.kover")
}

def obtainMinSdkVersion() {
def result = minVersion

if (project.hasProperty("minSdkVersion")) {
result = project.getProperties().get("minSdkVersion").toInteger()
}

result
}
38 changes: 18 additions & 20 deletions purchases/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,12 @@ android {
buildConfig true
}

productFlavors {
integrationTest {
testApplicationId obtainTestApplicationId()
testBuildType obtainTestBuildType()
packagingOptions {
resources.excludes.add("META-INF/LICENSE.md")
resources.excludes.add("META-INF/LICENSE-notice.md")
}
defaultConfig {
testApplicationId obtainTestApplicationId()
testBuildType obtainTestBuildType()
packagingOptions {
resources.excludes.add("META-INF/LICENSE.md")
resources.excludes.add("META-INF/LICENSE-notice.md")
}
}

Expand Down Expand Up @@ -81,18 +79,18 @@ dependencies {
testImplementation libs.okhttp.mockwebserver
testImplementation libs.playServices.ads.identifier

integrationTestImplementation libs.androidx.appcompat
integrationTestImplementation libs.material
integrationTestImplementation libs.androidx.constraintlayout

androidTestIntegrationTestImplementation libs.androidx.lifecycle.runtime.ktx
androidTestIntegrationTestImplementation libs.androidx.test.espresso.core
androidTestIntegrationTestImplementation libs.androidx.test.runner
androidTestIntegrationTestImplementation libs.androidx.test.rules
androidTestIntegrationTestImplementation libs.androidx.test.junit
androidTestIntegrationTestImplementation libs.assertJ
androidTestIntegrationTestImplementation libs.mockk.android
androidTestIntegrationTestImplementation libs.mockk.agent
androidTestImplementation libs.androidx.appcompat
androidTestImplementation libs.androidx.core
androidTestImplementation libs.androidx.lifecycle.common
androidTestImplementation libs.androidx.lifecycle.runtime.ktx
androidTestImplementation libs.androidx.lifecycle.process
tonidero marked this conversation as resolved.
Show resolved Hide resolved
androidTestImplementation libs.androidx.test.espresso.core
androidTestImplementation libs.androidx.test.runner
androidTestImplementation libs.androidx.test.rules
androidTestImplementation libs.androidx.test.junit
androidTestImplementation libs.assertJ
androidTestImplementation libs.mockk.android
androidTestImplementation libs.mockk.agent
}

tasks.dokkaHtmlPartial.configure {
Expand Down
13 changes: 13 additions & 0 deletions purchases/src/androidTest/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<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?

<application
android:allowBackup="true"
android:label="RevenueCatIntegrationTests"
android:supportsRtl="true"
android:debuggable="true"
tools:ignore="HardcodedDebugMode">
<activity android:name="com.revenuecat.purchases.MainActivity" android:exported="true"/>
</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package com.revenuecat.purchases

import android.os.Bundle
import android.widget.LinearLayout
import androidx.appcompat.app.AppCompatActivity
import androidx.fragment.app.FragmentActivity

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.

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(LinearLayout(this))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ internal fun Purchases.Companion.configure(
overrideBillingAbstract,
forceServerErrors,
forceSigningErrors,
runningIntegrationTests = true,
).also {
@SuppressLint("RestrictedApi")
sharedInstance = it
Expand Down
17 changes: 0 additions & 17 deletions purchases/src/integrationTest/AndroidManifest.xml

This file was deleted.

This file was deleted.

Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
10 changes: 0 additions & 10 deletions purchases/src/integrationTest/res/values/colors.xml

This file was deleted.

3 changes: 0 additions & 3 deletions purchases/src/integrationTest/res/values/strings.xml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import android.content.Context
import android.content.pm.PackageManager
import android.preference.PreferenceManager
import androidx.annotation.VisibleForTesting
import com.revenuecat.purchases.api.BuildConfig
import com.revenuecat.purchases.common.Anonymizer
import com.revenuecat.purchases.common.AppConfig
import com.revenuecat.purchases.common.Backend
Expand Down Expand Up @@ -45,7 +44,6 @@ import java.util.concurrent.ThreadFactory
internal class PurchasesFactory(
private val apiKeyValidator: APIKeyValidator = APIKeyValidator(),
) {
private val integrationTestFlavor = "integrationTest"

@Suppress("LongMethod", "LongParameterList")
fun createPurchases(
Expand All @@ -55,6 +53,7 @@ internal class PurchasesFactory(
overrideBillingAbstract: BillingAbstract? = null,
forceServerErrors: Boolean = false,
forceSigningError: Boolean = false,
runningIntegrationTests: Boolean = false,
): Purchases {
validateConfiguration(configuration)

Expand All @@ -67,7 +66,7 @@ internal class PurchasesFactory(
proxyURL,
store,
dangerousSettings,
BuildConfig.FLAVOR == integrationTestFlavor,
runningIntegrationTests,
forceServerErrors,
forceSigningError,
)
Expand All @@ -77,8 +76,8 @@ internal class PurchasesFactory(
val sharedPreferencesForETags = ETagManager.initializeSharedPreferences(context)
val eTagManager = ETagManager(sharedPreferencesForETags)

val dispatcher = Dispatcher(service ?: createDefaultExecutor())
val diagnosticsDispatcher = Dispatcher(createDiagnosticsExecutor())
val dispatcher = Dispatcher(service ?: createDefaultExecutor(), runningIntegrationTests)
val diagnosticsDispatcher = Dispatcher(createDiagnosticsExecutor(), runningIntegrationTests)

var diagnosticsFileHelper: DiagnosticsFileHelper? = null
var diagnosticsTracker: DiagnosticsTracker? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ internal enum class Delay(val minDelay: Duration, val maxDelay: Duration) {

internal open class Dispatcher(
private val executorService: ExecutorService,
private val runningIntegrationTests: Boolean = false,
) {
private companion object {
const val INTEGRATION_TEST_DELAY_PERCENTAGE: Double = .01
}

abstract class AsyncCall : Runnable {
@Throws(JSONException::class, IOException::class)
Expand Down Expand Up @@ -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

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

executorService.schedule(command, delayToApply, TimeUnit.MILLISECONDS)
} else {
executorService.submit(command)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package com.revenuecat.purchases.common

import kotlin.time.Duration.Companion.milliseconds

// These values are modified in the `integrationTest` flavor to make the tests run faster.
internal object DispatcherConstants {
val jitterDelay = 5000L.milliseconds
val jitterLongDelay = 10000L.milliseconds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ internal abstract class BaseBackendIntegrationTest {
every { forceServerErrors } returns false
every { forceSigningErrors } returns false
}
dispatcher = Dispatcher(Executors.newSingleThreadScheduledExecutor())
diagnosticsDispatcher = Dispatcher(Executors.newSingleThreadScheduledExecutor())
dispatcher = Dispatcher(Executors.newSingleThreadScheduledExecutor(), runningIntegrationTests = true)
diagnosticsDispatcher = Dispatcher(Executors.newSingleThreadScheduledExecutor(), runningIntegrationTests = true)
sharedPreferencesEditor = mockk<SharedPreferences.Editor>().apply {
every { putString(any(), any()) } returns this
every { apply() } just Runs
Expand Down
Loading