DRY between Production and Test Code Constants

Question

I normally try to avoid duplication and adhere to the DRY principle. However, I'm wondering about a case like this:

public class Feature {
    final static String FEATURE_LABEL = "blah";

    public void doSomething() { ... }
    ...
}

public class FeatureTest {
    ...
    @Test
    public void doSomethingShouldMakeSomethingHappen() {
         assertEquals(Feature.FEATURE_LABEL, 
             feature.getSomethingHappens().getLabel());
    }

If the requirement is that the the label be "blah" and someone changes FEATURE_LABEL to "bleh", the test will pass even though it no longer meets the requirement. Is this a valid place to violate DRY?

Answer 1

I would stick with the reference for the moment.

The thing is, if the requirement changes that should trigger someone changing a test. Arguably the right way to change the test is to change it to the new value as a literal, see it fail, change the production static, see it pass, then change the test to also use the production static again and see it still pass.

Does that make any sense?

Answer 2

Yes, use a literal here.

Quoting myself from a question on literals:

Hardcoded literals should appear in unit tests for the test values, unless there is so much reuse of a value within a single test class that a local constant is useful.

The unit tests are a description of expected values without any abstraction or redirection. Imagine yourself reading the test - you want the information literally in front of you.

Answer 3

To test something -- anything -- an important consideration is that your test conditions stand independent of what you're testing. Otherwise, your tests don't have a single, dependable meaning; they morph into some other test every time the object under examination changes.

Not a very good thing.

The same idea applies to unit tests. In a context like above the string you're testing against should be absolutely independent of what's inside the tested class. In other words, yes, you can and should violate the DRY principle here.

Answer 4

A different way to express what the others already said: If the test can never fail, there is no point in keeping it. So this would not make sense:

assertEquals(Feature.FEATURE_LABEL, Feature.FEATURE_LABEL);

Say, for example, you have a limit for a list. There is no point in testing that limit == limit, the test should try to put more than limit elements into the list.

OTOH, if you want to make sure that the constants is being used in the proper place (i.e. it should be used as the label of some UI element), it makes sense to use the test using the string constant (instead of a new literal).

That said, for my own UI tests, I use scrapers which collect all strings visible and compare the resulting (long) string with the contents of a file. This is a very simple catch-all test for unexpected changes in the UI and works best for UIs in HTML (I download the HTML and compare it) but the same pattern can be applied to any UI.

Answer 5

I think what you have is good, and yes it is a valid use of DRY: if this value is going to end up in several tests you don't want to change several if the value changes. But you should also add an additional test, the one that validates the value of Feature.FEATURE_LABEL.

This is a good place to apply "once and only twice": if you only had a single test where the value of FEATURE_LABEL was tested then I'd just use the literal string. It is only if you have multiple tests using it where I'd start using the reference (and add a test for the value).