test: Generate random, increasing user IDs for example data by default #425
Conversation
test/example_data.dart
Outdated
| userId: userId ?? 123, // TODO generate example IDs | ||
| userId: userId ?? _nextUserId(), // TODO generate example IDs |
There was a problem hiding this comment.
This TODO is resolved now, I think :-)
| /// If user ID `userId` is not given, it will be generated from a random | ||
| /// but increasing sequence. |
There was a problem hiding this comment.
My first thought was that user IDs don't need to be ordered like message IDs do. But I guess when writing lists of users in a DM conversation one needs them sorted (unless using some function that does the sorting itself), so it's convenient for the order to be deterministic.
There was a problem hiding this comment.
Hmm, yeah, I guess the ordering isn't even necessary for my use case in #410, which prompted this PR. The eg.users there aren't being used to identify a DM conversation.
But in #410 I did find it nicer to let the caller omit a user ID, and for that we needed eg.user() to give unique user IDs, and for that it was convenient to follow the pattern of message IDs in example_data.dart.
There was a problem hiding this comment.
Sure. If for something else we want to generate random distinct IDs without constraining the ordering, zulip-mobile has a handy example we can follow, implemented by Ray years ago.
(Rereading it, I guess it can't just be transcribed from JS to Dart, because it relies on JS's sparse arrays. But a sparse array is basically just a map with int keys, plus an int for the length.)
Like we did for message IDs in 13e54f6. This time there aren't any (or anyway I didn't find any) call sites where we could simply remove a `userId` argument and the test would still work. I've made the one simplification to a test that seemed very easy and minimal.
chrisbobbe
force-pushed
the
pr-test-random-user-ids
branch
from
5745c8f to
bb78e6c
Compare
|
Thanks for the review! Merged, with that fix. |
Like we did for message IDs in 13e54f6.
This time there aren't any (or anyway I didn't find any) call sites where we could simply remove a
userIdargument and the test would still work. I've made the one simplification to a test that seemed very easy and minimal.