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.

Sign up for GitHub

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

Jump to bottom

Fix flaky tests #1967

Merged
merged 5 commits into from
Apr 4, 2024
Merged

Fix flaky tests #1967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
044673f
Fix flaky test in DefaultAndroidInfoProviderTest
xgouchet Apr 3, 2024
4313898
Fix flaky test in TrackingConsentProviderTest
xgouchet Apr 3, 2024
a94b16f
Fix flaky test in DatadogCoreTest
xgouchet Apr 3, 2024
b0bc017
Fix flakiness in DatadogEventListenerTest
xgouchet Apr 4, 2024
a1cf20b
Remove hardcoded seed in tests
xgouchet Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 26 additions & 18 deletions ...re/src/main/kotlin/com/datadog/android/core/internal/system/DefaultAndroidInfoProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,30 @@ import com.datadog.android.api.context.DeviceType
import java.util.Locale

internal class DefaultAndroidInfoProvider(
appContext: Context
appContext: Context,
rawDeviceBrand: String,
rawDeviceModel: String,
rawDeviceId: String,
rawOsVersion: String
) : AndroidInfoProvider {

constructor(appContext: Context) : this(
appContext,
Build.BRAND.orEmpty(),
Build.MODEL.orEmpty(),
Build.ID.orEmpty(),
Build.VERSION.RELEASE.orEmpty()
)

// lazy is just to avoid breaking the tests (because without lazy type is resolved at the
// construction time and Build.MODEL is null in unit-tests) and also to have value resolved
// once to avoid different values for foldables during the application lifecycle
override val deviceType: DeviceType by lazy(LazyThreadSafetyMode.PUBLICATION) {
resolveDeviceType(appContext)
resolveDeviceType(rawDeviceModel, appContext)
}

override val deviceName: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
if (deviceBrand.isEmpty()) {
if (deviceBrand.isBlank()) {
deviceModel
} else if (deviceModel.contains(deviceBrand)) {
deviceModel
Expand All @@ -37,24 +49,18 @@ internal class DefaultAndroidInfoProvider(
}

override val deviceBrand: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
Build.BRAND.replaceFirstChar {
rawDeviceBrand.replaceFirstChar {
if (it.isLowerCase()) it.titlecase(Locale.US) else it.toString()
}
}

override val deviceModel: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
Build.MODEL
}
override val deviceModel: String = rawDeviceModel

override val deviceBuildId: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
Build.ID
}
override val deviceBuildId: String = rawDeviceId

override val osName: String = "Android"

override val osVersion: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
Build.VERSION.RELEASE
}
override val osVersion: String = rawOsVersion

override val osMajorVersion: String by lazy(LazyThreadSafetyMode.PUBLICATION) {
// result of split always have at least 1 element
Expand All @@ -71,12 +77,12 @@ internal class DefaultAndroidInfoProvider(
const val FEATURE_GOOGLE_ANDROID_TV = "com.google.android.tv"
const val MIN_TABLET_WIDTH_DP = 800

private fun resolveDeviceType(appContext: Context): DeviceType {
private fun resolveDeviceType(model: String, appContext: Context): DeviceType {
return if (isTv(appContext)) {
DeviceType.TV
} else if (isTablet(appContext)) {
} else if (isTablet(model, appContext)) {
DeviceType.TABLET
} else if (isMobile(appContext)) {
} else if (isMobile(model, appContext)) {
DeviceType.MOBILE
} else {
DeviceType.OTHER
Expand Down Expand Up @@ -104,18 +110,20 @@ internal class DefaultAndroidInfoProvider(
}

private fun isTablet(
model: String,
appContext: Context
): Boolean {
with(Build.MODEL.lowercase(Locale.US)) {
with(model.lowercase(Locale.US)) {
if (contains("tablet") || contains("sm-t")) return true
}
return appContext.resources.configuration.smallestScreenWidthDp >= MIN_TABLET_WIDTH_DP
}

private fun isMobile(
model: String,
appContext: Context
): Boolean {
if (Build.MODEL.lowercase(Locale.US).contains("phone")) return true
if (model.lowercase(Locale.US).contains("phone")) return true

val telephonyManager =
appContext.getSystemService(Context.TELEPHONY_SERVICE) as? TelephonyManager
Expand Down
2 changes: 1 addition & 1 deletion dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/DatadogCoreTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ internal class DatadogCoreTest {

@Test
fun `𝕄 stop all features 𝕎 stop()`(
@StringForgery fakeFeatureNames: List<String>
@StringForgery fakeFeatureNames: Set<String>
Copy link
Contributor

Choose a reason for hiding this comment

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

already here #1965 :)

) {
// Given
val mockCoreFeature = mock<CoreFeature>()
Expand Down
4 changes: 2 additions & 2 deletions ...test/kotlin/com/datadog/android/core/internal/data/privacy/TrackingConsentProviderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,12 @@ internal class TrackingConsentProviderTest {
countDownLatch.countDown()
}.start()
Thread {
Thread.sleep(2) // just to callback register thread to take the lock
Thread.sleep(10) // just to callback register thread to take the lock
testedConsentProvider.setConsent(fakeConsent1)
countDownLatch.countDown()
}.start()
Thread {
Thread.sleep(2)
Thread.sleep(10)
testedConsentProvider.setConsent(fakeConsent2)
countDownLatch.countDown()
}.start()
Expand Down
100 changes: 49 additions & 51 deletions ...rc/test/kotlin/com/datadog/android/core/internal/system/DefaultAndroidInfoProviderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ import android.os.Build
import android.telephony.TelephonyManager
import com.datadog.android.api.context.DeviceType
import com.datadog.android.utils.forge.Configurator
import com.datadog.tools.unit.setStaticValue
import fr.xgouchet.elmyr.Forge
import fr.xgouchet.elmyr.annotation.IntForgery
import fr.xgouchet.elmyr.annotation.StringForgery
import fr.xgouchet.elmyr.annotation.StringForgeryType
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
import fr.xgouchet.elmyr.junit5.ForgeExtension
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assumptions.assumeFalse
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -68,25 +66,28 @@ internal class DefaultAndroidInfoProviderTest {
@Mock
lateinit var mockResources: Resources

@StringForgery
lateinit var fakeDeviceBrand: String

@StringForgery
lateinit var fakeDeviceModel: String

@StringForgery
lateinit var fakeDeviceId: String

@StringForgery(regex = "[1-9]{1,3}\\.[1-9]{1,3}\\.[1-9]{1,3}")
lateinit var fakeOsVersion: String

@BeforeEach
fun setUp(forge: Forge) {
whenever(mockContext.getSystemService(Context.UI_MODE_SERVICE)) doReturn
mockUiModeManager
whenever(mockContext.getSystemService(Context.TELEPHONY_SERVICE)) doReturn
mockTelephonyManager
whenever(mockSdkVersionProvider.version) doReturn
forge.anInt(min = Build.VERSION_CODES.BASE)
whenever(mockContext.getSystemService(Context.UI_MODE_SERVICE)) doReturn mockUiModeManager
whenever(mockContext.getSystemService(Context.TELEPHONY_SERVICE)) doReturn mockTelephonyManager
whenever(mockSdkVersionProvider.version) doReturn forge.anInt(min = Build.VERSION_CODES.BASE)
whenever(mockContext.packageManager) doReturn mockPackageManager
whenever(mockContext.resources) doReturn mockResources
whenever(mockResources.configuration) doReturn Configuration()

Build::class.java.setStaticValue("MODEL", "")
}

@AfterEach
fun tearDown() {
Build::class.java.setStaticValue("MODEL", "")
Build.VERSION::class.java.setStaticValue("RELEASE", "")
fakeDeviceModel = ""
}

// region device type
Expand Down Expand Up @@ -138,10 +139,10 @@ internal class DefaultAndroidInfoProviderTest {

@Test
fun `𝕄 return Tablet type 𝕎 deviceType { Samsung SM-T series model }`(
forge: Forge
@IntForgery(1) tModelVersion: Int
) {
// Given
Build::class.java.setStaticValue("MODEL", "SM-T${forge.aPositiveInt()}")
fakeDeviceModel = "SM-T$tModelVersion"
testedProvider = createProvider()

// When
Expand All @@ -156,7 +157,7 @@ internal class DefaultAndroidInfoProviderTest {
@StringForgery(regex = "[a-zA-Z1-9 ]{0,9}Tablet[a-zA-Z1-9 ]{0,9}") fakeModel: String
) {
// Given
Build::class.java.setStaticValue("MODEL", fakeModel)
fakeDeviceModel = fakeModel
testedProvider = createProvider()

// When
Expand Down Expand Up @@ -192,8 +193,7 @@ internal class DefaultAndroidInfoProviderTest {
@StringForgery(regex = "[a-zA-Z1-9 ]{0,9}Phone[a-zA-Z1-9 ]{0,9}") fakeModel: String
) {
// Given
Build::class.java.setStaticValue("MODEL", fakeModel)

fakeDeviceModel = fakeModel
testedProvider = createProvider()

// When
Expand All @@ -207,21 +207,16 @@ internal class DefaultAndroidInfoProviderTest {
@MethodSource("phoneTypesWithDescription")
fun `𝕄 return Mobile type 𝕎 deviceType {smallest screen width less than 800dp + telephony}`(
phoneType: PhoneType,
@IntForgery(
min = 0,
max = DefaultAndroidInfoProvider.MIN_TABLET_WIDTH_DP
) fakeWidth: Int
@IntForgery(min = 0, max = DefaultAndroidInfoProvider.MIN_TABLET_WIDTH_DP) fakeWidth: Int
) {
// Given
val mockResources = mock<Resources>()
val fakeConfiguration = Configuration().apply {
smallestScreenWidthDp = fakeWidth
}

whenever(mockContext.resources) doReturn mockResources
whenever(mockResources.configuration) doReturn fakeConfiguration
whenever(mockTelephonyManager.phoneType) doReturn phoneType.value

testedProvider = createProvider()

// When
Expand Down Expand Up @@ -262,41 +257,40 @@ internal class DefaultAndroidInfoProviderTest {
// region os version + major version

@Test
fun `𝕄 return full version 𝕎 osVersion`(
@StringForgery(regex = "[1-9]{1,3}\\.[1-9]{1,3}\\.[1-9]{1,3}") fakeVersion: String
) {
fun `𝕄 return full version 𝕎 osVersion`() {
// Given
Build.VERSION::class.java.setStaticValue("RELEASE", fakeVersion)
testedProvider = createProvider()

// When
val osVersion = testedProvider.osVersion

// Then
assertThat(osVersion).isEqualTo(fakeVersion)
assertThat(osVersion).isEqualTo(fakeOsVersion)
}

@Test
fun `𝕄 return major version 𝕎 osMajorVersion { major - minor - patch format}`(
@StringForgery(regex = "[1-9]{1,3}\\.[1-9]{1,3}\\.[1-9]{1,3}") fakeVersion: String
@StringForgery(regex = "[1-9]{1,3}") fakeMajor: String,
@StringForgery(regex = "[1-9]{1,3}") fakeMinor: String,
@StringForgery(regex = "[1-9]{1,3}") fakeHotfix: String
) {
// Given
Build.VERSION::class.java.setStaticValue("RELEASE", fakeVersion)
fakeOsVersion = "$fakeMajor.$fakeMinor.$fakeHotfix"
testedProvider = createProvider()

// When
val osMajorVersion = testedProvider.osMajorVersion

// Then
assertThat(osMajorVersion).isEqualTo(fakeVersion.split(".").first())
assertThat(osMajorVersion).isEqualTo(fakeMajor)
}

@Test
fun `𝕄 return major version 𝕎 osMajorVersion { generic format }`(
@StringForgery(type = StringForgeryType.ALPHA_NUMERICAL) fakeVersion: String
) {
// Given
Build.VERSION::class.java.setStaticValue("RELEASE", fakeVersion)
fakeOsVersion = fakeVersion
testedProvider = createProvider()

// When
Expand All @@ -311,19 +305,16 @@ internal class DefaultAndroidInfoProviderTest {
// region device name

@Test
fun `𝕄 return device name 𝕎 deviceName { brand is blank }`(
@StringForgery fakeModel: String
) {
fun `𝕄 return device name 𝕎 deviceName { brand is blank }`() {
// Given
Build::class.java.setStaticValue("BRAND", "")
Build::class.java.setStaticValue("MODEL", fakeModel)
fakeDeviceBrand = ""
testedProvider = createProvider()

// When
val deviceName = testedProvider.deviceName

// Then
assertThat(deviceName).isEqualTo(fakeModel)
assertThat(deviceName).isEqualTo(fakeDeviceModel)
}

@Test
Expand All @@ -333,16 +324,16 @@ internal class DefaultAndroidInfoProviderTest {
@StringForgery modelSuffix: String
) {
// Given
val deviceModel = modelPrefix + fakeBrand.capitalize() + modelSuffix
Build::class.java.setStaticValue("BRAND", fakeBrand)
Build::class.java.setStaticValue("MODEL", deviceModel)
val fakeModel = modelPrefix + fakeBrand.capitalize() + modelSuffix
fakeDeviceModel = fakeModel
fakeDeviceBrand = fakeBrand
testedProvider = createProvider()

// When
val deviceName = testedProvider.deviceName

// Then
assertThat(deviceName).isEqualTo(deviceModel)
assertThat(deviceName).isEqualTo(fakeModel)
}

@Test
Expand All @@ -352,8 +343,8 @@ internal class DefaultAndroidInfoProviderTest {
) {
// Given
assumeFalse(fakeModel.contains(fakeBrand, ignoreCase = true))
Build::class.java.setStaticValue("BRAND", fakeBrand)
Build::class.java.setStaticValue("MODEL", fakeModel)
fakeDeviceBrand = fakeBrand
fakeDeviceModel = fakeModel
testedProvider = createProvider()

// When
Expand All @@ -370,7 +361,7 @@ internal class DefaultAndroidInfoProviderTest {
@StringForgery fakeBrand: String
) {
// Given
Build::class.java.setStaticValue("BRAND", fakeBrand)
fakeDeviceBrand = fakeBrand
testedProvider = createProvider()

// When
Expand All @@ -382,10 +373,17 @@ internal class DefaultAndroidInfoProviderTest {

// region private

private fun createProvider(): AndroidInfoProvider = DefaultAndroidInfoProvider(mockContext)
private fun createProvider(): AndroidInfoProvider = DefaultAndroidInfoProvider(
mockContext,
fakeDeviceBrand,
fakeDeviceModel,
fakeDeviceId,
fakeOsVersion
)

private fun String.capitalize() =
replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.US) else it.toString() }
private fun String.capitalize() = replaceFirstChar {
if (it.isLowerCase()) it.titlecase(Locale.US) else it.toString()
}

companion object {

Expand Down
2 changes: 1 addition & 1 deletion .../test/kotlin/com/datadog/android/sessionreplay/utils/AndroidMDrawableToColorMapperTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.mockito.quality.Strictness
ExtendWith(ApiLevelExtension::class)
)
@MockitoSettings(strictness = Strictness.LENIENT)
@ForgeConfiguration(value = ForgeConfigurator::class, seed = 0x3f3a03ceae05aL)
@ForgeConfiguration(value = ForgeConfigurator::class)
open class AndroidMDrawableToColorMapperTest : LegacyDrawableToColorMapperTest() {

override fun createTestedMapper(): DrawableToColorMapper {
Expand Down
2 changes: 1 addition & 1 deletion .../test/kotlin/com/datadog/android/sessionreplay/utils/AndroidQDrawableToColorMapperTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.mockito.quality.Strictness
ExtendWith(ApiLevelExtension::class)
)
@MockitoSettings(strictness = Strictness.LENIENT)
@ForgeConfiguration(value = ForgeConfigurator::class, seed = 0x3f3a03ceae05aL)
@ForgeConfiguration(value = ForgeConfigurator::class)
class AndroidQDrawableToColorMapperTest : AndroidMDrawableToColorMapperTest() {

override fun createTestedMapper(): DrawableToColorMapper {
Expand Down