3

Sorry for the long-winded question. I hope the question and its potential answers can serve as a useful example to developers who are wondering where to set the bar regarding testing fairly simple combinations of already tested functions.


Consider the following simple model for an order (definition of auxilliary types are omitted for brevity):

type Order =
    {OrderDate: DateTime
     CustomerId: CustomerId;
     SalesPersonId: SalesPersonId;
     OrderLines: OrderLine list}

Let's say we want to aggregate data from all orders in a certain time interval. Furthermore, let's say that we may want to only include data from orders that match a certain customer ID and/or a certain sales person ID. This could be implemented as a series of simple functions with signature Order -> Order option (after partial application of whatever the filter parameter is), then combined using Option.bind and finally applied to a list of orders using List.choose. For example:

type OrderFilter =
    {StartDate: DateTime;
     EndDate: DateTime;
     CustomerId: CustomerId option;
     SalesPersonId: SalesPersonId option}

// DateTime -> Order -> Order option
let filterStartDate startDate order = 
    if order.OrderDate >= startDate then Some order else None

// DateTime -> Order -> Order option
let filterEndDate endDate order = 
    if order.OrderDate <= endDate then Some order else None

// CustomerId -> Order -> Order option
let filterCustomerId custId order = 
    match custId with
    | None | Some id when order.CustomerId = id -> Some order
    | _ -> None

// SalesPersonId -> Order -> Order option
let filterSalesPersonId spId order = 
    match spId with
    | None | Some id when order.SalesPersonId = id -> Some order
    | _ -> None

// OrderFilter -> Order -> Order option
let applyOrderFilter filter order =
    let (>=>) f1 f2 = f1 >> (Option.bind f2)
    let combinedFilter =
        filterStartDate filter.StartDate
        >=> filterEndDate filter.EndDate
        >=> filterCustomerId filter.CustomerId
        >=> filterSalesPersonId filter.SalesPersonId
    order |> combinedFilter

Filtering is then performed like this from elsewhere in the codebase:

let filter = 
    {StartDate = ...;
     ...}

let orders = getOrders() |> List.choose (applyOrderFilter filter)
...

Now, to the question at hand: The individual filter functions are (and were designed to be) easy to unit test. In my fairly inexperienced opinion, the testworthiness of applyOrderFilter is then questionable: All its constituent central parts are thoroughly tested; one can easily verify (IMHO) just by looking at it that it works correctly; and composition of functions in this manner seems to mostly be a case of "if it compiles, it works".

The one very important exception to this is the actual list of functions you compose, where you'll get no warnings if you forget to include one of the filter functions. (The partial application is another source of potential errors - in this case, you could switch the start/end date arguments.) Since filtering the data correctly is business-critical, and not easily discovered by the user (who only sees aggregate data), I'm in favor of having some kind of automated test for applyOrderFilter.

Now, unit testing applyOrderFilter seems like it would have to duplicate all the tests for the individual filter functions and cause a combinatorial explosion of test cases. It would also be overkill, because the only relevant thing to test seems to be just the combination of the filters, to protect against regressions (accidentally deleting a line in the filter composition would be really bad).

Where I need help is in figuring out the following:

  1. Is it necessary to test such function composition at all? I've given my reasons above, but I might be erring on the side of too much testing and not seeing counter-arguments (such as "making that kind of edit is so obviously wrong it would be unforgivable" - at some point you simply can't take into account any more developer stupidity/oversight).

  2. How can the filter composition be easily and robustly tested without having to test the actual filter logic? It seems to me like it would need a refactor to make this possible, but I'm stuck figuring out how to design this for an easy test.

  • There is a budget and priority for testing, based on riskiness of the code, just like all other code. You also have to consider that every test is additional code, therefore additional cost in terms of maintenance, if the code is ever likely to change. So you are right to propose these questions. – Frank Hileman Sep 18 '17 at 16:35
3

Kent Beck:

I get paid for code that works, not for tests, so my philosophy is to test as little as possible to reach a given level of confidence (I suspect this level of confidence is high compared to industry standards, but that could just be hubris). If I don't typically make a kind of mistake (like setting the wrong variables in a constructor), I don't test for it. I do tend to make sense of test errors, so I'm extra careful when I have logic with complicated conditionals. When coding on a team, I modify my strategy to carefully test code that we, collectively, tend to get wrong.

The primary use case for unit tests is to support safe refactoring; so the sort of heuristic I would be thinking about here is "how likely is it to break this code while refactoring?". If the type system is going to protect you from the errors you commonly make, then you probably don't get a lot of value out of writing additional tests.

How can the filter composition be easily and robustly tested without having to test the actual filter logic?

By providing stub implementations in place of the actual filters? which your current design doesn't seem to support. That is to say, you seem to be missing a method that looks something like

let compose dateConstraint customerConstraint salesConstraint = ...

which you could invoke via

compose stub stub stub

You should probably review Mark Seeman's blog, he writes quite a bit about managing dependencies.

Still, it kinda feels like this only solves part of the problem and moves the rest up one layer - since now the caller (or an intermediary) will have to pass the correct functions. Did I perhaps misunderstand something?

It is mostly just moving stuff around.

let processOrder order =
    if   validateOrder order 
    then acceptOrder   order 
    else declineOrder  order

vs

let defineOrderProcess v a d order =
    if   v order
    then a order
    else d order

let processOrder order = defineOrderProcess validateOrder acceptOrder declineOrder order

Which is to say, if you agree that the first approach "obviously has no deficiencies", then you leave it alone. Otherwise, you introduce a two step, you create a testable composition (defineOrderProcess) along with an invoked specialization of the composition (which is too simple to fail).

In your specific example, part of the problem that I see is that your problem isn't properly decomposed

let applyOrderFilter filter order =
    let (>=>) f1 f2 = f1 >> (Option.bind f2)
    let combinedFilter =
        filterStartDate filter.StartDate
        >=> filterEndDate filter.EndDate
        >=> filterCustomerId filter.CustomerId
        >=> filterSalesPersonId filter.SalesPersonId
    order |> combinedFilter

This function has a number of different responsibilities; it's poking into the internals of filter, and its defining a list of filtering functions, and it's defining a fold over that list of functions.

So you should start out by thinking about the list of filtering functions, and what the implementation would look like if that list were explicit

[ filter -> filterStartDate     filter.StartDate
; filter -> filterEndDate       filter.EndDate
; filter -> filterCustomerId    filter.CustomerId
; filter -> filterSalesPersonId filter.SalesPersonId
; ... ]

So you would have three pieces -- the individual constraints (which have unit tests), the fold (too boring to fail once the type system blesses it), and the explicit list of constraints -- which with more investment in naming would probably reach the point of being obviously correct.

[ dateConstraint
; customerConstraint
; salesConstraint
; ... ]

In other words, you should be banging on this implementation until the list falls out, and then decide if you need to write unit tests for the elements of a list.

Punchline: with the idea of this attempt at refactoring in mind, how do you feel about attempting this refactoring with your current test coverage? If you find yourself hesitant, thinking that you would want to have more tests in place before you start the refactoring, then that is a big hint that yes, you do need "to test such function composition at all".

4

As you stated, it is easy to forget a filter function in the list and this error would be hard to detect in later tests. This means that you really should write tests for applyOrderFilter.

The tests for applyOrderFilter should not duplicate all tests for the individual filters, but they should test the functionality that applyOrderFilter adds on top of the individual filter functions.
This can be tested by using specially crafted test-data such that in each test case you know that one specific filter would cause an input element to be included or excluded from the result set. That way, you know that the filter function is being used by applyOrderFilter.

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.