dataconnect(test): add sum partition and optional variable Arbs - #8539
dataconnect(test): add sum partition and optional variable Arbs#8539dconeybe wants to merge 1 commit into
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
dconeybe
changed the title
dconeybe
changed the title
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces test utilities and Kotest generators for the Firebase Data Connect SDK. Specifically, it adds a SumPartitionArb generator to partition an integer into a fixed number of summands, along with custom optionalVariable and nullableOptionalVariable generators in DataConnectArb. It also includes corresponding unit tests and Kotest printers. The review feedback suggests simplifying the initialization of edgeCaseZeroesCountArb using Kotlin's by lazy delegation, and addressing a potential floating-point precision issue in the probability sum validation within nullableOptionalVariable by adding a small tolerance.
| private val edgeCaseZeroesCountArb: Arb<Int> = run { | ||
| if (sum == 0) { | ||
| arbitrary { throw IllegalStateException("internal error h5zagzq8g4: should never get here") } | ||
| } else { | ||
| Arb.int(1 until summandCount) | ||
| } | ||
| } |
There was a problem hiding this comment.
Using by lazy simplifies the initialization of edgeCaseZeroesCountArb and avoids the need for a dummy error-throwing Arb when sum == 0. Since edgeCaseZeroesCountArb is only accessed inside edgecase when summandCount >= 2 and sum > 0, we are guaranteed that 1 until summandCount is a valid, non-empty range when the lazy property is initialized.
private val edgeCaseZeroesCountArb: Arb<Int> by lazy {
Arb.int(1 until summandCount)
}| val probabilitiesSum = undefinedProbability + nullableProbability | ||
| require(probabilitiesSum <= 1.0) { |
There was a problem hiding this comment.
Due to floating-point precision limits, adding two valid probabilities that mathematically sum to 1.0 (such as 0.56 and 0.44) can result in a sum slightly greater than 1.0 (e.g., 1.0000000000000002). This causes the require check to throw an IllegalArgumentException unexpectedly. Adding a tiny tolerance (e.g., 1e-9) prevents these precision-related failures while still rejecting genuinely invalid inputs.
| val probabilitiesSum = undefinedProbability + nullableProbability | |
| require(probabilitiesSum <= 1.0) { | |
| val probabilitiesSum = undefinedProbability + nullableProbability | |
| require(probabilitiesSum <= 1.0 + 1e-9) { |
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
1 participant
This PR introduces several property-based testing utilities to
firebase-dataconnect, including a KotestArbfor integer sum partitioning and generators forOptionalVariabletypes.There should be no noticeable side effects to customers; this is internal test utility code.
Highlights
SumPartitionArb, a KotestArbthat generates random partitions of a non-negative integer sum into a fixed number of non-negative summands, including edge cases for zeroes, ascending order, and descending order.optionalVariableandnullableOptionalVariablearbitrary generators toDataConnectArbto generateOptionalVariableinstances with configurable probabilities for undefined and null states.PropTestConfig.withEdgeConfigEdgeCasesOnly()for running tests solely against generated edge cases, and registered custom printer formatting for Apache CommonsSignificanceResult.Changelog
SignificanceResult.withEdgeConfigEdgeCasesOnly()extension onPropTestConfig.Arbimplementation for partitioning an integer sum into a fixed count of non-negative summands.optionalVariableandnullableOptionalVariablearbitrary generator functions toDataConnectArb.SumPartitionArb.optionalVariableandnullableOptionalVariable.