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.

Sign up for GitHub

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

Make stub_with work with skip_optional_callbacks #96

Merged
merged 2 commits into from
Jul 9, 2020
Merged

Make stub_with work with skip_optional_callbacks #96

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad6f3ee
Make stub_with work with skip_optional_callbacks
flurin Jul 8, 2020
6ccea62
Update mox.ex
josevalim Jul 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/mox.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,9 @@ defmodule Mox do

for behaviour <- behaviours,
{fun, arity} <- behaviour.behaviour_info(:callbacks) do
stub(mock, fun, :erlang.make_fun(module, fun, arity))
if function_exported?(mock, fun, arity) do
Copy link
Member

Choose a reason for hiding this comment

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

We should only skip if they are optional though. Otherwise we can stop raising for functions that are required .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure because this checks if the mock function had been defined on the mock. So when you use skip_optional_callbacks with a specific set of callbacks it still works correctly.

Just checking if it's optional wouldn't cover that, right?

Copy link
Member

Choose a reason for hiding this comment

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

I see, I misunderstood. Looks good as is then!

Copy link
Member

Choose a reason for hiding this comment

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

Something along these lines:

for behaviour <- behaviours,
    optional = behaviour.behaviour_info(:optional_callbacks),
    {fun, arity} <- behaviour.behaviour_info(:callbacks) do
  if function_exported?(mock, fun, arity) or {fun, arity} not in optional do

We should add a test that stub_with fails when some non-optional callback is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that you should always implement the full behaviour in the Stub module, but enforcing this would possibly break a lot of current usage. Currently this test succeeds:

    defmodule PartialCalcImplementation do
      @behaviour Calculator
      def add(x, y), do: x + y
    end

    test "partial" do
      in_all_modes(fn ->
        stub_with(CalcMock, PartialCalcImplementation)
        assert CalcMock.add(1, 2) == 3

        assert_raise UndefinedFunctionError, fn ->
          assert CalcMock.mult(3, 4)
        end
      end)
    end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be happy to change that you can only stub full behaviour implementations of course.

stub(mock, fun, :erlang.make_fun(module, fun, arity))
end
end

mock
Expand Down
33 changes: 33 additions & 0 deletions test/mox_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,16 @@ defmodule MoxTest do
def exponent(x, y), do: :math.pow(x, y)
end

defmodule SciCalcFullImplementation do
@behaviour Calculator
def add(x, y), do: x + y
def mult(x, y), do: x * y

@behaviour ScientificCalculator
def exponent(x, y), do: :math.pow(x, y)
def sin(x), do: :math.sin(x)
end

test "can override stubs" do
in_all_modes(fn ->
stub_with(CalcMock, CalcImplementation)
Expand All @@ -603,6 +613,29 @@ defmodule MoxTest do
end)
end

test "stubs functions which are optional callbacks" do
in_all_modes(fn ->
stub_with(SciCalcMock, SciCalcFullImplementation)
assert SciCalcMock.add(1, 2) == 3
assert SciCalcMock.mult(3, 4) == 12
assert SciCalcMock.exponent(2, 10) == 1024
assert SciCalcMock.sin(0) == 0.0
end)
end

test "skips undefined functions which are optional callbacks" do
in_all_modes(fn ->
stub_with(SciCalcMockWithoutOptional, SciCalcImplementation)
assert SciCalcMockWithoutOptional.add(1, 2) == 3
assert SciCalcMockWithoutOptional.mult(3, 4) == 12
assert SciCalcMockWithoutOptional.exponent(2, 10) == 1024

assert_raise UndefinedFunctionError, fn ->
SciCalcMockWithoutOptional.sin(1)
end
end)
end

test "Leaves behaviours not implemented by the module un-stubbed" do
in_all_modes(fn ->
stub_with(SciCalcMock, CalcImplementation)
Expand Down
1 change: 1 addition & 0 deletions test/support/mocks.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
Mox.defmock(CalcMock, for: Calculator)
Mox.defmock(SciCalcMock, for: [Calculator, ScientificCalculator])
Mox.defmock(SciCalcMockWithoutOptional, for: [Calculator, ScientificCalculator], skip_optional_callbacks: true)

Mox.defmock(MyMockWithoutModuledoc, for: Calculator)
Mox.defmock(MyMockWithFalseModuledoc, for: Calculator, moduledoc: false)
Expand Down