1

There's a (mis)conception that you don't have to test configuration

But what if your configuration is in the form of runnable code? What if it's not just static values?

I wrote an abstraction for notification of our desktop app's users. Basically, it's supposed to abstract away all modal window invocations scattered throughout the code. I wanted to write tests for that. The problem is our CI server may not have a graphical environment (my understanding is it depends on the build agent used). So I discovered a "trick": I gutted the notifier class so that it delegates actual notification to an externally injected Consumer. The consumer, essentially, takes a message string, puts it in a dialog window, and shows it. But that consumer is created and supplied to my notifier by a factory now! So I'm all good, I don't have to test it, it's configuration now. At the same time, I could write (and push) tests for my notifier which still contained (a bit of) logic

But what if I tricked myself? After all, the "configuration" code is still code that may or may not work correctly. Should I have just accepted the fact that I couldn't test my notifier class (or at least, couldn't push the tests)? Would it be more honest and professional than "copping out" of proper testing on a technicality?

At any rate, where does the boundary between code (should be tested) and configuration (shouldn't be tested) lie? Or maybe any runnable (as opposed to parsable) configuration should still be tested, like any other code, shouldn't it?

/* I removed all the javadocs since they were not in English, 
 and I didn't want to spend time translating them */
public interface Notifier {
void notifyUnlessSuccess(Report report);

void notify(Report report);

void notify(Report report, Report.Status statusThreshold);

}

public abstract class BaseNotifier implements Notifier {
    @Override
    public void notifyUnlessSuccess(Report report) {
        Report.Status leastSevereNonSuccessStatus = Report.Status.WARNING;
        notify(report, leastSevereNonSuccessStatus);
    }

    @Override
    public void notify(Report report) {
        Report.Status leastSevereStatus = Report.Status.SUCCESS;
        notify(report, leastSevereStatus);
    }

    @Override
    public void notify(Report report, Report.Status statusThreshold) {
        if (report == null) return;
        boolean isStatusThresholdReached = report.getStatus().getSeverity() >= statusThreshold.getSeverity();
        if (isStatusThresholdReached) doNotify(report);
    }

    protected abstract void doNotify(Report report);
}
public class SimpleNotifier extends BaseNotifier {
    private final Consumer<Report> defaultConsumer;

    public SimpleNotifier(Consumer<Report> defaultConsumer) {
        this.defaultConsumer = defaultConsumer;
    }

    @Override
    protected void doNotify(Report report) {
        Optional<Consumer<Report>> customReportConsumer = report.getCustomReportConsumer();
        boolean hasCustomReportConsumer = customReportConsumer.isPresent();
        if (hasCustomReportConsumer) customReportConsumer.get().accept(report);
        else defaultConsumer.accept(report);
    }
}
// "configuration" :)
public class Notifiers {
    private static final Map<Report.Status, String> defaultSummaryMap = new HashMap<>();
    private static final Map<Report.Status, Function<String, ? extends BaseMsg>> statusToDialogFunctionMap = new HashMap<>();

    static {
        defaultSummaryMap.put(Report.Status.SUCCESS, Common.getResource("MSG_ERRS_NO"));
        defaultSummaryMap.put(Report.Status.WARNING, Common.getResource("MSG_ERR_WRN"));
        defaultSummaryMap.put(Report.Status.FAILURE, Common.getResource("MSG_ERR_CRIT"));

        statusToDialogFunctionMap.put(Report.Status.SUCCESS, InfoMsg::new);
        statusToDialogFunctionMap.put(Report.Status.WARNING, WarningMsg::new);
        statusToDialogFunctionMap.put(Report.Status.FAILURE, ErrorMsg::new);
    }

    public static Notifier notifier() {
        return new SimpleNotifier(defaultConsumer());
    }

    private static Consumer<Report> defaultConsumer() {
        return report -> {
            Report.Status reportStatus = report.getStatus();
            String summary = Optional.ofNullable(report.getSummary())
                    .filter(StringUtils::isNotBlank)
                    .orElse(defaultSummary(reportStatus));
            Function<String, ? extends BaseMsg> dialogFunction = statusToDialogFunctionMap.get(reportStatus);
            BaseMsg dialog = dialogFunction.apply(summary);
            dialog.show();
        };
    }

    private static String defaultSummary(Report.Status reportStatus) {
        return defaultSummaryMap.get(reportStatus);
    }
}
public interface Report {

    String getSummary();

    default boolean isSuccess() {
        return getStatus() == Status.SUCCESS;
    }

    Status getStatus();

    default Optional<Consumer<Report>> getCustomReportConsumer() {
        return Optional.empty();
    }

    enum Status {
        SUCCESS(0), WARNING(10), FAILURE(20);

        private final int severity;

        Status(int severity) {
            this.severity = severity;
        }

        public int getSeverity() {
            return severity;
        }
    }
}
// the advantage is I can now write and push tests for my Notifier implementation
class SimpleNotifierTest {

    @Test
    @SuppressWarnings("unchecked")
    void notify_ifReportNotSevereEnough_doesNotInvokeReportConsumer() {
        Consumer<Report> defaultReportConsumerMock = mock();
        willDoNothing().given(defaultReportConsumerMock).accept(any());
        SimpleNotifier notifier = new SimpleNotifier(defaultReportConsumerMock);
        Report successReport = createReport(Report.Status.SUCCESS);

        notifier.notify(successReport, Report.Status.FAILURE);

        then(defaultReportConsumerMock).shouldHaveNoInteractions();
    }

// more test methods...

5 Answers5

9

There's a (mis)conception that you don't have to test configuration

Well, you wrote it - it is probably a misconception.

Any configuration has surely to be tested - and it will be tested, at least at the point in time when a program is started in production environment and does not work properly because of an error in the configuration. It does not matter whether some configuration is just a bunch of boolean switches or some executable code, when its content makes the difference between a correctly working program and a program which does not work / work faulty.

The real questions you have to ask is

  1. whose job is it to test a configuration?

  2. should a configuration be tested by some automated test?

The responsibility of testing a configuration lies usually by its author. Configuration files are often used as a mechanism to allow non-developers like users or admins to control certain properties of a program, and if a user, power user or admin writes a configuration, they have to test that it produces the desired behaviour (your responsibility, as a developer, is to make sure the processing of the configuration works correctly). It is a really bad idea for an admin to change something in a configuration file, put that untested into production at Friday, 6pm, on the day before a 3 week vacation, and let users find out at monday that the configuration was buggy.

Of course, when you are speaking about certain configurations you, as a developer provide, maybe as part of the source code, then the responsibility for the tests is yours.

The second question about automated tests is just one about cost-effectiveness (as always, not very specific for configurations). A developer may be able to write automated tests for configurations more easily than a user, power user or admin. But even for a developer testing a configuration which makes a program pop-up certain modal message boxes (and hence blocking the execution) may not be cost-effective, because such tests tend to require certain testing tools and still may be brittle. For a configuration which an admin has to provide (when deploying to production environment), the most cost-effective way may be to built mechanisms into the software to validate the configuration, write information into logs which show how the configuration was interpreted and show specific error messages for any kind of misconfiguration. That's probably way more important than any automated tests.

So whether you should write automated tests for certain configurations, or test them manually once, or if it is better let someone else do the manual testing (supported by a suitable validation mechanisms in the application), can only be answered by a cost-/benefit analysis.

Doc Brown
  • 218,378
5

Should you test configuration?

Absolutely.

So I should create an automated test for it?

I didn’t say that. Reading code is a form of testing. Automated testing tends to be focused on code that handles business rules. The interesting, hard to read, iffy code.

Keep your configuration code free of that stuff, so it’s easy to read, and the need for automated tests is greatly reduced.

But what if your configuration is in the form of runnable code? What if it's not just static values?

Again, what we care about is how easy it is to read. The best test for that is make someone else read it. If they say it’s boring and obvious you’re good. If they say it’s interesting you’re in trouble. Move anything interesting to somewhere that makes automated testing easy.*

* See humble object

The power of an automated test comes from the fact that the test can be easier to read than the code it's testing. Use easy to read code to show how hard to read code behaves and you've made my life easier.

Use easy to read code to show how easy to read code behaves and you're wasting my time.

Conversely, while a test may provide the ability to repeatedly check for regressions and flag behavioral changes, is that doing you any good if you can't read the test?

What makes a test good isn't automation. Automation only adds convenience. Please don't confuse the two. Good testing will always be more than automation.

The consumer, essentially, takes a message string, puts it in a dialog window, and shows it.

Sounds boring and obvious. If the code reads that way you’re likely good to stick with code reviews for this. Oh sure, it’s possible to create automated tests for this but what are they proving to you? That you didn’t break it? Be careful here because it’s easy to create tests that end up just mimicking the code they test. Tests should make change easier not harder. Tests that have to die the moment you change anything aren’t helping you any more than source control would.

So I’m sorry that I can’t support a simple rule like “configuration code doesn’t need testing”. What I can support is a rule like “if you’re careful to keep your configuration code simple and obvious you can focus your automated tests on other stuff”

candied_orange
  • 119,268
1

There are def some misconceptions/confusion/odd use of terms here.

... gutted the notifier class so that it delegates actual notification to an externally injected Consumer

You have invented unit tests! You can now test your abstraction and various "this should open a dialogue" classes without running the whole app. +100xp

So I'm all good, I don't have to test it, it's configuration now.

Wait what?

DI framework was not yet greenlighted by the lead, so we have a bunch of factory classes that create and return objects through their public static methods. To me, those classes (I wrote them, btw) are somewhat akin to Spring's @Configuration classes, so I consider them configuration classes

The factory classes are not configuration

You should test your application works. You should test your application works in it various configurations.

You have a problem in that it's hard to write UI tests, you have to actually run the app and then use the windows API to mimic mouse clicks and get handles to UI elements to test they exists and what they say etc. You can get some frameworks to do this.

But in the meantime, your approach of allowing injection of Mock classes to enable unit tests is good.

It makes sense not to test "that the configuration is correct". The configuration of an app depends on where and how its deployed. There is no "correct" for all cases. But all the classes in your app should have tests.

Ewan
  • 83,178
1

When testing, one would usually chose some specific data values to cover the most of your code, and at least what could go wrong. This is best exemplified with the popular joke:

A software tester walks into a bar and orders a beer, then 3 beers, then 0 beer, then -1 beer, then orders nothing, then 65535 beers, then cancels the last order.

Configuration is part of the data used by your code. And its values may cause some code to be used rather than another, and might trigger some unexpected combination. You should therefore test your code using the relevant configuration samples to ensure proper coverage and make sure the pieces work well together (integration).

Or if you already know the configuration to be used and are not sure if it was properly tested, run the test suite with that exact configuration.

Christophe
  • 81,699
0

Contrary to some other answers I will say: you do not test configuration. In fact configuration is not testable at all, not in a sane way anyway.

Just like you don't test integers, you don't test boolean values, you don't test strings. You don't test JSONs, you don't test XMLs. You don't test data. And that's what configuration really is: a piece of data.

You can test parsers for that data. You can test validators for that data. You can test certain system behavior when initialized with some data. But you don't and can't test data. Data just is. It doesn't have any behaviour to test.

But what if your configuration is in the form of runnable code? What if it's not just static values?

Then it is no longer just a configuration. You have code with data embedded. And indeed this is testable.

Typically configuration is a piece of data that lives outside of application. Application then loads it and uses it at runtime. The point of configuration is to affect runtime without the need of recompilation/rebuild. From that point of view it is prefarable for configuration to be plain data.

There are certain frameworks that use code as configuration (e.g. Python Django's settings.py files) but this works only because certain languages (e.g. Python) abuse the fact that they are interpreted and don't really have (or need) compilation/build process. But this approach is an antipattern IMO.

But that consumer is created and supplied to my notifier by a factory now! So I'm all good, I don't have to test it, it's configuration now.

That's not configuration. Maybe it is in some broad sense, but not something we would typically consider a configuration.

Anyway that's a composition of services. Which indeed can and should be tested, at least through integration tests.

freakish
  • 2,803