change visibility of a private java constant to test it

Question

Having a private String constant in a java class that is used in some methods, when I want to make the test for this class, I will have to hardcode the string constant in the test because its private in the class. But if in the future the string constant is modified in the class, I will have to modify in the test class too. I dont want to modify the visibility of this constant just for the tests to avoid this possible modification in the future. Is this thinking correct according to the TDD methodology?.

Thanks.

Answer 1

Since this question is tagged tdd, I'm going to pretend that the OP didn't include

when I want to make the test for this class

After all, TDD means test-driven development; i.e. write the test before the implementation.

Additionally, since this issue is a problem in several programming languages, and not just Java, I'm going to respond with some C# code. I'm sure you'll be able to follow it.

The short answer, however, is that it depends on whether or not that constant is part of the specification of the System Under Test (SUT).

Imagine, as an illustrative example, that you have to TDD a little Hello World API.

You might write a few test cases like this:

[Theory]
[InlineData("Mary")]
[InlineData("Sue")]
public void ExactEnglishSpec(string name)
{
    var sut = new Greeter();

    var actual = sut.Greet(name);

    Assert.StartsWith("Hello, ", actual);
    Assert.EndsWith(name + ".", actual);
}

This might drive an implementation like this:

public sealed class Greeter
{
    private const string englishTemplate = "Hello, {0}.";

    public string Greet(string name)
    {
        return string.Format(englishTemplate, name);
    }
}

As in the OP, this implementation contains a private const string. What happens if that englishTemplate changes?

Obviously, the tests fail.

As they should, since you wrote them that way.

Tests specify the behaviour of the software, and if they assert that a string should be exactly a particular value, then that is part of the specification, and it's a breaking change if it changes.

This may or may not be what you want.

It depends on what you're doing. If you're implementing a formal specification that demands that exact template string, then that should be part of the contract, and it should be a breaking change if someone changes the template. This would also be true if it's part of an API where you've agreed with callers that the string will always be based on that template.

On the other hand, this may not be what you want. If you're implementing an (object-oriented) API that encapsulates behaviour, and you expect that the template might change in the future, write a more robust test:

[Theory]
[InlineData("Rose")]
[InlineData("Mary")]
public void RobustEnglishBehaviour(string name)
{
    var sut = new Greeter();

    var actual = sut.Greet(name);

    Assert.Contains(name, actual);
    Assert.NotEqual(name, actual);
}

The first assertion here verifies that the name is to be found somewhere in actual. The second assertion prevents the implementation from simply echoing back the name.

Now, if you change the template, this robust test still passes, while the first example would break.

By now, I'm sure you're asking: But how can I verify that the return value isn't just some nonsense?

In a sense, you can't, because you've made the decision that the actual value is no longer part of the contract. Imagine that, instead of an English greeting, you're asked to internationalise the API. Will you, as the developer, be able to verify that greetings in Romanian, Chinese, Thai, Korean, etc. are correct?

Probably not. Those strings are going to be opaque to you, hopefully instead supplied by professional translators.

How do you test against something like that?

Naively, one might attempt something like this:

[Theory]
[InlineData("Hans")]
[InlineData("Christian")]
public void ExactDanishSpec(string name)
{
    var sut = new Greeter();
    sut.Culture = "da-DK";

    var actual = sut.Greet(name);

    Assert.StartsWith("Hej ", actual);
    Assert.EndsWith(name + ".", actual);
}

This might drive an implementation like this:

public sealed class Greeter
{
    private const string englishTemplate = "Hello, {0}.";
    private const string danishTemplate = "Hej {0}.";

    public string? Culture { get; set; }

    public string Greet(string name)
    {
        if (Culture == "da-DK")
            return string.Format(danishTemplate, name);
        else
            return string.Format(englishTemplate, name);
    }
}

Again, this is an exact specification. I'll remind you that this example is just a placeholder for a 'real' problem. If the exact string is part of the specification, then writing the test as shown above seems appropriate, but that also means that if you change the strings, the tests will break. And that's by design.

If you instead want to write a more robust test, you might do something like this:

[Theory]
[InlineData("Charlotte")]
[InlineData("Martin")]
public void EnglishIsDifferentFromDanish(string name)
{
    var sut = new Greeter();
    var unexpected = sut.Greet(name);
    sut.Culture = "da-DK";

    var actual = sut.Greet(name);

    Assert.Contains(name, actual);
    Assert.NotEqual(name, actual);
    Assert.NotEqual(unexpected, actual);
}

These assertions again verify that the name is included in the response, and not just echoed back. Furthermore, the test asserts that the Danish greeting isn't the same as the English greeting.

This test is more robust than the exact specification, but isn't absolutely guaranteed never to fail. If, in the future, translators decide that the English greeting and the Danish greeting should use effectively equal templates, the test would fail.

Coming back to the OP. How about exposing the templates as public constants?

Sure, you could do that if the exact value is not part of the specification:

public const string EnglishTemplate = "Hello, {0}.";
public const string DanishTemplate = "Hej {0}.";

This would enable you to write a test like this:

[Theory]
[InlineData("Thelma")]
[InlineData("Louise")]
public void UseEnglishTemplate(string name)
{
    var sut = new Greeter();
    var actual = sut.Greet(name);
    Assert.Equal(string.Format(Greeter.EnglishTemplate, name), actual);
}

Does that, however, provide any value? It essentially just repeats the implementation code.

Don't repeat yourself.

Still, just to drive home the point: Such a test would be robust to changes of the template strings. The test would pass even if you edit the string. Is that appropriate?

That depends on whether or not the exact string value is part of the public contract for the SUT.

This line of reasoning applies not only to strings, but also to constant numbers.

If the constant is part of the contract, write your tests in such a way that they fail if the constant changes. If the constant is an implementation detail, write robust tests that pass when the value changes.

Answer 2

Changing visibility to package-private for tests should be fine. Still, it can be used in the same package, if some other developer is not aware he shouldn't do it. This can break stuff, if you change the value later on.

Better approach would be to use some abstraction to get this value, instead of using it directly. For example:

public class MyClass {

  private static final String SOME_VALUE = "value";

  private final Supplier<String> valueSupplier;

  public MyClass(Supplier<String> valueSupplier) {
    //new constructor
    this.valueSupplier = valueSupplier;
  }

  public MyClass() {
    //previous constructor
    //provides the default supplier with the constant
    this(() -> SOME_VALUE);
  }

  public String doStuff(String stringValue) {
    //method to test
    return this.valueSupplier.get() + "_" + stringValue;
  }
}

Now you can mock valueSupplier in your unit tests. This allows you to change SOME_VALUE however you like, without breaking existing tests, while still keeping it private.