Testing Private Functions

Chat / Discussions
1

I always wind up not having private functions in my modules because you can’t test them. I really have a strong preference for TDD and - at this point - I can barely write a function without having a test first. I recently had a function that is stupid simple:

def char_value(char), do: char - ?@

The point is to convert an uppercase letter to a numerical value (i.e. A == 1, B == 2, etc…). This is just a helper function that I’d never want to expose outside of the module, it’s logic is very simple, but It was tricky (for me) to figure out that ?@ == 64 and gave me the value I want. I suppose I could have just made the function char - ?A + 1 but this is just an example of when a function (which should be private IMO) benefits from being tested.

Am I just thinking about private functions wrong? To me, a private function is a helper for simplifying the complexity of public functions in the module. Should I just think of private functions as functions that are themselves simple enough to not need a test?

2

You could start a holy war with questions like this! :upside_down_face:

I’m a TDDer myself and I do not see value in testing private functions in that they mostly appear later as I’m refactoring. However, sometimes you can find yourself with a single function that does a lot of stuff. In those cases, you can always move these function to a sub “private” module (@moduledoc false) and write tests for them there (though personally when I do this I just end up deleting the tests when I’m done as I just want tests for my public interface).

In the case of your example, that is a candidate to just be moved to an external helper module. It’s one of those functions that does one small thing very well, so there’s no harm in other people using it, especially since it has tests! However, if you really want to keep it private in your current module, then the tests for whatever public function you are using this for will certainly cover it. If you really want to start with just a test for that one line, then I’d do what I suggested above (and sanctioned by Sandi Metz herself ;)), make it public, write a test for it and when you’re done, or at least when you have public functions that consume it, delete the test and make it private.

Otherwise, keeping tests for things at this level of granularity makes reafactorting a massive pain. It’s also exhausting for readers (although I guess no one reads code anymore and LLMs don’t get exhausted /s)

3 Likes
3

In isolation it surely makes sense to have tests for the behaviour. But given finite time it makes more sense to test from the perspective of what the software brings value to instead of the details. Like this char_value function will be used somewhere. Depending on how important it is to that user testing will include what char_value does.

Then there are layers in software. Just like with third party dependencies you can have first party dependencies by having modules in your codebase sitting at lower levels of abstraction tested in isolation to the higher levels - e.g. to keep test complexity down. That‘ll be different modules though, which can have public functions. If you‘re looking for that I‘d strongly consider not leaving them inline with the higher level abstraction code.

1 Like
4

For a concrete test example:

defmodule MyApp.CharConverter do
    def uppercase_to_numerical(char) when char in ?A..?Z do
      char_value(char)
    end

    defp char_value(char), do: char - ?@
  end

  defmodule MyApp.CharConverterTest do
    use ExUnit.Case, async: true

    alias MyApp.CharConverter

    describe "uppercase_to_numerical/1" do
      test "converts A to 1" do
        assert CharConverter.uppercase_to_numerical(?A) == 1
      end

      test "converts Z to 26" do
        assert CharConverter.uppercase_to_numerical(?Z) == 26
      end
    end
  end

As for private function thinking I also think of them in terms of keeping implementation details private for safer refactoring if needed later.

1 Like
5

TW: controversial statements ahead :stuck_out_tongue:

In languages like Elixir, where you don’t get truly private modules (another example of that would be Ruby), private functions don’t matter as much as people give them meaning. Say, you write a code like this:

defmodule Note do
  def normalize_title(title)
    title
    |> normalize_case()
    |> replace_illegal_characters()
  end

  def normalize_case(string), do: [something]
  def replace_illegal_characters(string), do: [something]
end

In my experience there is a 95% chance someone will call that out during the code review, because normalize_case and replace_illegal_charecters should be private. But do it like that, and nobody will bat an eye :wink:

defmodule Note do
  def normalize_title(title)
    title
    |> StringHelper.normalize_case()
    |> StringHelper.replace_illegal_characters()
  end
end

Now it’s okay. You can test the StringHelper in isolation, because its functions are “legally” public. This is encountered in codebases more often than people think.

This matters if you are writing a library, i.e. you don’t control how and by whom your code is used. In this scenario it makes sense to try to keep public API surface as small as possible. Make the function public and you never know if someone uses it, so even renaming it is a breaking change.

But in a classic “work project” it’s pretty much a cargo cult. You control the “supply” and “demand” side, both definition and usage. You can simply grep a project for usages before making a change - and it will be safe. Private vs public remains mostly a guideline.

And if someone needs your private function outside of the module, they will simply make it public. Another things I’ve witnessed dozens of times.

The only thing maybe keeping the sense of having private functions id adhering to another cargo cult - that every public function needs to be unit testing. Then having some function private serves the purpose of avoiding too many, too brittle tests.

1 Like
6

I know both future Vidar and Claude are both forgetful and clueless, so providing a private vs public even as a guideline do provide value for me. Sure keeping the public API consistent can be checked otherwise, but for me using private is a straight forward solution. As for Claude I have a hard rule to respect the intent of private functions which is a clean way to do it. Trying to explain Claude which functions are actually public API, and which ones can be changed for every single module in a refactoring, does not seem very attractive to me.

Private is a clear signal not to use the function outside of the module. If someone decide to use it anyway then that is on them, and any later refactoring that breaks their code will be their mess to clean up.

1 Like
7

I don’t think there is any real tension here. TDD is a method for writing code. Private functions are a convention for maintaining code. If TDD helps you build out working implementations, then write it that way and then make the functions private later and remove the tests.

But in my view the value of TDD is primarily as a tool to help me think about the API (since writing the test requires defining the inputs outputs and name). dbg is way more useful for sanity checks that I’ve implemented something correctly, as you describe in your example.

1 Like
8

I think this is the answer. If I want to use a private function, I have to test it through the interface I’m building. That’s a really good constraint for the type of private functions I’m thinking about.

9

It is completely reasonable to want to test private functions. Making function invisible for other modules doesn’t mean that it doesn’t need testing. It is possible to test private functions with Repatch.

For example, you want to test to_number/1 from this module

defmodule Calculator do
  def add(left, right) do
    to_number(left) + to_number(right)
  end

  defp to_number(x) when is_binary(x) do
    String.to_float(x)
  end

  defp to_number(x) when is_number(x) do
    x
  end
end

First, initialize Repatch in test/test_helper.exs

ExUnit.start()
Repatch.setup()

Second, in test do something like

defmodule CalculatorTest do
  use ExUnit.Case, async: true
  use Repatch.ExUnit
  import Repatch, only: [private: 1]
  
  setup do
     Repatch.spy(Calculator)
  end

  test "to_number/1" do
    assert 1234.0 == private Calculator.to_number("1234")
    assert 1234.5 == private Calculator.to_number("1234.5")
    assert 1234 == private Calculator.to_number(1234)
    assert 1234.5 == private Calculator.to_number(1234.5)
  end
end
10

I don’t think this is unquestionably true. Maybe debatable, but I’d strongly argue that one of the underrated benefits of unit tests is that they document what a module does. If you maintain tests for private functions you are now also documenting how it does it. Not terrible, but why do that when you can write a test for the caller than exercises the private function?

3 Likes
11

I understand your point and I find it completely valid and right. However, I personally don’t write unit tests as documentation, and I also use ExUnit as a generic testing framework for property testing, behavior testing, performance testing, stress-testing, fuzzing etc. Even for functionality tests, I find it useful to be able to test private functions

For example, I have a module

defmodule Statistics do
  def top_users(source, amount) do
    source
    |> download()
    |> parse_stream()
    |> extract_users()
    |> calculate_rating()
    |> take_top(amount)
  end
  
  ...
end

All functions except top_users are private, because I don’t want other modules to rely on this logic. But I want to be able to test them to make sure that each step of this pipeline is free of errors and covers every corner case I come up with.

Other options are to

  1. Make these functions public (or move them to separate module). Then some other module might be able to call them, creating a dependency I don’t want
  2. Test only top_users. Then I will have to create an input for every possible corner-case, having a lot of work to do

Again, writing code in a way that you never have to test a private function, or writing a code in a way that every ExUnit test is more than just a test, but it is a documentation of the feature, is good and I accept these approaches. However, I prefer to write the code the other way and I prefer to set up the limits of the system myself

1 Like
12

Isn’t the work you are doing when you test each private function separately?

13

Well, if I calculate a rating like user.games_won / user.games_total, I need to test that it doesnt fail when user.games_total is zero.

Testing a private function looks like

result = Enum.to_list(calculate_rating([%{id: 1, games_won: 0, games_total: 0}]))
assert [%{id: 1, rating: 0}] = result

Testing this case from top_users would look like: I set up a mock server, which replies to an HTTP request with an encoded file, which contains a user which has games_total = 0 and then I pass it in the top_users(source_mock_server_url, 1) and take top one and compare.

And if some step in the pipeline above the calculate_rating fails, this test case would fail too

14

I see, yes, you gain some benefits from isolation of those tests that helps pinpoint exactly where something went wrong, avoid duplicating setup etc. I guess for me those are not real pain points but you’ve convinced me if a team member wanted to do something like that it might make sense, but I would probably still want to segregate them from the “true” unit tests.

15

Yikes, perversive use of public functions in an obsessive quest of TDD, have you ever thought about performance and the cost of compiling public functions.

I hope you find some balance in your life. :laughing:

FWIW you can have a public api that calls private functions, the best of both worlds.

1 Like
16

Interesting, I know you weren’t talking to me but no, I never have! Not that it’s a problem for me but it’s a good thing to keep in mind.

FWIW you can have a public api that calls private functions, the best of both worlds.

Amusingly, I think this is the point almost every responder so far (including myself) was trying to express, each in their own very unique way :sweat_smile:

2 Likes
17

Private functions to me should be:

  • An implementation detail;
  • A way to enforce boundaries.

If you find yourself needing to test private functions then it’s time to ask yourself or the team these questions:

  • Is it time to promote these functions into a public API and accept that any other module in the app can call them now?
  • Are you getting obsessive about testing every single thing? Indeed testing the public API and letting a private function fail an assertion as a part of that test is quite fine and not a reason to make private functions public.

I don’t agree with the argument that only libraries should carefully control what’s public. I’m currently working hard, together with Claude, to untangle quite the spaghetti code. It’s painful and it really drives home the point that scope and access creep are real problems.

3 Likes
18

BTW @hauleth wrote a package to test private functions years ago: GitHub - hauleth/ex_unit_embedded: Define ExUnit tests alongside your private functions to test them. · GitHub

No idea if it still works though.

19

It should still work. There was no fancy things, so it is pretty much done as is.

20

Exactly that. Private functions should be used (because compiler theoretically can optimise them better, especially now with type tracking), however if you need to test them, then there is probably something wrong in your API.

How to test that a function is called just once