How to arrange expected result model for test in a cleaner,readable way when the model contains many properties

Question

In one of my projects I saw wrapping the expected results as a static variable in non static class. The reason for doing so is to make the code more readable, so that the massive expected result model wont clutter the code. From my point of view this is a code smell because

1. The expected result is not a good candidate for a class since the data is static and there can be no valid objects made out of it( It cannot be seen as a valid blue print).
2. Usage of static variables will make it stay in memory until all tests are run.
3. Expected results are meant to be part of test scenario itself and not to be kept under a different class.

I prefer to populate the expected result for a particular case in its corresponding case itself( or separate it out as function in same class), but somehow the readability argument and YAGNI argument over-weighs my argument since the tests are running fine. This is an end to end test and hence only few test scenarios are here (rest is covered in unit test and integration test) and uses CI to run tests.

I need help to confirm whether my arguments are valid. If so how to make it more readable without cluttering the code.

Note: we use BDD testing(specflow) for E2E.

//all the expected test data result for foo class goes here
public class ExpectedFooResults
{
    public static readonly Foo fooCase1Result = new Foo
    {
        //30+ properties are assigned expected values
    }
    public static readonly Foo fooCase2Result = new Foo
    {
        //30+ properties are assigned expected values
    }
}

And inside the test

public void TestFooWithCase1()
{
    //arrange
    var result = Foo.Sut();
    result..Should().BeEquivalentTo(ExpectedFooResults.fooCase1Result);
}

Answer 1

What I've often done for large objects like this (which should be avoided if possible, but can't always be avoided) is to dump the object into a JSON file (you could of course use any handy/useful serialization format, but text-based is preferable as you can read the diffs in source control).

The test method wouldn't look much different other than a call out to a utility method to load the file. Something like:

public void TestFooWithCase1()
{
    //arrange
    var result = Foo.Sut();
    result..Should().BeEquivalentTo(loadJson("fooCase1Result.json"));
}

This keeps the actual test code pretty lightweight while still allowing for tests across large object graphs where you can still see what changed during code reviews.

Answer 2

Your points

One thing you may have glossed over, which can change your outlook on things, is that const values are inherently static.

While they each serve a different purpose, there is some similarity in why you can justify the use of static to list some fixed values, effectively using it in the same way you'd use a const (but with less restriction on its constness).

1. The expected result is not a good candidate for a class since the data is static and there can be no valid objects made out of it( It cannot be seen as a valid blue print).

Your expectation (object instantiation) clashes with using statics (predefined values for comparison), but we can argue over which of these should change.

If you consider your static properties as readonly properties (i.e. "const-like"), which is the basis for using statics to store expected values, then object initialization is not a relevant concern.

If you expect to dynamically generate and store values, then statics are not the way to go.

2. Usage of static variables will make it stay in memory until all tests are run.

That is correct, but not necessarily a relevant concern for every test suite.

How appropriate an implementation is, is very much defined by how necessary its existence is. For a tiny test suite, I don't set up an elaborate fixture, but for an enterprise-grade test suite I do.

Similarly, your memory concern is relevant for a large test suite (or one that is expected to grow large at some point), but can be ignored in sufficiently small test suites.

3. Expected results are meant to be part of test scenario itself and not to be kept under a different class.

It depends on the complexity of your domain and the tests surrounding it. If your domain uses a lot of information (e.g. metadata, audit fields, outcomes based on input, ...), then listing every expected/actual value in every test scenario can massively lower readability.
Especially when those same values are reusable across many tests, there's a good argument to be made for storing these concrete values away from the test logic itself.

That doesn't mean it has to be static properties, any storage method will do. It could be another file on disk, or an in-memory database, or ... The method of storing the data is irrelevant - what matters is that there are cases where storing the data external to the main body of the test is very much justified.

---

Example

As a real-world example, I work on a payroll-like software project where you need a boatload of data (people, authorization, schedules, grants, ...) if you want to be able to test a significant software feature. Similarly, a simple action can have a rather complex outcome which you'd want to test.
Both cases mean that any test drowns in literal values and it's hard to spot where the actual logic is.

To counteract this, we've created a reusable test fixture with a fluent syntax to significantly reduce clutter in the test bodies. Think something along the lines of:

fixture
   .Given.PersonExists("Anna");
   .Given.WorksUnderManager("Bob");
   .Given.HasRights(...);

fixture
    .When.RequestsPermission("Anna", "Bob", "to go on lunch break");

fixture
    .Then.EmailReceived("Bob", "Anna", "RE: Lunch break");

This example has been heavily redacted for proprietary reasons.

When you see "Anna" and "Bob", those are actually references to JSON files stored on disk. We've created certain characters and predefined their data in a test file, and instead of constantly reinventing (and setting up) a new character for every test, we simply use a repository of "test people".

The goal of the test is clear, and the complicated setup process is hidden from view. Each of those methods is actually an arrange (Given) which mocks some data, an act (When) which performs real actions, or an assert (Then) which performs a complicated assertion.

The point I'm trying to get across here is that how applicable a given approach is completely depends on how complex your domain is. What is right for one use case, may be over/underengineered for another.

Answer 3

The expected result is not a good candidate for a class since the data is static and there can be no valid objects made out of it( It cannot be seen as a valid blue print).

This is actually a limitation of C#. Everything must exist in a class, struct or enum. If all members are static, then make the class static. The class is needed simply because you cannot declare variables scoped to a namespace. It is a necessary wrapper, because that's how C# works.

As others mentioned, if the intent is to set the values on these properties and never change them, make them readonly as well.

public static class ExpectedFooResults
{
    public static readonly Foo fooCase1Result = ...

    public static readonly Foo fooCase2Result = ...
}

Putting them in a static class isn't necessarily a code smell. It is a test fixture, which is a valid design technique for tests. The intent of a fixture is to encapsulate a large amount of information that doesn't fit well in code. Examples would be a JSON file used as a response to a web service, or an image in case you are testing image manipulation code.

An object with 30 properties is not too bad. If the result is needed in multiple test cases then a static class is just fine.

Usage of static variables will make it stay in memory until all tests are run.

The amount of memory required is negligible. Worrying about this would be premature optimization. Furthermore, even if you hard code it as a field in a test class, this data still must be compiled in to the test assembly, so it will still take up hard drive space and get copied in to memory when the test runner loads the test assembly in order to run tests.

Expected results are meant to be part of test scenario itself and not to be kept under a different class.

This is up for debate. I'm inclined to agree with you. It's nice to keep the data used in an assertion along side the test case, but don't ignore the importance of a good name. You can name the static properties according to the use case. Instead of fooCase1Result name it what it represents:

ExpectedFooResults.ValidFooResponseBecauseReason
ExpectedFooResults.InvalidFooResponseBecauseReason

You assertion will read much better:

result.Should().BeEquivalentTo(ExpectedFooResults.ValidFooAfterProcessing);

Then again, you can create a readonly static field on the test class itself with the same name.

Don't get too hung up on this. There are much bigger architectural sins in test code. If these static properties make tests brittle and hard to refactor, now it is worth addressing this issue. Until then, make the ExpectedFooResults class a static class, make the properties readonly, and keep writing tests until this class becomes an architectural issue.