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

Fix several allowances with same keys #6

Merged
merged 2 commits into from
Jul 22, 2024
Merged

Fix several allowances with same keys #6

merged 2 commits into from
Jul 22, 2024

Conversation

am-kantox
Copy link
Contributor

In mox (including but not limited to the latest version,) when one adds several deferred mocks to the list of allowed processes, this results in the following allowances passed to the private fix_resolved/2

[
  {#PID<0.237.0>, %{MyMod1.Mox => #PID<0.221.0>}},
  {#PID<0.237.0>, %{MyMod2.Mox => #PID<0.221.0>}}
]

and gets effectively ruined by Map.new/1 into

%{#PID<0.237.0> => %{MyMod2.Mox => #PID<0.221.0>}}

This PR provides a proper map construction from a proplist above by merging maps.


The test could have been done better, please guide me if you want it to be prettified.

Thanks!

end)

send(parent_pid, :done)
f.(f).()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This works for doing recursion, but it's gonna be clearer to use a separate private function. Also, do you actually need to receive multiple times here, or just twice? You could do:

for _ <- 1..2 do
  receive ...
end

to avoid the recursion altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for _ <- 1..2 do
  receive ...
end

What have I used instead of a brain for my whole life?! :)

Thanks, of course, pushed.

@whatyouhide whatyouhide merged commit d6b379f into dashbitco:main Jul 22, 2024
3 checks passed
@whatyouhide
Copy link
Collaborator

Fantastic catch, thanks @am-kantox 💟

@whatyouhide
Copy link
Collaborator

Released v0.3.2.

@am-kantox am-kantox deleted the fix_several_allowances_merging branch July 22, 2024 17:23
@am-kantox
Copy link
Contributor Author

Released v0.3.2.

Would you want me to backport it to Mox v1.1.1? This would be a one-liner, but I believe it’d make sense.

@am-kantox
Copy link
Contributor Author

Released v0.3.2.

FWIW, feel free to cherry-pick it from https://github.com/dashbitco/mox/compare/v1.1.0...am-kantox:mox:fix_several_allowances_merging?expand=1 (I cannot make a PR to a tag.)

@whatyouhide
Copy link
Collaborator

@am-kantox we might do a Mox release but for now you should be able to just mix deps.update nimble_ownership since Mox will accept 0.3.2, no?

@am-kantox
Copy link
Contributor Author

@whatyouhide mox does not have any release that uses nimble_ownership yet, but technically I am fine grabbing mox from github: "…" source.

@whatyouhide
Copy link
Collaborator

Aaaaah I didn't get that this issue is there in Mox as well. @josevalim should we just make a Mox release that uses nimble_ownership then?

@josevalim
Copy link
Member

Yeah, it is probably time for a minor ownership release!

@am-kantox
Copy link
Contributor Author

Not that I was nudging anyone, this is just a kind reminder that if we finally had nimble_ownership v0.4.0 and consequently mox v1.2.0, some toucans in Barcelona Zoo would be happy.

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