Skip to content

Conversation

@groyoh
Copy link
Contributor

@groyoh groyoh commented Oct 17, 2025

Context

b3131f4 introduced a shared_examples to improve consistency between the event stores tests. The following methods were not tested in the shared_examples though:

  • prorated_events_values
  • prorated_sum
  • grouped_prorated_sum
  • weighted_sum
  • grouped_weighted_sum
  • prorated_unique_count
  • grouped_prorated_unique_count
  • prorated_unique_count_breakdown

Furthermore, the weighted_sum_breakdown method was not tested.

Description

This moves the following methods tests to the shared_examples:

  • prorated_events_values
  • prorated_sum
  • grouped_prorated_sum
  • weighted_sum
  • grouped_weighted_sum

Tests were also added for the weighted_sum_breakdown method.

It looks like the behavior of the prorated unique count methods actually differs from the Postgres and Clickhouse stores so they were not moved to the shared_examples.

This also fixes an issue with the weighted_sum_breakdown method of the Clickhouse store which returned the second_duration as string rather than int. That's because date_diff actually returns an Int64 and ActiveRecord transform that into a String so we call #to_i to ensure consistency between implementations:

lago-api(staging)> Clickhouse::BaseRecord.connection.select_one("SELECT 1::Int64")
=> {"CAST('1', 'Int64')" => "1"}
lago-api(staging)> Clickhouse::BaseRecord.connection.select_one("SELECT 1::Int32")
=> {"CAST('1', 'Int32')" => 1}

Note that the weighted_sum_breakdown method of the Clickhouse stores returns timestamps with a 5 decimal precision rather than 3 as in the other methods.

@groyoh groyoh changed the base branch from main to fix/event-store-connection October 17, 2025 13:22
@groyoh groyoh marked this pull request as ready for review October 17, 2025 13:24
@groyoh groyoh changed the title test(event): Add more event store tests to shared_example test(event): Add more event store tests to shared_examples Oct 17, 2025
@groyoh groyoh changed the title test(event): Add more event store tests to shared_examples test(event): Move more event store tests to shared_examples Oct 17, 2025
Copy link
Contributor

@ancorcruz ancorcruz left a comment

Choose a reason for hiding this comment

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

looks good.

@groyoh
Copy link
Contributor Author

groyoh commented Oct 17, 2025

Thanks for the review @ancorcruz 🙏

Base automatically changed from fix/event-store-connection to main October 20, 2025 09:53
@groyoh groyoh merged commit 6868c5b into main Oct 20, 2025
14 checks passed
@groyoh groyoh deleted the test/event-store branch October 20, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants