5

I have this method, which has multiple private calls: GetConfigStatuses(), ApplyFilters(), GetConfigListInfo().

Since these do not expose anything public I cannot test them like I would a public. And I am not changing access level, that would be a smell.

So the only thing I can actually Verify and Assert in this method is the GetPackages() call. That's at least how I see it.

But I am not sure, because I am fairly new to mocking things up :)

Below is the method I am testing, and one of my tests.

Does things look ok ? Thank you :)

public IEnumerable<UIConfigListInfo> GetUiConfigs(string segment, UIConfigInfo uiConfigInfo) { try { var configStatus = string.IsNullOrEmpty(uiConfigInfo.Status) ? "Active" : uiConfigInfo.Status; var configListInfoList = new List<UIConfigListInfo>(); foreach (var currentConfigStatus in GetConfigStatuses(configStatus)) { var filters = ApplyFilters(segment, uiConfigInfo.Name.ToLowerInvariant(), currentConfigStatus.ToLowerInvariant(), uiConfigInfo.Version); var packages = _storageRepository.GetPackages(filters, Order.Descending, configStatus).ToList(); foreach (Package package in packages) { try { //TODO: comment this. var configListInfo = GetConfigListInfo(package, uiConfigInfo.Version, uiConfigInfo.Name, currentConfigStatus); //TODO: comment this. GetConfigDefinition(package.Data, configListInfo); configListInfoList.Add(configListInfo); } catch (Exception ex) { //TODO: log! Throw ? } } } return configListInfoList; } catch (Exception ex) { //... } }

And the test below. It's important to notice my Service class only has one public dependency (the IStorageRepository). The GetPackages() returns a List<Package>.

[TestMethod] public void TestMethod1() { //Arrange var mockStorageRepository = new Mock<IStorageRepository>(); mockStorageRepository.Setup(s => s.GetPackages(It.IsAny<List<string>>(), Order.Ascending, "Active")); var sut = new UIConfigService(mockStorageRepository.Object); //Act sut.GetUiConfigs("somesegment", new UIConfigInfo { Name = "TestFun" }); //Assert mockStorageRepository.Verify(mock => mock.GetPackages(It.IsAny<List<string>>(), Order.Descending, "Active"), Times.Once); }
Improve this question
8
  • what do test-coverage results say, is code in these private methods shown as covered? Commented Sep 13, 2017 at 7:48
  • @gnat it actually says that my private calls are really good covered (50-85%). That's good right ? :) Commented Sep 13, 2017 at 8:13
  • 2
    it's up to you to decide. Look into code that isn't yet covered, is it important? If yes, you need to try harder. if not, you're good Commented Sep 13, 2017 at 8:20
  • 2
    Look at the code that isn't covered, and write a test-case that only uses the public methods to get there. If you can't, then you can comment / assert that it is unreachable Commented Sep 13, 2017 at 8:24
  • 2
    Strive for 100% coverage. Almost every bug I have ever seen in production was in code that was never tested. Commented Sep 13, 2017 at 16:49

3 Answers 3

Reset to default
14

Unit Tests verify the public observable behavior of a unit of code.

Distribution of this behavior to a bunch of private methods is implementation detail which you do not test. The reason is that you don't want to fail your test when changing this implementation details for some reason, most likely because you're adding more behavior.

Unchanged tests are the most reliable proof that the desired behavior still exists after a change on the production code.

Improve this answer
10
  • 4
    Of course there's also a complementary view: unit tests can also be developer tests that verify the code works how the developer expects it to work. This (legitimate!) testing goal is more about debuggability than about the public interface. For these developer tests, breaking encapsulation and accepting increased test fragility might be OK. Commented Sep 13, 2017 at 10:39
  • 3
    I argue that. This kind of implementation tests cause the bad reputation of unittests being fragile and hindering changes. Off cause you may use them in private but you should not commit them to a common SCM repository you share with other developers. Commented Sep 13, 2017 at 11:34
  • 4
    @amon if there's a need to test a private method, there's a good chance you have another problem: the class is too big and should be broken up. I can't imagine a situation where the class has high cohesion/follows SRP but you still need to test the private methods. Commented Sep 13, 2017 at 12:41
  • 2
    @TimothyTruckle Your comment sounds like tests should be thrown away even if they seem to have value. I'll concede that it's better and more stable to test through the public interface. But I think it's more important to have good tests than it is to have elegant tests. For example, white-box testing techniques have a bad reputation because they necessarily peek behind the curtain and might be more fragile. On the other hand, these tests are often much, much better at finding bugs + preventing regressions. Those can be difficult trade-offs. Principles are useful, but not if followed blindly. Commented Sep 13, 2017 at 14:09
  • 2
    As a general practice, I test every method I write, regardless of its accessibility. That practice has been made more difficult by people like you who dismiss white-box testing as useless. The current crop of unit testing tools no longer adequately supports such white box testing. For example, MSTest deprecated their "accessor" functionality that made white box testing not only possible but convenient; you now have to either make all of your methods public, put them in a utility class, or resort to horrible workarounds like reflection, virtual methods or proxy classes. Commented Sep 14, 2017 at 0:22
2

I'll step up and advocate the completely opposite answer.... If ApplyFilters() has enough logic to be considered a unit then it should have its own unit tests that test it directly

Feeding all the private method test cases through the public method makes your tests fragile in a different and more impactful way.. if you change the logic of the public method you are likely to break tests whose purpose is to test the behavior you the private method. As the number of private methods grows quickly makes modifying the public method incredibly painful.

If a key function of the public class is to call the private class then mock private class and test in which cases and with what arguments the private is called. Then in separate tests check that the private function handles those cases.

Improve this answer
1
  • 1
    I buy this...you might have a 100 LOC class with 2 public methods and 3 private ones that do simple, pure calculations. Those calculations are too tied to the class to be worth breaking out and the application has no need to expose them. But by testing them directly you can more easily find little bugs than by testing the class via the public methods. Commented Sep 13, 2017 at 21:15
2

Sometimes you do want to test private methods.

There's an inherent tradeoff to the granularity of a test. Tests for smaller units of functionality tend to be easier to write as they require less setup and they can supply input and receive output more directly. They also can often avoid portions of the code base that rely on side effects. Furthermore they run faster and they pinpoint errors. These advantages are why we write unit tests in the first place.

Tests for larger units of functionality test the logic of the application closer to the requirements and rely less on implementation details. A refactor is less likely to require changing a test calling a high level API than a test calling a low level API. They also test the components of the program together, so they can find problems with interoperability. These advantages are why we write integration tests.

Unit tests and integration tests fall at two points of a continuum. That continuum extends past unit tests for public functions all the way to private functions, and the aforementioned advantages and disadvantages of unit tests apply to the extreme to testing these functions. Private functions often contain nontrivial logic and often implement functionality directly under the class's or module's responsibility so they belong there. Testing them directly gives you the advantages of unit tests: simple, fast, precise location of error. However, they are the most likely to suffer from a refactor.

In practice I find myself writing a lot of these tests for algorithms involving multiple steps. A simple example is K-Means, which alternates a step of averaging vectors and partitioning them by which average from the previous step they are closest to. Each of these steps can be unit tested easily, and it is easier to test each step individually than as part of the whole algorithm.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.