Skip to content

ScheduledMerges: also test nested unions and arbitrary configs#814

Merged
jorisdral merged 6 commits intomainfrom
mheinzel/prototype-tests-nested-union
Mar 24, 2026
Merged

ScheduledMerges: also test nested unions and arbitrary configs#814
jorisdral merged 6 commits intomainfrom
mheinzel/prototype-tests-nested-union

Conversation

@mheinzel
Copy link
Collaborator

Description

We had already noted that testing nested unions (unions of tables that are themselves result of a union) would be a good idea. Also, after making some LSM parameters configurable in #716, it seems sensible to test more than just the default values.

As a preparation for further changes to the prototype, this PR addresses these points and makes small improvements to the invariants and properties we assert.

@mheinzel mheinzel force-pushed the mheinzel/prototype-tests-nested-union branch from 458fbf9 to dabce7d Compare February 16, 2026 13:18
@mheinzel mheinzel force-pushed the mheinzel/prototype-tests-nested-union branch from dabce7d to 182cd28 Compare February 20, 2026 16:20
@mheinzel
Copy link
Collaborator Author

Small change: I used newtypes for test inputs now, making their Show instances much more readable than with the NonEmpty { getNonEmpty = ... } sprinkled in. This made it easier to understand the counterexamples from failing properties.

@mheinzel mheinzel force-pushed the mheinzel/prototype-tests-nested-union branch 2 times, most recently from 0fb5b34 to 83f94db Compare March 3, 2026 15:21
@mheinzel mheinzel force-pushed the mheinzel/prototype-tests-nested-union branch from 83f94db to 9e43f0e Compare March 3, 2026 17:36
Copy link
Collaborator

@jorisdral jorisdral 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 to me! Good that we are improving the prototype tests 😄. I've left some comments but they shouldn't be blockers for this PR.

@mheinzel mheinzel force-pushed the mheinzel/prototype-tests-nested-union branch from 9e43f0e to b7bc5c2 Compare March 17, 2026 16:59
bufferSize :: Buffer -> Int
bufferSize = Map.size
-- | Flush a write buffer. In the real implementation, this involves IO.
-- Note that we should not never flush an empty write buffer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-- Note that we should not never flush an empty write buffer.
-- Note that we should not ever flush an empty write buffer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

so we should always flush empty write buffers? 😜

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, I will fix that as part of the next PR.

@jorisdral jorisdral added this pull request to the merge queue Mar 24, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 24, 2026
@jorisdral jorisdral added this pull request to the merge queue Mar 24, 2026
Merged via the queue into main with commit bb526d1 Mar 24, 2026
32 checks passed
@jorisdral jorisdral deleted the mheinzel/prototype-tests-nested-union branch March 24, 2026 09:17
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.

2 participants