Is it ok to assert on the behavior of return values of a testable class?

Question

So I have a dialog for generating a random password. The user can set min, max, character categories (upper, lower, etc.). What the user basically does is configuring a StringGenerator that does the actual job, generation of a password string

StringGenerator is a Generator<String> implementation (Generator is essentially a Supplier with a different method name). It takes Generator<Character>s and randomly appends the characters they supply until a randomly calculated length is reached (I simplify a few things, but that's the general idea). For example, if I pass only one Generator<Character> that returns a hardcoded "a" every time, set both the min and max to 3, the generator is guaranteed to generate "aaa". Not very secure, but that's what it's supposed to do under those circumstances

Anyway, the dialog has a getPasswordGenerator() method. It maps all the values into a StringGenerator (the values are guaranteed to be valid, entering invalid values is impossible)

So I write a test for that. I want it to be a unit test. However, if I assert on what the generator generates (after all, I want to be sure it returns the right generator), my test ceases to be a unit test as I would in fact test the generator as well

On the other hand, if I don't do that, I won't be able to test one thing that I actually need to test (asserting on validation is important, but it's not the "real meat" of the test)

So, is it possible to unit test?

It's not SO so I figure I can skip the MRE part. I'll clarify all the points I am asked to clarify in the comments

// PasswordDialogTest
    @Test
    void whenValidPasswordDialogCreated_returnsExpectedPasswordGenerator() {
        int length = 5;
        char onlyLetter = 'a';
        // CharacterGroup is a wrapper around Collection<Character>
        CharacterGroup characterGroup = CharacterGroup.optionalGroup(Collections.singleton(onlyLetter));
        PasswordDialog dialog = PasswordDialog.builder()
                .withMinLength(length)
                .withMaxLength(length)
                .addCharacterGroup(characterGroup) // will be mapped in GUI to: ▢ a
                .build();
        Generator<String> passwordGenerator = dialog.getPasswordGenerator();
        String generatedPassword = passwordGenerator.generate();
        assertNotNull(generatedPassword);
        assertEquals(length, generatedPassword.length());
        boolean onlyIncludesExpectedLetter = generatedPassword.chars().allMatch(symbol -> ((char) symbol) == onlyLetter);
        assertTrue(onlyIncludesExpectedLetter);
    }

// PasswordDialog
    public Generator<String> getPasswordGenerator() {
        StringGenerator.Builder stringGeneratorBuilder = StringGenerator.builder()
                .withMinLength(getMinPassLength())
                .withMaxLength(getMaxPassLength());
        addCharacterGenerators(stringGeneratorBuilder);
        return stringGeneratorBuilder.build();
    }

    private void addCharacterGenerators(StringGenerator.Builder stringGeneratorBuilder) {
        getChosenCharacterGroups().stream()
                .map(CharacterGroup::getCharacters)
                .map(CharacterGenerator::fromValues)
                .forEach(stringGeneratorBuilder::supplyFrom);
    }

Answer 1

Let me for a moment accept the design of this dialog's public API and the fact it assembles a generator by itself and exposes the method getPasswordGenerator to return the result. You described the goal of this test not to test the password generator itself, but only to find out if the generator returned by getPasswordGenerator was correctly parametrized by the dialog.

Of course, you could run the generator and assert that the generated pseudo-random password fits to the parameters - but a single run will not tell us reliably if the parametrization was done correctly (even if random seed is fixed). One could make a statistical test of it, run the generator a few thousand times and hope to detect a regression that way - but I think that's tedious to implement and still not very reliable.

Hence, the most straightforward approach to solve this is IMHO: simply ask the object returned by getPasswordGenerator about its parameters. For this, one needs just getters in the class Generator<String> for everything which was passed during construction (getMinLength(), getMaxLength(), getAddedCharacterGroup()). Then the only asserts your test requires are the asserts for these getters - no need to "deduce" the correct parameters indirectly from the behaviour of the generator, which might be error prone.

In case one doesn't like those getters (maybe because one is a dogmatic follower of the "getters are bad"-school-of-thought, for whatever reason), I think it would be best to inject a StringGenerator.Builder into the dialog's constructor, which allows to replace it by something like a StringGenerator.BuilderMock during a test. Such a mock will allow to test whether all expected builder methods were called with the right values. It can also make the getPasswordGenerator obsolete, in case it was only introduced for testing and nothing else (which I cannot assess from these code snippets).

This "mock approach" would not be my preferred approach, however, since mocking a builder class will require more boilerplate code than I really like. I think for a Generator<String> class, where we know exactly which type of parameters are required for correct initialization, it is perfectly fine to ask the initialized objects later about the initial values. One may not utilize these "getters" in tests exclusive. They could be useful, for example, for a use case where a user changes repeatedly the parameters of the generator, or maybe for some other use cases.

Answer 2

TL;DR
Your tests are malformed. There are several ways to tackle this, based on context that is not fully clear. I'm going to answer with the generally most applicable approaches. Not all of them will apply to your specific case but (a) I can't conclusively judge which it is and (b) it's still useful information on how to structure your tests in general.

When reading a test for the first time, I read it bottom to top so that I can understand, in logical order:

1. What is this test trying to assert? (green)
2. Where did that behavior come from? (purple)
3. What was needed for this behavior to occur? (blue)

Applied to your test, this is what my mental picture looks like:

The key thing to notice here is that step 2 answers what the unit under test is, which in this case is the password generator, not the dialog. The only thing the dialog is doing in this test is providing access to a generator that you seem to be unable to otherwise access. This suggests tight coupling between the dialog and generator that could be avoided with proper use of dependency injection.

But before I get ahead of myself, the core issue here is that your approach muddies the lines between what should be very different scenarios, and the correct test structure depends on which scenario it is.

Pick the below option based on the title that best represents how you think about this domain/space. I'll point out specifically what part of your current test creates friction with that interpretation, and how to resolve it.
I suggest reading them all to get the full picture, but I suspect option 3 is going to be the one you end up picking.

Option 1 - Dialog and generator each have their own independently defined responsibility. Dialog represents the UI, generator implements the needed password generation logic.

Main thing to fix:
Separate your dialog/generator unit tests, rely on dependency injection to compose the two components, use a mocked generator in the dialog unit tests

This is the straightforward dependency injection approach that is most commonly used. The classes are defined as their own thing, with their own reasons for existing, and their own unit test suite surrounding them.

You should be able to independently develop the generator, including test suite, before the dialog even exists. The dialog, while it depends on the existence of a generator, can be developed and tested with a mocked generator. When composing the two (during runtime), rely on dependency injection (which is generally done via a DI container, I'm not familiar enough with Java to know which one is generally used).

That's the general advice, but your case is slightly more nuanced, as evidenced by your comment here.

You don't need a single instance of a generator that's tied to the dialog, you need the dialog to be able to generate several instances. I'm not sure if that assertion is correct or not (not enough context for me to decide), and I'm wondering whether you could reasonably make do with a single instance whose parameters are mutable, rather than needing to generate immutable instances on the fly.

I don't want to dig into a philosophical discussion on the necessity of immutability. I'll boil it down to this: if you were to take the mutable route, you could significantly simplify this by relying on straightforward dependency injection of a single generator instance. I suggest avoiding additional complexity unless you have a good enough justification for it.

Option 2 - Dialog's responsibility is to return a password. The fact that this is done via a generator is a private implementation detail.

Main thing to fix:
Dialog unit tests should not be actively aware of the existence of a password generator. Dialog public interface can't return/reference generator class in any way. Dialog should be returning a password (string), and you should write your assertions on that string value.

This is a bit of a variation on the above. Some "private implementation details" are still injected dependencies, because the "detail" is actually complex enough to warrant having its own class and test suite.
However, there are also cases where you might create a private (nested) class that performs a small duty, not enough for it to be considered a component but rather to keep it hidden as a specific implementation of this class.

If this is the case, then for all intents and purposes everyone except the dialog should not in any way be aware of the existence of this generator class. Unit tests focus on public behavior, and that does not include private implementation details.
What I would expect to see in your test is that the dialog return a password, not a generator, and then you can perform your test assertions on that returned password.

Option 3 - Dialog's responsibility is that of a "generator factory", i.e. it provides access to a generator, it doesn't use make us of it itself.

Main thing to fix:
Separate the dialog from the factory. Refer back to option 1, with the only difference that the generator factory is injected instead of the generator itself.

Based on your comment, I suspect this is the most applicable scenario for you.

Just to be clear here, the above title is phrasing it how you're currently approaching it, not how I think it could be better approached.

A factory is a valid pattern, but it is a decidedly different responsibility from what a "dialog" handles. A dialog is a UI component, a factory is an internal tool that helps you generate instances of another class based on your on-the-fly needs. Making the dialog also a factory is a violation of SRP. These need to be two separate classes.

For the purpose of what I'm trying to convey here, I don't care whether you use a factory pattern or a builder pattern (which is effectively just a method-chained fluent factory pattern anyway), although I would suggest having a reusable builder (i.e. something that can repeatedly build new instances). I'm going to refer to it as a factory from this point on for brevity.

The key friction I see in your code is that the dialog contains a tight coupling to the static string generator builder. If you instead inject that as a factory, that issue gets resolved.

You might wonder why I'm arguing against tight coupling between the dialog/generator, and am not similarly opposed to the same tight coupling between the factory/generator. The short and simple answer is that the very nature of a factory makes it tightly coupled to the product that it creates. It has to be innately aware of the product's type and constructor argument in order to make it make sense.
From a dependency perspective, the factory and the product are a package deal. You can still write unit tests for the product without including the factory, but at the same time you don't really need to write tests for the factory with a mocked product, since the factory tends to be in charge of instantiating the product (meaning you can't mock it). And that's fine, on the basis of what a factory is.

From this point on, a lot of the advice from option 1 starts applying here again, except that it applies to the generator factory instead of the generator itself.

Answer 3

Whenever writing a test for something that uses random, control your random!

Among other things, unit tests should be deterministic. To do that they need deterministic things to test. For the unit to be deterministic it needs to be given known predictable “random” for the test.

There’s no problem if you decide the “unit” for this test is more than one class. What the test needs is a chunk of code that when you send the same stuff in the same stuff always comes out. We call that deterministic.

Mocks are best used, not when isolating classes from other classes just because they are other classes, but when cutting out code or systems (like real random) that would make it non-deterministic.

Hopefully with that perspective you’ll feel less pressure to know what exactly was created and more comfortable focusing on how it behaves.

Answer 4

However, if I assert on what the generator generates (after all, I want to be sure it returns the right generator), my test ceases to be a unit test as I would in fact test the generator as well

Let's say that (one of) your test for Generator<String> looks like the assertions at the bottom of your Dialog test:

String generatedPassword = passwordGenerator.generate();
assertNotNull(generatedPassword);
assertEquals(length, generatedPassword.length());
boolean onlyIncludesExpectedLetter = generatedPassword.chars().allMatch(symbol -> ((char) symbol) == onlyLetter);
assertTrue(onlyIncludesExpectedLetter);

Here you are also testing String, not just Generator<String>. If length or chars, or allMatch stop working, this test will fail as well. But we still call this a unit test. It is a misconception that a unit test must only call methods on one of your classes.

If the only way to observe the state of a Generator<String> is to call generate(), then calling it must be how you test generator producers. Alternatively, as the other answers discuss, there are other things you can do to assert the dialog sets correct state in the generator.

Is it ok to assert on the behavior of return values of a testable class?

Of course! That's the only thing you can do.

Answer 5

my test ceases to be a unit test as I would in fact test the generator as well

A "unit" in unit testing does not necessarily mean a class. It can be any grouping of functionality, ideally with high internal cohesion and an easily testable interface.

Another important point is that you should test the interface of your unit. You should ideally only need to change your test when the interface changes. But separating your code into well designed modules/units is notoriously difficult.

There can also be some confusion regarding the terms "unit" vs "integration" vs "system" tests. I do not see the distinction as particularly useful, and prefer the term "automated tests". If you feel the need to categorize tests I would suggest categorizing on domain context instead, or in some cases, runtime.

If PasswordDialog is the most sensible unit to test, go ahead and test on that level, regardless of how it is implemented internally. But I would have liked a much more comprehensive tests, since your posted example could be passed by a simple return "aaaaa";.

Also note that randomness is difficult to test, and it can be easy to introduce bias. So it might be a good idea to design your class so that you can use a well curated and reproducible entropy source for testing, and write some automated statistical tests. Since these kinds of tests might take some time to run, you might want to put them in some special category to avoid slowing down your regular unit tests.

Answer 6

Component's interface includes interfaces of all of its return values.

It is fine to call return value's method in a test.

In particular, as long as Generator<String> is returned by getPasswordGenerator(), Generator<String>.generate() is a part of dialog's interface. It should be tested in dialog's unit test.

_{Sure, I could make dialog return GeneratorSettings instead, and never cover UI component by unit tests in the first place, but that depends on the degree of modularization you want to achieve, and the highest degree is not always best.}