Skip to content

dataconnect: minor unit test improvements #7257

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

dataconnect: minor unit test improvements #7257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 16 additions & 10 deletions ...ataconnect/src/test/kotlin/com/google/firebase/dataconnect/DataConnectSettingsUnitTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,11 @@ class DataConnectSettingsUnitTest {

@Test
fun `hashCode() should return a different value when only 'host' differs`() = runTest {
checkAll(propTestConfig, Arb.dataConnect.dataConnectSettings(), Arb.dataConnect.string()) {
settings1,
newHost ->
checkAll(
hashEqualityPropTestConfig,
Arb.dataConnect.dataConnectSettings(),
Arb.dataConnect.string()
) { settings1, newHost ->
assume { settings1.host.hashCode() != newHost.hashCode() }
val settings2 = settings1.copy(host = newHost)
settings1.equals(settings2) shouldBe false
Expand All @@ -153,7 +155,7 @@ class DataConnectSettingsUnitTest {

@Test
fun `hashCode() should return a different value when only 'sslEnabled' differs`() = runTest {
checkAll(propTestConfig, Arb.dataConnect.dataConnectSettings()) { settings1 ->
checkAll(hashEqualityPropTestConfig, Arb.dataConnect.dataConnectSettings()) { settings1 ->
val settings2 = settings1.copy(sslEnabled = !settings1.sslEnabled)
settings1.equals(settings2) shouldBe false
}
Expand All @@ -180,9 +182,8 @@ class DataConnectSettingsUnitTest {
newHost ->
val settings2 = settings1.copy(host = newHost)
assertSoftly {
settings1 shouldNotBeSameInstanceAs settings2
settings1.equals(settings2) shouldBe false
settings2.host shouldBeSameInstanceAs newHost
settings2.sslEnabled shouldBe settings1.sslEnabled
}
}
}
Expand All @@ -194,8 +195,7 @@ class DataConnectSettingsUnitTest {
newSslEnabled ->
val settings2 = settings1.copy(sslEnabled = newSslEnabled)
assertSoftly {
settings1 shouldNotBeSameInstanceAs settings2
settings1.equals(settings2) shouldBe (settings1.sslEnabled == newSslEnabled)
settings2.host shouldBeSameInstanceAs settings1.host
settings2.sslEnabled shouldBe newSslEnabled
}
}
Expand All @@ -211,8 +211,6 @@ class DataConnectSettingsUnitTest {
) { settings1, newHost, newSslEnabled ->
val settings2 = settings1.copy(host = newHost, sslEnabled = newSslEnabled)
assertSoftly {
settings1 shouldNotBeSameInstanceAs settings2
settings1.equals(settings2) shouldBe false
settings2.host shouldBeSameInstanceAs newHost
settings2.sslEnabled shouldBe newSslEnabled
}
Expand All @@ -221,5 +219,13 @@ class DataConnectSettingsUnitTest {

private companion object {
val propTestConfig = PropTestConfig(iterations = 20)

// Allow a small number of failures to account for the rare, but possible situation where two
// distinct instances produce the same hash code.
val hashEqualityPropTestConfig =
propTestConfig.copy(
minSuccess = propTestConfig.iterations!! - 2,
maxFailure = 2,
)
}
}
95 changes: 43 additions & 52 deletions ...taconnect/src/test/kotlin/com/google/firebase/dataconnect/core/QueryResultImplUnitTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ package com.google.firebase.dataconnect.core

import com.google.firebase.dataconnect.testutil.property.arbitrary.DataConnectArb
import com.google.firebase.dataconnect.testutil.property.arbitrary.dataConnect
import com.google.firebase.dataconnect.testutil.property.arbitrary.distinctPair
import com.google.firebase.dataconnect.testutil.property.arbitrary.queryRefImpl
import com.google.firebase.dataconnect.testutil.shouldContainWithNonAbuttingText
import io.kotest.assertions.assertSoftly
import io.kotest.assertions.withClue
import io.kotest.common.ExperimentalKotest
import io.kotest.matchers.shouldBe
import io.kotest.matchers.shouldNotBe
Expand All @@ -31,9 +33,10 @@ import io.kotest.matchers.string.shouldStartWith
import io.kotest.matchers.types.shouldBeSameInstanceAs
import io.kotest.property.Arb
import io.kotest.property.PropTestConfig
import io.kotest.property.arbitrary.arbitrary
import io.kotest.property.arbitrary.bind
import io.kotest.property.arbitrary.choice
import io.kotest.property.arbitrary.int
import io.kotest.property.arbitrary.map
import io.kotest.property.assume
import io.kotest.property.checkAll
import kotlinx.coroutines.test.runTest
Expand All @@ -43,22 +46,17 @@ import org.junit.Test
class QueryResultImplUnitTest {

@Test
fun `'data' should be the same object given to the constructor`() = runTest {
checkAll(propTestConfig, Arb.dataConnect.queryRefImpl(), Arb.dataConnect.testData()) {
query,
data ->
val queryResult = query.QueryResultImpl(data)
queryResult.data shouldBeSameInstanceAs data
}
}

@Test
fun `'ref' should be the QueryRefImpl object that was used to create it`() = runTest {
checkAll(propTestConfig, Arb.dataConnect.queryRefImpl(), Arb.dataConnect.testData()) {
query,
data ->
val queryResult = query.QueryResultImpl(data)
queryResult.ref shouldBeSameInstanceAs query
fun `properties should be the same objects given to or inferred by the constructor`() = runTest {
checkAll(
propTestConfig,
Arb.dataConnect.queryRefImpl(),
Arb.dataConnect.testData(),
) { ref, data ->
val queryResult = ref.QueryResultImpl(data)
assertSoftly {
withClue("ref") { queryResult.ref shouldBeSameInstanceAs ref }
withClue("data") { queryResult.data shouldBeSameInstanceAs data }
}
}
}

Expand Down Expand Up @@ -125,10 +123,8 @@ class QueryResultImplUnitTest {
checkAll(
propTestConfig,
Arb.dataConnect.queryRefImpl(),
Arb.dataConnect.testData(),
Arb.dataConnect.testData()
) { query, data1, data2 ->
assume(data1 != data2)
Arb.dataConnect.testData().distinctPair()
) { query, (data1, data2) ->
val queryResult1 = query.QueryResultImpl(data1)
val queryResult2 = query.QueryResultImpl(data2)
queryResult1.equals(queryResult2) shouldBe false
Expand All @@ -139,11 +135,9 @@ class QueryResultImplUnitTest {
fun `equals() should return false when only 'ref' differs`() = runTest {
checkAll(
propTestConfig,
Arb.dataConnect.queryRefImpl(),
Arb.dataConnect.queryRefImpl(),
Arb.dataConnect.queryRefImpl().distinctPair(),
Arb.dataConnect.testData()
) { query1, query2, data,
->
) { (query1, query2), data ->
assume(query1 != query2)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This assume call appears to be redundant now that you are using distinctPair(), which presumably generates a pair of distinct values. You've removed a similar assume in other tests after introducing distinctPair. This one can likely be removed for consistency and clarity.

val queryResult1 = query1.QueryResultImpl(data)
val queryResult2 = query2.QueryResultImpl(data)
Expand All @@ -152,12 +146,11 @@ class QueryResultImplUnitTest {
}

@Test
fun `equals() should return false when data of first object is null and second is non-null`() =
fun `equals() should return false when data of first data is null and second is non-null`() =
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The new test name is a bit confusing. The phrase "data of first data" is hard to parse. The original name ... when data of first object is null ... was clearer. Consider renaming it for better readability, for example, to ... when first object's data is null and second's is not.

Suggested change
fun `equals() should return false when data of first data is null and second is non-null`() =
fun `equals() should return false when first object's data is null and second is non-null`() =

runTest {
checkAll(propTestConfig, Arb.dataConnect.queryRefImpl(), Arb.dataConnect.testData()) {
query,
data,
->
data ->
val queryResult1 = query.QueryResultImpl(null)
val queryResult2 = query.QueryResultImpl(data)
queryResult1.equals(queryResult2) shouldBe false
Expand All @@ -169,8 +162,7 @@ class QueryResultImplUnitTest {
runTest {
checkAll(propTestConfig, Arb.dataConnect.queryRefImpl(), Arb.dataConnect.testData()) {
query,
data,
->
data ->
val queryResult1 = query.QueryResultImpl(data)
val queryResult2 = query.QueryResultImpl(null)
queryResult1.equals(queryResult2) shouldBe false
Expand All @@ -192,8 +184,7 @@ class QueryResultImplUnitTest {
fun `hashCode() should return the same value on equal objects`() = runTest {
checkAll(propTestConfig, Arb.dataConnect.queryRefImpl(), Arb.dataConnect.testData()) {
query,
data,
->
data ->
val queryResult1 = query.QueryResultImpl(data)
val queryResult2 = query.QueryResultImpl(data)
queryResult1.hashCode() shouldBe queryResult2.hashCode()
Expand All @@ -203,12 +194,10 @@ class QueryResultImplUnitTest {
@Test
fun `hashCode() should return a different value if 'data' is different`() = runTest {
checkAll(
propTestConfig,
hashEqualityPropTestConfig,
Arb.dataConnect.queryRefImpl(),
Arb.dataConnect.testData(),
Arb.dataConnect.testData()
) { query, data1, data2,
->
Arb.dataConnect.testData().distinctPair(),
) { query, (data1, data2) ->
assume(data1.hashCode() != data2.hashCode())
val queryResult1 = query.QueryResultImpl(data1)
val queryResult2 = query.QueryResultImpl(data2)
Expand All @@ -219,12 +208,10 @@ class QueryResultImplUnitTest {
@Test
fun `hashCode() should return a different value if 'ref' is different`() = runTest {
checkAll(
propTestConfig,
Arb.dataConnect.queryRefImpl(),
Arb.dataConnect.queryRefImpl(),
hashEqualityPropTestConfig,
Arb.dataConnect.queryRefImpl().distinctPair(),
Arb.dataConnect.testData()
) { query1, query2, data,
->
) { (query1, query2), data ->
assume(query1.hashCode() != query2.hashCode())
val queryResult1 = query1.QueryResultImpl(data)
val queryResult2 = query2.QueryResultImpl(data)
Expand All @@ -239,21 +226,25 @@ class QueryResultImplUnitTest {
private companion object {
val propTestConfig = PropTestConfig(iterations = 20)

// Allow a small number of failures to account for the rare, but possible situation where two
// distinct instances produce the same hash code.
val hashEqualityPropTestConfig =
propTestConfig.copy(
minSuccess = propTestConfig.iterations!! - 2,
maxFailure = 2,
)

fun DataConnectArb.testVariables(string: Arb<String> = string()): Arb<TestVariables> =
arbitrary {
TestVariables(string.bind())
}
string.map { TestVariables(it) }

fun DataConnectArb.testData(string: Arb<String> = string()): Arb<TestData> = arbitrary {
TestData(string.bind())
}
fun DataConnectArb.testData(string: Arb<String> = string()): Arb<TestData> =
string.map { TestData(it) }

fun DataConnectArb.queryResultImpl(
query: Arb<QueryRefImpl<TestData?, TestVariables>> = queryRefImpl(),
data: Arb<TestData> = testData()
): Arb<QueryRefImpl<TestData?, TestVariables>.QueryResultImpl> = arbitrary {
query.bind().QueryResultImpl(data.bind())
}
data: Arb<TestData> = testData(),
): Arb<QueryRefImpl<TestData?, TestVariables>.QueryResultImpl> =
Arb.bind(query, data) { query, data -> query.QueryResultImpl(data) }

fun DataConnectArb.queryRefImpl(): Arb<QueryRefImpl<TestData?, TestVariables>> =
queryRefImpl(Arb.dataConnect.testVariables())
Expand Down