3

I have a function which returns the availability of a module, where the module is only available if multiple conditions are all met. The code looks like this:

bool isShipAvailable() {
  bool isAvailable;

  isAvailable = this->areSystemsCalibrated;
  isAvailable = isAvailable && this->areEnginesFunctional;
  isAvailable = isAvailable && this->isWarpDriveOnline;
  isAvailable = isAvailable && this->isFluxCapacitorOnline;
  isAvailable = isAvailable && this->areStabilizersOnline;

  return isAvailable;
}

I'm trying to test this code without creating something large or difficult to maintain. I know I could write tests for all permutations, but would it be acceptable to test the case where all conditions are true and then test each scenario where only one condition is false? Would that be making too many assumptions about the implementation in the tests?

Edit: I'm not asking how to refactor the boolean logic to be concise, but rather what the shortest way to test every permutation of that logic would be. The details of the implementation itself are not terribly important.

  • 9
    Why do you keep re-assigning isAvailable when you could simply string together all those && clauses? (put each on its own line, starting with &&) – Robert Harvey May 31 '18 at 18:20
  • This is just the style my team agreed on, I'm not terribly keen on it – codehearts May 31 '18 at 18:26
  • 1
    That said, if you really want tests for this, I think one test with all flags true and one test with one of the flags false really ought to suffice. Otherwise, there's 5 flags here; a fully comprehensive test would require 32 test permutations. – Robert Harvey May 31 '18 at 18:45
  • 1
    Possible duplicate of How to turn truth table into smallest possible if / else block – gnat May 31 '18 at 19:13
  • 1
    @robert it can be useful with many debuggers if you want to know why the “and” failed. – gnasher729 Jun 1 '18 at 7:26
9

Unit testing is about behaviour of the code under test from the point of view of its clients. Therefore, as your test class is one of its clients, you should define what are your expectations.

There's a few expectations I see from your implementation :

  1. Given that the systems are calibrated, the engines are functional, the warp drive is online, the flux capacitor is online and the stabilizers are online, then the ship is available.1
  2. Given that the systems are not calibrated, then the ship is unavailable.
  3. Given that the engines are non functional, then the ship is unavailable.
  4. And so on.

Therefore, I would expect 6 tests, as there are 6 distinct behaviours I care about as a client.

Would that be making too many assumptions about the implementation in the tests?

Quite funny, because I actually believe the reverse is true: you are making too many assumptions about your implementation when you test every combination. Remember that you always test from the point of view of your clients. If you test every single combination, then you're saying that your clients care about all those combinations. My understanding of your code is that it doesn't matter which combination of systems are offline or non functional. As long as one subsystem is offline, then the ship is unavailable, period. Therefore, I would test it as such.

1. Phew, that might require quite of a setup to test this assertion. If that's your feeling as well, then maybe your tests are speaking to you: your code under test is doing quite a lot.

  • I feel like your stated expectations call for different code that what is in the OP, from a semantic perspective. For example, if(!areSystemsCalibrated) return false corresponds to expectation 2. – Robert Harvey May 31 '18 at 18:51
  • 2
    It could, but even if you write the function that way, it results in the exact same behaviour and your tests will still pass. This is, in the end, the goal of unit tests: it doesn't care about the implementation, only about expectations and behaviours. – Vincent Savard May 31 '18 at 19:00
3

The standard library provides a function specifically and explicitly for tasks like this. I'd simply use it, and see little reason to test that it does is job.

#include <algorithm>

class foo {
    bool areSystemsCalibrated,
        areEnginesFunctional,
        isWarpDriveOnline,
        isFluxCapcacitorOnline,
        areStabilizersOnline;

    bool foo::*conditions[5] = {
        &foo::areSystemsCalibrated,
        &foo::areEnginesFunctional,
        &foo::isWarpDriveOnline,
        &foo::isFluxCapcacitorOnline,
        &foo::areStabilizersOnline
    };

public:
    bool isShipAvailable() const {
        return std::all_of(std::begin(conditions), std::end(conditions),
            [&](bool foo::*b) { return this->*b; });
    }
};

If, at that point, you really want to test it anyway, you have a collection of conditions. That makes it relatively simple to test whatever combination of conditions you see fit--up to and including an exhaustive test of every possible combination, if you really want.

  • This doesn't help with writing the tests, and I'd be wary of leaving it untested simply because it's using the standard library. If the warp drive check is accidentally removed, we'd still want to catch it in testing. – codehearts Jun 1 '18 at 15:27
  • @codehearts: Yes, it does help with writing tests. Having all the conditions in a collection makes it trivial to do quite a few tests that are non-trivial otherwise. For example, you can do all the usual loop-things to generate various combinations of conditions, which is painful (at best) otherwise. – Jerry Coffin Jun 1 '18 at 17:01
1

With a large, but finite number of test cases Data-Driven Testing is the answer.

... testing done using a table of conditions directly as test inputs and verifiable outputs as well as the process where test environment settings and control are not hard-coded. In the simplest form the tester supplies the inputs from a row in the table and expects the outputs which occur in the same row. The table typically contains values which correspond to boundary or partition input spaces.

You need 1 explicit test where all conditions are true saying "it works".

By using a data driven test, you can test all of the "false" values by parameterizing each boolean field as an input to the test.

How you do this depends on the tech stack and unit testing framework, but it could be reduced down to a test method and a for-loop that iterates over the expected inputs (example in C#, MS Test framework):

[TestClass]
public class ShipTests
{
    [TestMethod]
    public void ItWorks()
    {
        var ship = new SpaceShip(true, true, true, true, true);

        Assert.IsTrue(ship.isShipAvailable());
    }

    [TestMethod]
    public void ItDoesntWork()
    {
        var inputs = new bool[][]
        {
            { false, true, true, true, true },
            { false, false, true, true, true },
            // ...
            { false, false, false, false, false },
        };

        foreach (var row in inputs)
        {
            var ship = new SpaceShip(row[0], row[1], row[2], row[3], row[4]);

            Assert.IsFalse(ship.isShipAvailable());
        }
    }
}

Robert Harvey commented on the question:

That said, if you really want tests for this, I think one test with all flags true and one test with one of the flags false really ought to suffice.

It doesn't suffice, because any one flag being false causes the ship to be unavailable. You don't want to test that the warp drive is not available, and say all tests pass if a defect causes the flux capacitor to be unexpectedly offline.

The flux capacitor is important, you know. VERY important. It must be online no matter what -- even more than Facebook!

Otherwise, there's 5 flags here; a fully comprehensive test would require 32 test permutations.

Yes, there are a large number of test cases, but the work required to maintain those tests is minimal if using a data driven test. And if this is all in-memory data manipulation, test execution time is negligible. I think I spent more time writing this answer than all the developers on the team will spend running these tests over the course of the project's lifetime.

-1

would it be acceptable to test the case where all conditions are true and then test each scenario where only one condition is false?

Yes.

But you should test it according to your use case i.e. as long as your conditions vary independently.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.