Skip to content

Conversation

@dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Dec 23, 2025

This PR refactors of the dataconnect arbitrary data generation for Struct and ListValue types for testing. The changes streamline the complex logic involved in creating diverse and deeply nested test data, enhancing the overall maintainability and clarity of the property-based testing framework. By consolidating common patterns and introducing a dedicated generator class, the codebase becomes more organized and easier to extend.

Highlights

  • Refactored Arbitrary Data Generation: The core logic for generating arbitrary Struct and ListValue instances has been significantly refactored to reduce code duplication. This involved introducing a new CompositeValueArb abstract class and a StructOrListValueGenerator helper.
  • Consolidated Nested Value Generation: The mechanism for generating nested Struct and ListValue instances has been centralized within the StructOrListValueGenerator, replacing previous duplicated logic and helper functions.
Changelog
  • property/arbitrary/proto.kt
    • Removed unused imports (withAddedField, withAddedListIndex, Sample, withEdgecases) and added new ones (DataConnectPathSegment, withAddedPathSegment).
    • Modified value function to accept an optional structKey parameter, which is propagated to struct and listValue calls.
    • Updated listValue and struct function signatures to include new parameters (structKey, structSize, listSize) and adjusted their constructor calls to use new parameter names (sizeRange, depthRange, etc.).
    • Replaced the individual StructArb and ListValueArb classes with a new abstract CompositeValueArb base class.
    • Introduced GenerateCompositeValueProbabilities data class to group probability parameters for cleaner function signatures.
    • Implemented StructOrListValueGenerator to centralize the core logic for generating Struct and ListValue instances, including handling nesting and edge cases.
    • Refactored StructArb and ListValueArb to extend CompositeValueArb and delegate their generation logic to the new StructOrListValueGenerator.
    • Removed NextNestedValueResult data class and NextNestedValueCase enum, along with the RandomSource.nextNestedValue extension function, as their functionality is now integrated into the new generator architecture.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a significant and well-executed refactoring of the Struct and ListValue arbitrary generators. By introducing the CompositeValueArb abstract class and the StructOrListValueGenerator, you've successfully eliminated a large amount of duplicated code and centralized the complex logic for generating nested data structures. The new implementation is more powerful, allowing for finer control over generation parameters like nested list/struct sizes, and the approach to handling edge cases is more robust and flexible. My only suggestion is to encapsulate the numerous probability parameters into a data class to improve the readability and maintainability of the generator functions.

@dconeybe dconeybe marked this pull request as ready for review December 23, 2025 18:59
@firebase firebase deleted a comment from gemini-code-assist bot Dec 23, 2025
@firebase firebase deleted a comment from github-actions bot Dec 23, 2025
@firebase firebase deleted a comment from google-oss-bot Dec 23, 2025
@firebase firebase deleted a comment from gemini-code-assist bot Dec 23, 2025
@google-oss-bot
Copy link
Contributor

Coverage Report 1

Affected Products

No changes between base commit (a9f0d20) and merge commit (ca74a5b).

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/tDhowps6Rm.html

@dconeybe dconeybe merged commit e99cb3a into main Dec 23, 2025
42 checks passed
@dconeybe dconeybe deleted the dconeybe/dataconnect/ProtoStructListValueArbRefactor branch December 23, 2025 21:35
@github-actions github-actions bot mentioned this pull request Jan 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants