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.
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. ^.^;
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.
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:
Separation of concerns
The ability to call the encrypt function from test, which means you don’t need to jump hoops
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
–
There was also this proposal thread (in which the package private was mentioned):
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.
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.
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.
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.
@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.
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.
Different sort of tests. Yes, the API tests are the most important. They are there to make sure you don’t break the contract of your public API, but I think it is a very bad idea to test implementation detail here. Your users don’t care about implementation detail and I don’t think these tests should either. But the implementation has to be tested somewhere.
Perhaps it is more idomatic in elixir to lift this out into a “private” module as @josevalim suggests but for me this is the same concept as having privates tests. Just a different implementation.[quote=“sasajuric, post:16, topic:9281”]
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.
[/quote]
