Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

DRY between Production and Test Code Constants

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?

like image 209
Paul Croarkin Avatar asked Jan 14 '09 13:01

Paul Croarkin


4 Answers

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.

like image 195
Andy Dent Avatar answered Sep 26 '22 06:09

Andy Dent


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.

like image 40
Frederick The Fool Avatar answered Sep 22 '22 06:09

Frederick The Fool


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.

like image 42
Aaron Digulla Avatar answered Sep 25 '22 06:09

Aaron Digulla


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?

like image 42
Jon Skeet Avatar answered Sep 24 '22 06:09

Jon Skeet