Tests that rely on private methods

Questions / Help
,
1

So I have a module that has a private method.

  defp encrypt_token(token) do
    :crypto.hmac(:sha256, BeffectWeb.Endpoint.config(:secret_key_base), token)
    |> Base.encode16(case: :lower)
  end

its used in a method that I would like to write a test for.

  def find_invite!(invite_token) do
    Repo.get_by!(User, invite_token: encrypt_token(invite_token))
  end

My test would need to create a fake user who has a invite_token that was encrypted the same way the private encrypt_token/1 encodes

test "find_invite!/1 returns a user with a given invite_token" do
  token = "SHOULD_BE_HMAC" 
  
  # token needs to be encrypted the same way as the encrypt_token
  # when passing it to the fixture, but I can't invoke the private method encrypt_token/1
  user = user_fixture(invite_token: token) # invite_token: encrypt_token(token)
  
  assert Accounts.find_invite!(token) == user
end

My question is what is the best pattern for this since I don’t want to test private functions but I do want to make sure my test uses the same methods as the real method so that way my tests don’t become brittle.

2

Extract that function to its own module and make it public?

You can even test the implementation of encrypt_token function that way if you want.

2 Likes
3

I’ve struggled with that bit about making it public, while yes in this case you could but then why have any private methods? Should I make a method public just because I want to test it?

4

Another very interesting point just made in slack was to make private methods available while only in tests.

i use the export_all erlang compilation directive to make them available in test suites
you just include erlc_opts: [ :export_all ] in mix.exs (for Mix.env == test or whatever)

  • alisdair

I very much like this idea. I wonder what bad side effects could arise from this.

2 Likes
5

It’s less about private vs public but also about separating dependencies. Having a own module to do the encoding does allow you to test the encoding in isolation as well as being able to inject an mock for the encryption module on your user handling. In other words find_invite should not really be concerned with how your token is created, it just needs to be able to match tokens.

4 Likes
6

Thats totally fare, My only hesitation for extraction was that nothing else in my app (“with exception to my test”) needed that method.

7

If it’s worth to test encrypt_token separately:

  • make it public in own module
  • test the module
  • in find_invite test, use the function so that you do not test the behavior of encrypt_token in find_invite test

However, if this is only place, and you do not want to expose it, then you should NOT use the implementation in tests. If you do so, then you’re not testing the behavior of encrypt_token! In this case, you have to have to manually create the encrypt token and use it on test, so that even you change the implementation, the whole behavior remains same.

8

You can also check mix to figure out if def or defp should be used, or make a test function wrapped in an if Mix.env == :test block at module-level that can be called as well.

I’m partial to the export_all approach for testing from my old erlang work though. ^.^

9

I’d just like to add this one here, because it’s where I got the ideas from: https://vimeo.com/15007792

1 Like
10

Another very good point made in slack is

I think the only way to test the contract you’ve established is to create the user via the same code-path that you would do it at runtime. I’m guessing there’s some type of create_user_by_invite/2 function or something

  • david.antaramian
1 Like
11

There is the package private available on hex.pm. Perhaps it helps you…

I’ve never worked with it, but I do have the feeling that it was announced here a while back, but I am unable to find the post…

12

I very much oppose making functionality public just because it needs testing.

I guess the reasoning from doing this is that it is only the public APIs that matter any way. This shines though in elixir documentation as well which only works for public functions (at least I think it still does)

Yes, you should test and document your public APIs, they are for the users of your module. But I think you should test and document the private functions as well. They are for the developers of the module. Just different audience.

I find myself having quite complicated private functions doing implementing the public API and it doesn’t make sense to make them public at all. You just want to make sure they are well tested. Not from an API point of view but because of implementation.

I do use the export_all directive, and generally in erlang I like to have the private testing done in the same module using eunit. I miss having the test code close a bit in elixir.

2 Likes
13

This exactly, that is why I put test functions in my modules that only exist at Mix.env == :test time. I guess that is the point of doctests now though, I use a lot of doctests just to keep them in-module. ^.^;

Doctests are awesome though.

1 Like
14

One thing to watch out for is that when you disable the private-ness of a function in the test env, you’re not just allowing your tests to access private functions, you’re letting all your code access all private functions, so if you have some code that is accidentally calling a private function, your test suite won’t catch the error.

2 Likes
15

I definitely wasn’t suggesting that. Just that in your case it just makes sense to me to separate it. @LostKobrakai sounded my opinions nicely.

But your test is using that method (function) nevertheless, so (again, for me) it just makes sense to make it public. Then again, this is probably the flaw of languages that only have two access modifiers. In Java, for example, I can just define the method package-private and let the test be in the same package, therefore giving access to all methods necessary. Yeah, I can’t believe I just compared Elixir to Java either.

By separating that function into another module, basically you get:

  1. Separation of concerns
  2. The ability to call the encrypt function from test, which means you don’t need to jump hoops
  3. The ability to create a mock of the Encryption module (e.g. via the amazing Mox library) if you don’t actually care about the implementation of the encrypt function

That indeed introduce the cost of indirection and opening possibilities of other modules “accidentally” calling that function. But yeah, most libraries I’ve seen in languages like this usually just define an Internal modules with big warnings that you shouldn’t depend on them, while still actually allowing others to access it. A trade-off that you need to weigh.

I wholeheartedly agree.

This too. It makes sense to test internal implementations, although I can see the test code would definitely change a lot as implementation changes, but not necessarily a thing you need to avoid. Btw, OP wants to use the private function, not test it, I don’t know if that makes a difference :slight_smile:

–

There was also this proposal thread (in which the package private was mentioned):

https://elixirforum.com/t/proposal-docp-for-private-function-documentation-and-doctests/3732

16

I mostly disagree with this. In my experience private functions are mostly internal details of the implementation, and the main reason of their existence is to organize the module internal code and make it easier to follow.

The API of the module is what the module guarantees, and this is IMO the only thing that should be tested.

In such cases, I find that there’s usually potential to split the module, and move complex internal functions as public functions of the new module, so they can be properly documented and tested.

Occasionally I do see the need to explain some private function, in which case I simply use a comment.

Finally, it’s worth noting that in Elixir, functions which are public but not meant to be invoked directly, should be marked with @doc false. This should indicate that a function is internal (even though marked public), and the clients should not depend on it directly. Such function will not appear in the generated doc, and the users will be unaware of its existence. People who read the code will see the function, but they will also see that it is marked with @doc false, and hence not meant to be invoked directly.

6 Likes
17

Actually I often (mostly back in erlang days) made a function called __tests__ that tested things in that module itself and it only existed at test time and was called by the testing framework. :slight_smile:

18

Most of the dissonance in these discussions come with the disagreement of what a private function means. To me, a private function is an implementation detail. I don’t care how it is named, I don’t care about the argument it receives. If I refactor my private functions and a test breaks, I have a bad test. This is also how the compiler is designed. A private function may not exist at all after the code is compiled.

That’s also why Elixir makes a distinction between code comments and documentation. Code comments are for those reading the source code.

In any case, if a private function has complexity to the point you feel you need to test it and/or document it, then it is most likely worth its own module. And you can still do so while keeping it private and using doctests. To provide an actual example, let’s see how the code above could be rewritten.

Let’s assume @polygonpusher’s code looks like this:

  defmodule User.Invitation do
    def find_invite!(invite_token) do
      Repo.get_by!(User, invite_token: encrypt_token(invite_token))
    end

    defp encrypt_token(token) do
      :crypto.hmac(:sha256, BeffectWeb.Endpoint.config(:secret_key_base), token)
      |> Base.encode16(case: :lower)
    end
  end

I would rewrite it to:

defmodule User.Invitation do
  defmodule Token do
    @moduledoc false

    @doc """
    Now I can doctest this too!
    """
    def encrypt(token) do
      :crypto.hmac(:sha256, BeffectWeb.Endpoint.config(:secret_key_base), token)
      |> Base.encode16(case: :lower)
    end
  end

  def find_invite!(invite_token) do
    Repo.get_by!(User, invite_token: Token.encrypt(invite_token))
  end
end

This way you keep everything in the same file, you provide a logical place for grouping all of the token functionality, you can write tests and doctests and you still don’t expose it to your “final” users.

PS: Note ex_doc now allows custom groups, so you can even have modules targeting different audiences and you can use the grouping functionality to break those apart in the UI.

15 Likes
19

@eahanson already pointed out that you can now have bugs that won’t be caught in tests. It is worth mentioning that @compile :export_all emits warnings from OTP 20 on:

warning: export_all flag enabled - all functions will be exported
  lib/elixir/test/elixir/code_formatter/general_test.exs:3

FWIW, the warning and functionality comes from the Erlang compiler, Elixir has nothing to do with it.

3 Likes
20

Yes, we are likely arguing semantics. I agree here to. Private functions are for implementation detail, I don’t care how they are named. And if an “interface test” breaks if I refactor a private function then it is a bad test.

But I feel that in a number of cases the implementation detail also needs to be tested and I don’t want these test mingled with my other testing because they test different things.

You have a good suggestion to lift this out into a “private” module and that seems like a good solution. I don’t see this as much different than having a “private” section of the same module though, except than perhaps it is a better fit for how elixir do things.

1 Like