Property based testing with StreamData #8
Conversation
|
Thanks for the feedback @michaelstalker! I addressed your comments and I also refactored the vote calculation module to rely on a |
| check all(users <- user) do | ||
| {_event, winner} = PointingParty.VoteCalculator.calculate_votes(users) | ||
|
|
||
| if is_list(winner) do |
There was a problem hiding this comment.
Since the entire set of assertions gets wrapped in a conditional, I think it would be better to set up your preconditions in such a way that winner will always be a list.
There was a problem hiding this comment.
So in order to create such a precondition, we have to ensure that the point value for each user shake out such that there is a tie. Given that StreamData is generating a random integer in the given range, is there anyway to control for that? I.e. to ensure that, of the random set of users, the random points for each user are each a different integer?
There was a problem hiding this comment.
The first thing you'll need to do is to change your map size so the map will always have at least two elements. One-element maps will never result in a winner value that is a list.
After that, there are few strategies you could employ:
- Use
filterto discard all maps whose users all have the same point values - Use
bindorbind_filterto ensure that the second user's point value is different than the first user's point value. I'm not 100% sure how to do this, since I've never usedbindbefore. I'm also not 100% sure this would work. - Generate a one-user map. That user will have a point value in
[0, 1, 3, 5]. Then, generate another one-user map. When you do this, your generator would remove the first user's point value from the list of possible point values for the second user. Then, you'd generate a map of zero or more users with a random point value. Finally, you'd join all the maps.
I'm fine leaving the conditional for now, since it won't be trivial to craft a generator that guarantees the results we want. I'd like to see if we can make a generator like that prior to the conference, though.
| calculated_votes | ||
| |> Enum.sort_by(&elem(&1, 1)) | ||
| |> Enum.take(2) | ||
| |> Enum.map(&elem(&1, 0)) |
There was a problem hiding this comment.
The code from lines 69-80 is the same as the application code. That's generally not ideal because it won't catch bugs in the original algorithm. Is there a different way we can express this code in the test?
There was a problem hiding this comment.
Yea I was definitely wondering this myself! I took a stab at finding a slightly different way of expressing it, but there is at least one line that is still the same as application code. Let me know what you think.
| check all(users <- user) do | ||
| {_event, winner} = PointingParty.VoteCalculator.calculate_votes(users) | ||
|
|
||
| if is_list(winner) do |
There was a problem hiding this comment.
The first thing you'll need to do is to change your map size so the map will always have at least two elements. One-element maps will never result in a winner value that is a list.
After that, there are few strategies you could employ:
- Use
filterto discard all maps whose users all have the same point values - Use
bindorbind_filterto ensure that the second user's point value is different than the first user's point value. I'm not 100% sure how to do this, since I've never usedbindbefore. I'm also not 100% sure this would work. - Generate a one-user map. That user will have a point value in
[0, 1, 3, 5]. Then, generate another one-user map. When you do this, your generator would remove the first user's point value from the list of possible point values for the second user. Then, you'd generate a map of zero or more users with a random point value. Finally, you'd join all the maps.
I'm fine leaving the conditional for now, since it won't be trivial to craft a generator that guarantees the results we want. I'd like to see if we can make a generator like that prior to the conference, though.
|
|
||
| describe "calculate_votes/1" do | ||
| setup do | ||
| points_map = fixed_map(%{points: integer(1..5)}) |
There was a problem hiding this comment.
It would probably be better to use this, to match what the user does in the real app more closely:
points_map = fixed_map(%{points: member_of(0, 1, 3, 5)})There was a problem hiding this comment.
The Elixir code uses [0, 1, 3, 5]. The HTML we create in the JavaScript uses [1, 2, 3, 5]. We should probably iron that out. Maybe 1..5 is fine for now, as long as we make a to-do to fix it before the workshop.
| end | ||
|
|
||
| property "winning value is a list or a integer", %{users: users} do | ||
| check all users <- users do |
There was a problem hiding this comment.
💯 for renaming the property parameter from user to `users. I discourage people from reassigning values to parameters. (Martin Fowler used to have a refactoring just for this, but reassigning to parameters is now a general case of what he calls "Split Variable.") It can create some confusion about what exactly a variable refers to. What would you think about this?
check all user_map <- users do
SophieDeBenedetto
force-pushed
the
test-vote-calculation-with-stream-data
branch
from
5a2df4b to
c89a28a
Compare
michaelstalker
force-pushed
the
test-vote-calculation-with-stream-data
branch
from
b058fb0 to
9b2705e
Compare
Extracted vote calculation logic into its own module and tested with StreamData
@michaelstalker my question for you is: how much property testing + stream data content do you think we need for the workshop on day 2? How many such tests do we want to see participants write? I'm thinking probably more than what I have here? If so, what other opportunities for property-based testing can we find in our app--more cases for vote calculation and/or other features to test?