Monday, 3 May 2021

Testing: A Horror Story

G'day:

My adventures in Lucee / CFWheels / Docker continues. This time I'm looking at what CFWheels offers by way of testing.

2021-05-04 - editorial update

In this article I am very - but I feel justifiably - harsh about RocketUnit (you need to read down a bit before you get to this). I'm also harsh towards the CFWHeels team's historical decision to include it in CFWheels. I really do think that was a poor decision. But so be it, it's done: other than mentioning it, there's no point me dwelling on it beyond that.

Some of the tone and wording of this article has be read as an indictment of CFWheel and/or its team. Beyond that initial poor decision, all my opprobrium here is directed at RocketUnit, not CFWheels, and not the team. I can see how people would think otherwise though.

To be clear: this article is largely about RocketUnit. CFWheels was just the path via which I came to know about it.

I'm going to be brief, because it's so appalling I don't want to waste too much time on it. It can be summarised thus:

Are you f***ing joking?

Firstly I'm gonna quote the CFWheels docs (Testing Your Application › The Test Framework):

Like everything else in CFWheels, the testing framework is very simple, yet powerful. You don't need to remember a hundred different functions because CFWheels' testing framework contains only a handful.

(my emphasis there)

Sorry but I see wording like this implying something about CFML developers' capabilities all the time, and it really annoys me. To me that statement is patronising as well as disingenuous. It plays on this perception that somehow CFML devs aren't capable of doing anything unless it's reeeeally easy, and that that is just how things should be. Can we please not? Can we please not normalise CFML devs as somehow not being able to cope with a small amount of complexity if it will ultimately assist them growing as developers? It's also a bit disingenuous because no testing framework requires one to learn any more than a handful of methods, and learning how to write code is our job. It suggests testing tooling is somehow usually hard (which it just isn't).

Next I read further into the docs:

Evaluation

assert(): This is the main method that you will be using when developing tests. To use, all you have to do is provide a quoted expression. The power of this is that ANY 'truthy' expression can be used.

An example test that checks that two values equal each other:

function testActualEqualsExpected() {
    actual = true;
    expected = true;
    assert("actual eq expected");
}

I'm sorry, come again? You. Give. It. A. Quoted. Expression. It's using evaluate(), isn't it? Now I'm not actually fundamentally against evaluate as a thing. Not like some of the CFML community groupthink. It has its place, just that that place is seldom "in your code": one hardly ever needs to use it. I can't wait to see why it's being used here. Other than, like, cos for some reason the assert function expects a string, so it needs to be evaluated to even work.

And "[the] power of this is that ANY 'truthy' expression can be used"? So: just like any other implementation of assert that any other testing framework ever written. Except none of those need to pass a string to the assert function. This is not a MSP of this testing framework, and the implementation we're being shown is inferior to any other testing framework I've seen. I won't look at the actual code for this just yet, as there's still more horror to see in the guidance docs first.

I'm going to wind back up the docs a bit now:

Do not var-scope [any] variables used in your tests. In order for the testing framework to access the variables within the tests that you're writing, all variables need to be within the component's variables scope.

When I first read that I was like "oh yer having a laugh, right?" and it was not until I got to the "boolean expression as a string" and "evaluate" that it suddenly made "sense". Because the string expression needs to be evaluated within the implementation of assert, of course the variables in the "expression" can't be function-local to your test: assert's code won't be able to see those. So now because the implementation has chosen to pass the test expression as a string, it's forcing us to write bad, flaky, fragile test code, with variables bleeding all over the place by design. I can't really see how this framework could ever be used seriously outside a proof of concept or other very superficial environment. It'd drive the devs batty.

And now I hasten to add, the rest of the CFWHeels docs on testing are actually pretty helpful when it focuses-away from the test framework, and back to CFWheels stuff.

So what's with this test code? What is going on with this assert function? Oh yeah, btw: assert is the only assertion this framework offers. So there's goes your elegant, expressive, easy to understand test code. It's just a bunch of vanilla assert calls now. This is a telling stark contrast every other test framework out there that has decided to implement a lot of different assertions, and sees this as a good thing. I mean obviously at the end of the day they are all asserting if something is true; if you look at the internal implementations, generally all the facade assertions end up doing just that: calling a central "assert-true" assert method. The situation-specific assertions are there to make your test code easier and simpler to understand, which is vital in testing. I cannot understand why there was a perception that it was a good thing for the framework to have only one assertion.

Right. The code. Which is all in wheels/test/functions.cfm (nice file name there, CFWheels Team :-|. And I also have yet to work out why CFWheels is designed such that all the code for its .cfc files are implemented in .cfm files: this is a question for another day).

This is interesting:

Copyright 2007 RocketBoots Pty Limited - http://www.rocketboots.com.au

OK so here's where I clock that CFWheels have just bundled this "RocketUnit" thing into the framework. OK so in defence of the CFWheels team, this is possibly just a really poor decision to include this, rather than the team actively writing this… this… thing. I note that CFWheels (in 2021) still bundles only v1.0 of RocketUnit (from something like 2007).

And here we go:

public void function assert(required string expression) {
    // other stuff snipped
    if (!Evaluate(arguments.expression)) {

And why is it doing this? This is hilarious. The way this code has been implemented, and the reason that one needs to pass a string to assert is because if the assertion fails, then the code basically picks through the string, does a primitive tokenisation effort to find dynamic expressions, and then evaluates them again to get values to return in the assertion-failed message. EG; if your expression is "x == 3" and variables.x is 4, it'll re-evaluate each element of the string so it can say something like "uh-oh x was 4 not 3". And the entire reason it needs to do this with an equality-check is it's shot itself in the foot by only having assert, instead of taking the obvious route of having an equality assertion that takes two values to compare. Neither of which need to be in a global scope; neither of which need to be re-evaluated strings, because the assertion was passed their values. It could be called, I dunno, assertEquals or something.

It actually gets better (read: "worse"). In v2.x of RocketUnit, there's no need for the quoted string any more, because what the assertion implementation does when the assertion fails is it thows a fake exception to generate a callstack, and then crawls its way up that to try to find the line of code that called assert, and extract the expression from that. Seriously, have a look:

try {
    throw(errorCode=DUMMY_ERRCODE);
} catch(any) {
    // assert is one stack frame up from this function [1], therefore [2]
    source = cfcatch.tagContext[2].codePrintPlain;
    startLineNumber = lineNumber = cfcatch.tagContext[2].line;
}

If you look further down, you can see how the implementation itself knows how flaky its own approach even is, given the various places the code that tries to extract the expression needs to bail out.

Argh!

When discussing this with another CFML community member, they sent me this:

* see footnote regarding usage of this image

I think this is something that should have set-off alarm bells when the RocketUnit project first started… evolving.


Here's how it would seem that one needs to safely write tests with this shambles:

component extends="app.tests.Test" {

    function testBasicRoutingWorks() {
        variables.response = processRequest(
            params = {
                controller = "testroute",
                action = "debug"
            },
            returnAs = "struct"
        )
        try {
            assert("variables.response.status eq 200")
            assert("variables.response.body contains 'EXPECTED_CONTENT'")
        } finally {
            structDelete(variables, "response")
        }
    }
}

One can't just leave variables-scoped variables lying around the place in test code, so you need to get rid. Test code needs to be well encapsulated and robust and not prone to interference from unexpected avenues. Or one could just have one test per CFC. Or one could painstakingly make sure that tests don't accidentally share variables. Or hey just suck it and see (I suspect this is what RocketUnit expects). It's just too easy to accidentally not reinitialise a variable and be using an uncontrolled value from another test in a subsequent one.

Do not var-scope [any] variables

I have to wonder why - at the point one realises one needed to document something that contravenes standard good coding practice that languages go out of their way to accommodate - the person didn't go "ah now lads we've f***ed this one", and rethink things.


Do me a favour if you're using CFWheels? Don't use its in-built testing framework. It's shite. Use TestBox instead. It's dead easy to use, still supported, still being actively developed on a daily basis, and makes for really nice easy to read, easy to develop test code. There's a strong and willing community out there to help you if you get stuck with anything. It'll also be a cinch to test yer CFWheels work with, as it's completely situation-agnostic.

And to the CFWheels team: please consider just ripping this out of CFWheels wholesale. This is really letting your framework down, I think.

Righto.

--
Adam

* I am not the copyright holder of this image. I'm hoping I'm covered by "fair use" here, but if you are the copyright holder and disagree: let me know, and I'll replace it with something else.