Skip to content
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.

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

test: Generate random, increasing message IDs for example data by default #343

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Oct 27, 2023

This allows eg.streamMessage and eg.dmMessage to be used in
contexts where it's important that the message IDs don't collide,
or even that they're increasing, without the test having to explicitly
give specific message IDs when no other fact about them is relevant.

As a result, we get to cut numerous explicit message IDs that are now redundant.

@gnprice
Copy link
Member Author

gnprice commented Oct 27, 2023

I ran flutter test 100 times on this with no failures:

$ time { for i in {1..100}; do flutter test; done; }
00:09 +655 ~7: All tests passed!                                                 
[… 98 more successes …]
00:10 +655 ~7: All tests passed!                                                 

time: 1128.545s wall (3879.761s u, 898.730s s)

So it looks like we don't have any existing tests that flake from this randomness. I don't think there are any super likely patterns we'd use that would introduce such flakes, but if we start seeing them then we can adjust how this works.

@gnprice gnprice mentioned this pull request Oct 27, 2023
@gnprice
Copy link
Member Author

gnprice commented Oct 27, 2023

(This was prompted by #317 (comment) — an existing test that wasn't passing explicit message IDs started to need them, because an unrelated change caused more of the app to start caring that the IDs of different messages were different. After this PR, that change doesn't affect that test.)

…ault

This allows `eg.streamMessage` and `eg.dmMessage` to be used in
contexts where it's important that the message IDs don't collide,
or even that they're increasing, without the test having to explicitly
give specific message IDs when no other fact about them is relevant.

Many call sites that were passing explicit message IDs no longer need
to do so after this change.  We'll clear those out in the next commit.
@chrisbobbe chrisbobbe merged commit 256be9a into zulip:main Oct 27, 2023
1 check passed
@chrisbobbe
Copy link
Collaborator

Thanks, this is helpful! Merged.

@gnprice gnprice deleted the pr-test-messageids branch October 30, 2023 18:38
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