Showing posts with label TestBox. Show all posts
Showing posts with label TestBox. Show all posts

Friday 24 March 2023

CFML / TestBox: spying on a method without mocking it

G'day:

Whilst looking for something else, I came across a ticket in TestBox's Jira system the other day that I had voted for a while back: Ability to spy on existing methods: $spy(). About a year or so ago I really had a need for this - hence finding the issue in there in the first place - sadly it was not around that the time, but I'm pretty pleased to see it's coming to TestBox 5.x soon (it still seems to be shipping to ForgeBox as a snapshot for now, but it's installable).

I can't actually remember what I needed to be testing back when this first became relevant for me, but I work on a legacy application which was never written with testing in mind, and we need to spend a lot of time horsing about with mocking stuff out so as to be able to test code adjacent to stuff that absolutely won't run in a test environment. That's fine if one just wants to mock something away: TestBox has that nailed. But sometimes - sometimes - with legacy code one really needs to leave a piece of code running - its side-effects might be essential to part of the test for example - but also see what arguments it gets passed, probably because we're dealing with an method that hundreds of lines long, and the change we're making hits logic in multiple parts of it. In the perfect world we'd refactor code like that before we test it, but a) we don't reside in the perfect world; b) if you don't already have tests (we don't), then it's not "refactoring", it's "just changing shit". Sometimes we have to accept further technical debt and not "just change shit" to make testing easier. I really hate this, but it's a reality we have.

Anyway. The way one does this in a test framework is via spying. There's plenty of writing out there that differentiates between stubs, doubles, mocks and spies, but in my opinion that thinking is itself largely legacy. These days doing any of that is likely to be effected via a dedicated framework (like MockBox embedded in TestBox), and it's more a difference in usage rather than being anything really that different.

Let's have a look at some code.

For this exercise, I have this simple function to monkey about with:

public string function reverseThisString(required string stringToReverse) {
    return stringToReverse.reverse()
}

It reverses a string. Yeah I know CFML already has a function to do that (I'm using it here!), it's just something easy to use for some tests.

A baseline test here is simply to test it works:

it("shows the reverseThisString working as a baseline", () => {
    sut = new SpyTest()

    result = sut.reverseThisString("G'day world")

    expect(result).toBe("dlrow yad'G")
})

Yep. It works.

Oh, one thing to note in all these tests is that reverseThisString is actually within my test suite - SpyTest - so I'm actually instantiating an instance of the very class the test is running from (and then later mocking it and stuff). It's important to remember the instance of the class being used in the test run is not the same as the one I'm testing in the test run. if that makes sense.

Next let's demonstrate that preparing an object for mocking doesn't actually disable an object's methods at all:

component extends=BaseSpec {

    function beforeAll() {
        variables.mockbox = getMockBox()
    }

    function run() {
        describe("Tests $spy function in TestBox", () => {
            // …

            it("shows how mocking an object does not impact its methods", () => {
                sut = new SpyTest()
                mockbox.prepareMock(sut)

                result = sut.reverseThisString("G'day world")

                expect(result).toBe("dlrow yad'G")
            })
            
            // …
        })
    }
    // …
}

reverseThisString is still doing it's thing. It's not mocked.

And a quick look that a mocked object has a call log, but the call log doesn't include unmocked methods (only the mocked ones):

it("shows how an object's callLog does not include non-mocked methods", () => {
    sut = new SpyTest()
    mockbox.prepareMock(sut)
    sut.$("mockMe")

    sut.reverseThisString("G'day world")
    sut.mockMe()

    callLog = sut.$callLog()

    expect(callLog).toHaveKey("mockMe")
    expect(callLog).notToHaveKey("reverseThisString")
})

Oh yeah, there's a mockMe method in there too:

public void function mockMe() {
    throw "Test is invalid: this should be mocked-out"
}

So this is the crux of it. Mocked methods have call logs, so we are in effect spying on them all the time. But unmocked methods: no.

No - to state the obvious, I hope - if one mocks a method, it does not do anything:

it("shows how mocking a method prevents it from executing", () => {
    sut = new SpyTest()
    mockbox.prepareMock(sut)
    sut.$("reverseThisString")

    result = sut.reverseThisString("G'day world")

    expect(isNull(result)).toBeTrue()
})

Nuff said.

OK, so here's the solution: this new $spy functionality:

it("shows how spying a method leaves it operational, and has a call log", () => {
    sut = new SpyTest()
    mockbox.prepareMock(sut)
    sut.$spy("reverseThisString")

    result = sut.reverseThisString("G'day world")

    expect(result).toBe("dlrow yad'G")

    callLog = sut.$callLog()

    expect(callLog.reverseThisString[1][1]).toBe("G'day world")
})

Here I am just spying on my method, not mocking it; so when I call it: it still works. But I also have the call log. Job done.

That's all there is to that. I hasten to add that this sort of feature is only useful to have occasionally, but sometimes with untestable legacy code it's a life-saver.

I would say that if you need to use this when testing new code that you're developing, yer likely doing something wrong. That said, if you have a real-world example where this is useful when testing well-written new code, please share.

And the code for this effort is here: SpyTest.cfc.

Righto.

--
Adam

Sunday 16 May 2021

CFWheels: running TestBox instead of RocketUnit

G'day:

CFWheels ships with its own inbuilt (albeit third-party) testing framework. I discuss its merits in an earlier article: "Testing: A Horror Story". You can probably work out my opinion of the inbuilt testing framework - RocketUnit - from the title. That's really all you need to know to contextualise why I am now going to get TestBox working in a CFWheels context. One would expect that this would simply be a matter of installing TestBox and then using the CFWheels API to call methods on its classes to… um… use it. Not so fast there chief.

The CFWheels application has been implemented via a few opaque (to me) architectural decisions, which means it's not straight-forward to run the code outside the context of an application solely designed to respond to HTTP request in an MVC fashion. You've read along already perhaps with my experiences of hacking it about to even be installed in a sensible place ("Short version: getting CFWheels working outside the context of a web-browsable directory"). Code-wise, CFWheels is not an object-oriented application, instead being a more procedural affair, relying on include to share files full of functions. Lack of OO design means it's difficult to interact with in an isolated fashion that one might expect: leveraging/relying on encapsulation, abstraction, inheritance and various other programming techniques familiar to modern CFML. There's no API really, there's just a bunch of independent functions that are simultaneously disconnected from each other, but at the same time strongly-coupled: using a function homed in one .cfm file might require a function homed in another .cfm file, and one just has to know that. All functions are public (given they're not in CFCs, this is can't be helped), but not all of them are designed to be used outside of the CFWheels app (they're prefixed with $). Also the functions might not necessarily keep their variables encapsulated within their own scope; and also fairly frequently rely on other functions having done the same: exposing data into the calling context for it to be there for another funtion to use. There's no CFCs to instantiate, one just needs to include one or more of these .cfm files full of functions. This makes for a fairly fragile and confusing coding experience, IMO.

I'm not going to be daunted by this. I'm also not going to let this architectural approach bleed into my own code: I'm not writing procedural code when I'm using an OO language in 2021. Somehow I'm gonna wrap it up in an adapter component that my tests can leverage to test my code.

Controllers

My first task is to work out how to execute the code for a route programmatically. I could simply make an external HTTP request to to the route and inspect its response, but this approach won't help me with lower level testing (like calling model functionality, in the next step). So I'm going to work out how I can write code along these lines: myResponseObject = cfwheelsAdapter.someMethod(), so I can then poke and prod the response object to see if it's done the expected thing.

Looking through the CFWheels docs ("Testing Your Application"), I find this test example:

function testRedirectAndFlashStatus() {
  // define the controller, action and user params we are testing
  local.params = {
    controller="users",
    action="create",
    user={
      firstName="Hugh",
      lastName="Dunnit",
      email="hugh@somedomain.com",
      username="myusername",
      password="foobar",
      passwordConfirmation="foobar"
    }
    };

  // process the create action of the controller
  result = processRequest(params=local.params, method="post", returnAs="struct");
  
  // make sure that the expected redirect happened
  assert("result.status eq 302");
  assert("result.flash.success eq 'Hugh was created'");
  assert("result.redirect eq '/users/show/1'");
  
}

OK so I don't use a route, I just call the controller. That's cool, although I'd rather be doing this by providing a route, a method, and the various params the controller needs - query params, body content, headers, etc - and make like a request, but I might look into that later. For now the object of the exercise is to be able to call the CFWheels code and check the response. And the function I want to call is this processRequest. This is an example of what I was saying before: it's not a method of a controller object or anything, it's just a headless function. And looking at that code I have no idea where it's implemented or how it came to be available to call in my test method. CFWheels just assumes any code you write will already be in the right context for this to work. I'm going to create my adapter object and include whatever file that processRequest function is in.

Well. Actually first I'm gonna create a test for this operation (/test/functional/WheelsRequestRunnerTest.cfc):

component extends=testbox.system.BaseSpec {

    function run() {
        describe("Test WheelsRequestRunner functionality", () => {
            it("can execute a request and receive the expected response", () => {
                runner = new test.WheelsRequestRunner()

                result = runner.processRequest(
                    params = {
                        controller = "testRoute",
                        action = "httpStatusOkTest"
                    },
                    method = "get",
                    returnAs = "struct"
                )

                expect(result).toHaveKey("status")
                expect(result.status).toBe(200)
                expect(result).toHaveKey("body")
                expect(result.body).toInclude("[200-OK]")
            })
        })
    }
}

This is kinda eating its own dogfood. My test of the runner class is that it can be used to test a controller. The controller in question simply renders a view (indeed: there's no controller, it's just the view in this case), and the view does nothing other than emit [200-OK].

The WheelsRequestRunner class is gonna encapsulate any CFWheels files I need to include to make stuff work (and contain an variables-scope bleeding the include files leak). First up I need to expose this processRequest function, and before I do that, I need to find where CFWheels implements it. The only way to do this is just doing a find in the codebase for function processRequest. It turns out it's in /wheels/global/misc.cfm. Not a hugely helpful filename that, and I wonder why a very controller-specific function is in a file called global/misc: it'd be nice if a controller function was in a file called controller.cfm or something. Anyway, I sling misc.cfm into my runner class:

component {
    include template="/wheels/global/misc.cfm"
}

When I run my test now, I get an error:

No matching function [$ARGS] found
global.misc_cfm$cf.udfCall2(/wheels/global/misc.cfm:284)

I track-down $args (not an excellent fuction name: what is "$args" supposed to do?) in /wheels/global/internal.cfm, so I include that too.

component {
    include template="/wheels/global/misc.cfm"
    include template="/wheels/global/internal.cfm"
}

At this point my WheelsRequestRunner is exposing 72 public methods, when in reality I only need one of those. Less than ideal.

But we're not done: we have a second error:

No matching function [$GET] found
global.misc_cfm$cf.udfCall2(/wheels/global/misc.cfm:307)

I found $get in /wheels/global/util.cfm. I wonder what the perceived difference is between global/misc, and global/util? All three components of those file paths are just meaningless and generic filler terms. But anyway… I'll include this too, and get another error, suggesting probably a fourth file I'm going to need to include:

No matching function [$DOUBLECHECKEDLOCK] found
global.misc_cfm$cf.udfCall2(/wheels/global/misc.cfm:482)

I found $doubleCheckedLock in /wheels/global/cfml.cfm. I'll include this too, and…

 

It works!

Note: WheelsRequestRunner has needed to expose 99 functions, spread across four disparate files, to avail itself of the one function it needed to call:

component {
    include template="/wheels/global/misc.cfm"
    include template="/wheels/global/internal.cfm"
    include template="/wheels/global/util.cfm"
    include template="/wheels/global/cfml.cfm"
}

I just want to do another controller test as well: an unhappy-path one:

it("can test a 400 response", () => {
    try{
        result = runner.processRequest(
            name = "testRoute",
            params = {
                controller = "testRoute",
                action = "badRequest"
            },
            method = "get",
            returnAs = "struct"
        )

        expect(result).toHaveKey("status")
        expect(result.status).toBe(400)
        expect(result).toHaveKey("body")
        expect(result.body).toInclude("[400-BAD-REQUEST]")
    } finally {
        header statuscode=200;
    }
})

This worked too without any more adjustment to WheelsRequestRunner. I've needed to put that try/finally around the test code because of the way CFWheels handles responses. It doesn't create a response object or anything like that that I could set my response status code in, the CFWheels approach is just that if I want to change the response status, I just change it … directly changing the header of the current request (in this case the request being made by the test runner). This is less than ideal. I'm currently using CFML's inbuilt header statement to do this, later when I work out how to do dependency injection with CFWheels I'll write my own wrapper to do this, and mock it out for the test. I do need to actively address this in my test though, because returning a 400 also means the test run reports as a failure because it doesn't complete with a 200-OK. Easily fixed, but I really oughtn't have to do this sort of thing.

For now, I'm satisfied I can test basic controllers. I want to move on to test a basic model, because I expect I'll need to do some more reconfiguration to make that work.

Models

I've created a dead-simple model class:

component extends=models.Model {

    function init()
    {
        table(false)
        property(name="name", label="Name")
        property(name="email", label="Email address")
        property(name="password", label="Password")
    }
}

I'm just going to check that I can create an instance of one of these, and the property values get used:

it("can test a model", () => {
    properties = {name="TEST_NAME", email="TEST_EMAIL", password="TEST_PASSWORD"}

    model = runner.model("SomeModel")
    testModel = model.new(properties)
    expect(testModel).toBeInstanceOf("models.SomeModel")
    expect(testModel.properties()).toBe(properties)
})

When I run this, I get another instance of CFWheels not being able to find something:

invalid component definition, can't find component [...src.models.SomeModel]

This is a different situation though: this is because - I am betting - because I am running the request from the /test/ directory, not the site's root. I'm going to need to dig around to see how CFWheels concludes that it should be looking for SomeModel in ...src.models.SomeModel. To misquote Captain Oates: "I am just going [into the CFWheels codebase] and may be some time."…

Right so I was close, but it was more my fault than anything. Kinda. Firstly I had a setting slightly wrong in my config/settings.cfm, due to a misunderstanding as to what CFWheels was expecting with some settings. Previously when configuring my CFWheels site to work with the source code "above" the web root, I had to make a coupla settings changes for CFWheels to be able to find my controllers and views, and those settings took file system paths:

set(viewPath = "../src/views")
set(controllerPath = "/app/controllers")

Without thinking I had just assumed that the equivalently-named modelPath would also be a file-system path, and had set it thus:

set(modelPath = "../src/models")

However it's actually a component path reference, so just needs to be this:

set(modelPath = "/models")

And this is used in conjunction with a mapping I had missing in Application.cfc:

component {

    // ...    

    thisDirectory = getDirectoryFromPath(getCurrentTemplatePath())
    // ... 
    this.mappings["/models"] = getCanonicalPath("#thisDirectory#models")
    // ... 
}

If I didn't have both of those, CFWheels was resolving the CFC path to be ...src.models.SomeModel, and even with that fixed, it was looking for the correct path (models.SomeModel) from a base context of the public directory, not the src directory. Once I sorted that out, the test passed.

Views

I wasn't actively going to test this because I've been wittering on enough already in this article, but last week I'd actually already written some tests that checked how the linkTo function worked under different URLRewriting settings, and I've just noticed the one test I had that was working satisfactorily regarding this is currently broken, and it looks to be down to what I've been doing with TestBox recently. Here's the test:

it("uses the URI when URLRewriting is on", () => {
    runner = new test.WheelsRequestRunner()
    runner.set(URLRewriting="On")

    result = runner.processRequest(
        params = {
            controller = "testRoute",
            action = "linktotest"
        },
        method = "get",
        returnAs = "struct"
    )

    expect(result).toHaveKey("body")
    expect(result.body).toMatch('<a\b[^>]*href="/testRoute/linkToTestTarget"[^>]*>')
})

It's pretty simple: it hits an endpoint that returns mark-up with a link generated by linkTo:

<cfoutput>#linkTo(route="testRouteLinkToTestTarget",text="Click Me")#</cfoutput>

This test had been working, but it's erroring with because the URL for the link it's returning is runTests.cfm/testroute/linktotesttarget. it should be just /testRoute/linkToTestTarget. "Coincidentally" the name of the file I'm running the tests from is… runTest.cfm. Hazarding a guess, I am assuming the linkTo is doing some shenanigans with a find and replace expecting the CGI.script_name to be just /index.cfm. The script name shouldn't come into it with a fully re-written URL, given it doesn't figure in it, but… well…. I'll track down the code in question…

… yeah it was as I suspected. In /wheels/global/misc.cfm we have this:

// When URL rewriting is on we remove the rewrite file name (e.g. rewrite.cfm) from the URL so it doesn't show.
// Also get rid of the double "/" that this removal typically causes.
if (arguments.$URLRewriting == "On") {
    local.rv = Replace(local.rv, application.wheels.rewriteFile, "");
    local.rv = Replace(local.rv, "//", "/");
}

(For future reference, CFWheels Team, if you feel the need to block-out code like that with an explanatory comment, it means a) your function is doing too much (this function is close to 200 lines long!!!); b) the code that's been blocked-out should be in its own function)

So yeah, CFWheels expecting one of two possibilities for the file part of the script name here: index.cfm or rewrite.cfm. My runTests.cfm doesn't cut the mustard. However this is easy enough to solve, I can override the setting in test/Application.cfc:

component extends=cfmlInDocker.Application {

    this.name = "testApplication"

    // ...

    function onApplicationStart() {
        super.onApplicationStart()
        set(rewriteFile="runTests.cfm")
    }

    // ...

}

I've also given the test app a new name so its application scope settings don't interfere with the application scope for the front of the site.

Summary

To get at least controllers and models testable wasn't much effort. I had to do this:

That's not so bad. It was more hassle finding out where the relevant CFWheels functions were, and following the logic to see what I had to override/fix and why, but the end result was all right. I'm just glad I can write decent tests now.

Righto.

--
Adam

Monday 19 April 2021

Adding TestBox, some tests and CFConfig into my Lucee container

G'day:

On Fri/Sat (it's currently Sunday evening, but I'll likely not finish this until Monday now) I started looking at getting some CFML stuff running on Lucee in a Docker container (see earlier/other articles in this series: Lucee/CFWheels/Docker series). If you like you can read about that stuff: "Using Docker to strum up an Nginx website serving CFML via Lucee" and "Repro for Lucee weirdness". This article resumes from where I got to with the former one, so that one might be good for some context.

Full disclosure: I spent all today messing around in a spike: experimenting with stuff, and now am finally happy I have got something to report back on, so I have rolled-back the spike and am going to do the "production" version of it via TDD again. I just say this - and it's not the first time - if yer doing TDD it's OK to spike-out and do a bunch of stuff to work out how to do things without testing every step. Especially if yer like me and start from a position of having NFI what you need to do. However once yer done: revert everything and start again, testing-first as you go. What I've done here is save all my stuff in a branch, and now I'm looking at a diff of that and main, as a reference to what I actually need to do, and what is fluff that represents a dead end, or something I didn't need to do anyhow, or whatever.

It needs to only expose public stuff to the public

As per the article subject line, today I'm gonna install a bit more tooling and get me in a better position to do some dev work. The first thing I noticed is that as things stand, the Nginx wesbite is serving everything in the Lucee application root (/var/www), whereas that directory is going to be a home directory for the application code, test code and third-party apps, so I don't want that browsable. I'm going to shift things around a bit. A notional directory structure would be along these lines:

root
├── public
│   ├── css
│   ├── images
│   ├── js
│   ├── Application.cfc
│   ├── favicon.ico
│   └── index.cfm
├── src
│   └── Application.cfc
├── test
└── vendor
    ├── cfwheels
    └── testbox

I've taken a fairly Composer / PHP approach there, but I could equally follow a more Java-centric approach to the directory structure:

root
├── com
│   └── ortussolutions
│       └── testBox
├── me
│   └── adamcameron
│       └── cfmlInDocker
│           ├── src
│           │   └── Application.cfc
│           └── test
├── org
│   └── cfwheels
│       └── cfwheels
└── public
    ├── css
    ├── images
    ├── js
    ├── Application.cfc
    ├── favicon.ico
    └── index.cfm

The point being: the website directory and my code and other people's code should be kept well away from one another. That's just common-sense.

Anyway, back to the point. Whichever way I organise the rest of things, only stuff that is supposed to be browsed-to should be browsable. Everything else should not be. So I'm gonna move the website's docroot, as well as the files that need to be served. This is just a "refactoring" exercise, so no tests should change here. We just want to make sure they still all pass.

This just means some changes to docker-compose.yml:

lucee:
    build:
        context: ./lucee
    volumes:
    	- ../public:/var/www
        - ../public:/var/www/public
        - ../root/src:/var/www/src
        - ../test:/var/www/test
        - ../var/log/tomcat:/usr/local/tomcat/log
        - ../var/log/lucee:/opt/lucee/web/logs
        - ./lucee/root_home:/root

And the website config (docker/nginx/sites/default.conf):

location ~ \.(?:cfm|cfc) {
    # ...

    proxy_pass  http://cfml-in-docker.lucee:8888$fastcgi_script_name$is_args$args;
    proxy_pass  http://cfml-in-docker.lucee:8888/public$fastcgi_script_name$is_args$args;
}

Once a rebuild the containers, I get two failing tests. Oops: I did not expect that. What's going on? Checking the front-end, the public-facing website still behaves the same. So… erm …?

One thing that didn't occur to me when doing this change is that a couple of the tests are hitting the internal Lucee website (reminder: the public-facing website for me is http://cfml-in-docker.frontend/ and the internal Lucee web site is http://cfml-in-docker.lucee:8888/). And that internal website still points to /var/www/, so where previously I'd access http://cfml-in-docker.lucee:8888/remoteAddrTest.cfm, now the URL for the backend site is be http://cfml-in-docker.lucee:8888/public/remoteAddrTest.cfm. This is by design (kinda, just… for now), but I forgot about this when I made the change.

This means to me that my change is not simply a refactoring: therefore I need to start with a failing tests. I roll back my config changes, fix the tests so they hit http://cfml-in-docker.lucee:8888/public/gdayWorld.cfm and http://cfml-in-docker.lucee:8888//public/remoteAddrTest.cfm respectively, and watch them fail. Good. Now I roll forward my config changes again and see the tests pass: cool. Job done.

Later when I'm reconfiguring things I might remap it to /var/www/public, if I can work out how to do that without hacking Tomcat config files too much. But remember the test case here: It needs to only expose public stuff to the public. And we've achieved that. Let's not worry about a test case we don't need to address for now. Moving on…

It can run tests with TestBox

Currently I am running my tests via a separate container running PHP and PHPUnit. This has been curling the website to test Nginx and Lucee behaviour. Now that I have Lucee working, I can shift the tests to TestBox, which is - as far as I know - the current state of the art when it comes to testing CFML code. It provides both xUnit and Jasmine-style testing syntax.

The test for this is going to be a "physician heal thyself" kind of affair. I'm going to write a TestBox test. Once I can run it and it doesn't just go splat: job done. The test is simply this:

component extends=testbox.system.BaseSpec {

    function run() {
        describe("Baseline TestBox tests", () => {
            it("can verify that true is, indeed, true", () => {
                expect(true).toBe(true)
            })
        })
    }
}

Testbox seems to be web-based. I'd much prefer just running my tests from the shell like I would any other testing framework I've used in the last 7-8 years, but CFML has much more "it's aimed at websites, so evertything is implemented as a website" mentality (hammers and nails spring to mind here). So be it I guess. I do see that TestBox does have the ability to integrate with Ant, but that seems to be via an HTTP request as well. Hrm. What I know is I can't simply do something like testbox /path/to/my/tests or similar. What I do need to do is write a small runner file (runTests.cfm), which I then browse to:

<cfscript>
    testBox = new testbox.system.TestBox(directory="cfmlInDocker.test")
    result = testBox.run(
        reporter = "testbox.system.reports.SimpleReporter"
    )
    writeOutput(result)
</cfscript>

To use that testbox.system and cfmlInDocker.test paths, I need to define mappings for them at application level (ie: not in the same file that uses it, but a different unrelated file, Application.cfc):

component {
    this.mappings = {
        "/cfmlInDocker/test" = expandPath("/test"),
        "/testbox" = expandPath("/vendor/testbox")
    }
}

And when I browse to that, I get a predictable error:

Let's call that our "failing test".

OK so right, we install TestBox from ForgeBox (think packagist.org or npmjs.com). And to install stuff from ForgeBox I need CommandBox. And that is pretty straight forward; just a change to my Lucee Dockerfile:

FROM lucee/lucee:5.3

RUN apt-get update
RUN apt-get install vim --yes

COPY ./root_home/.bashrc /root/.bashrc
COPY ./root_home/.vimrc /root/.vimrc

WORKDIR  /var/www

RUN curl -fsSl https://downloads.ortussolutions.com/debs/gpg | apt-key add -
RUN echo "deb https://downloads.ortussolutions.com/debs/noarch /" | tee -a /etc/apt/sources.list.d/commandbox.list
RUN apt-get update && apt-get install apt-transport-https commandbox --yes
RUN echo exit | box
EXPOSE 8888

That last step there is because CommandBox needs to configure itself before it works, so I might as well do that when it's first installed.

Once I rebuild the container with that change, we can get CommandBox to install TestBox for us:

root@b73f0836b708:/var/www# box install id=testbox directory=vendor savedev=true
√ | Installing package [forgebox:testbox]
   | √ | Installing package [forgebox:cbstreams@^1.5.0]
   | √ | Installing package [forgebox:mockdatacfc@^3.3.0+22]

root@b73f0836b708:/var/www#
root@b73f0836b708:/var/www#
root@b73f0836b708:/var/www# ll vendor/
total 12
drwxr-xr-x 3 root root 4096 Apr 19 09:23 ./
drwxr-xr-x 1 root root 4096 Apr 19 09:23 ../
drwxr-xr-x 9 root root 4096 Apr 19 09:23 testbox/
root@b73f0836b708:/var/www#

Note that commandbox is glacially slow to do anything, so be patient rather than be like me going "WTH is going on here?" Check this out:

root@b73f0836b708:/var/www# time box help

**************************************************
* CommandBox Help
**************************************************

Here is a list of commands in this namespace:

// help stuff elided…

To get further help on any of the items above, type "help command name".

real    0m48.508s
user    0m10.298s
sys     0m2.448s
root@b73f0836b708:/var/www#

48 bloody seconds?!?!. Now… fine. I'm doing this inside a Docker container. But even still. Blimey fellas. This is the equivalent for composer:

root@b21019120bca:/usr/share/cfml-in-docker# time composer --help
Usage:
  help [options] [--] [<command_name>]

// help stuff elided…


To display the list of available commands, please use the list command.

real    0m0.223s
user    0m0.053s
sys     0m0.035s
root@b21019120bca:/usr/share/cfml-in-docker#

That is more what I'd expect. I suspect they are strumming up a CFML server inside the box application to execute CFML code to do the processing. Again: hammer and nails eh? But anyway, it's not such a big deal. The important thing is: did it work?

Yes it bloody did! Cool! Worth the wait, I say.

I still need to find a way to run it from the shell, and I also need to work out how to integrate it into my IDE, but I have a minimum baseline of being able to run tests now, so that is cool.

The installation process also generated a box.json file, which is the equivalent of a composer.json / packages.json file:

root@b73f0836b708:/var/www# cat box.json
{
    "devDependencies":{
        "testbox":"^4.2.1+400"
    },
    "installPaths":{
        "testbox":"vendor/testbox/"
    }
}

It doesn't seem to have the corresponding lock file though, so I'm wondering how deployment works. The .json dictates what could be installed (eg: for testbox it's stating it could be anything above 4.2.1+400 but less than 5.0), but there's nothing controlling what is installed. EG: specifically 4.2.1+400. If I run this process tomorrow, I might get 4.3 instead. It doesn't matter so much with dev dependencies, but for production dependencies, one wants to make sure that whatever version is being used on one box will also be what gets installed on another box. Which is why one needs some lock-file concept. The Composer docs explain this better than I have been (and NPM works the same way). Anyway, it's fine for now.

Now that I have the box.json file, I can simply run box install in my Dockerfile:

# …
WORKDIR  /var/www

RUN curl -fsSl https://downloads.ortussolutions.com/debs/gpg | apt-key add -
RUN echo "deb https://downloads.ortussolutions.com/debs/noarch /" | tee -a /etc/apt/sources.list.d/commandbox.list
RUN apt-get update && apt-get install apt-transport-https commandbox --yes
RUN echo exit | box

COPY ./box.json /var/www/box.json
RUN mkdir -p /var/www/vendor
RUN box install

EXPOSE 8888

It runs all the same tests via TestBox as it does via PHPUnit

I'm not going to do some fancy test that actually tests that my tests match some other tests (I'm not that retentive about TDD!). I'm just going to implement the same tests I've already got on PHPUnit in TestBox. Just as some practise at TestBox really. I've used it in the past, but I've forgotten almost everything I used to know about it.

Actually that was pretty painless. I'm glad I took the time to properly document CFScript syntax a few years ago, as I'd forgotten how Railo/Lucee handled tags-in-script, and the actual Lucee docs weren't revealing this to me very quickly. That was the only hitch I had along the way.

All the tests are variations on the same theme, so I'll just repeat one of the CFCs here (NginxProxyToLuceeTest.cfc):

component extends=testbox.system.BaseSpec {

    function run() {
        describe("Tests Nginx proxies CFML requests to Lucee", () => {
            it("proxies a CFM request to Lucee", () => {
                http url="http://cfml-in-docker.frontend/gdayWorld.cfm" result="response";

                expect(response.status_code).toBe( 200, "HTTP status code incorrect")
                expect(response.fileContent.trim()).toBe( "G'day world!", "Response body incorrect")
            })

            it("passes query values to Lucee", () => {
                http url="http://cfml-in-docker.frontend/queryTest.cfm?testParam=expectedValue" result="response";

                expect(response.status_code).toBe( 200, "HTTP status code incorrect")
                expect(response.fileContent.trim()).toBe( "expectedValue", "Query parameter value was incorrect")
            })

            it("passes the upstream remote address to Lucee", () => {
                http url="http://cfml-in-docker.lucee:8888/public/remoteAddrTest.cfm" result="response";
                expectedRemoteAddr = response.fileContent

                http url="http://cfml-in-docker.lucee:8888/public/remoteAddrTest.cfm" result="testResponse";
                actualRemoteAddr = testResponse.fileContent

                expect(actualRemoteAddr).toBe(expectedRemoteAddr, "Remote address was incorrect")
            })
        })
    }
}

The syntax for making an http request uses that weirdo syntax that is neither fish nor fowl (and accordingly confuses Lucee itself as to what's a statement and what isn't, hence needing the semi-colon), but other than that it's all quite tidy.

And evidence of them all running:

I can get rid of the PHP container now!

It uses CFConfig to make some Lucee config tweaks

An observant CFMLer will notice that I did not var my variables in the code above. To non-CFMLers: one generally needs to actively declare a variable as local to the function its in (var myLocalVariable = "something"), otherwise without that var keyword it's global to the object it's in. This was an historically poor design decision by Macromedia, but we're stuck with it now. Kinda. Lucee has a setting such that the var is optional. And I've switched this setting on for this code.

Traditionally settings like this need to be managed through the Lucee Administrator GUI, but I don't wanna have to horse around with that: it's a daft way of setting config. There's no easy out-of-the-box way of making config changes like this outside the GUI, but there's a tool CFConfig that let's me do it with "code". Aside: why is this not called ConfigBox?

Before I do the implementation, I can actually test for this:

component extends=testbox.system.BaseSpec {

    function run() {
        describe("Tests Lucee's config has been tweaked'", () => {
            it("has 'Local scope mode' set to 'modern'", () => {
                testVariable = "value"

                expect(variables).notToHaveKey("testVariable", "testVariable should not be set in variables scope")
                expect(local).toHaveKey("testVariable", "testVariable should be set in local scope")
            })
        })
    }
}

Out of the box that first expectation will fail. Let's fix that.

Installing CFConfig is done via CommandBox/Forgebox, and I can do that within the Dockerfile:

RUN box install commandbox-cfconfig

Then I can make that setting change, thus:

RUN box cfconfig set localScopeMode=modern to=/opt/lucee/web

I'm going to do one more tweak whilst I'm here. The Admin UI requires a coupla passwords to be set, and by default one needs to do the initial setting via putting it in a file on the server and importing it. Dunno what that's all about, but I'm not having a bar of it. We can sort this out with the Dockerfile and CFConfig too:

FROM lucee/lucee:5.3

ARG LUCEE_PASSWORD

# a bunch of stuff elided for brevity…

RUN box install commandbox-cfconfig
RUN box cfconfig set localScopeMode=modern to=/opt/lucee/web
RUN box cfconfig set adminPassword=${LUCEE_PASSWORD} to=/opt/lucee/web # for web admin
RUN echo ${LUCEE_PASSWORD} > /opt/lucee/server/lucee-server/context/password.txt # for server admin (actually seems to deal with both, now that I check)

EXPOSE 8888

That argument is passed by docker-compose, via docker-compose.yml:

lucee:
    build:
        context: ./lucee
        args:
            - LUCEE_PASSWORD=${LUCEE_PASSWORD}

And that in turn is passed-in via the shell when the containers are built:

adam@DESKTOP-QV1A45U:/mnt/c/src/cfml-in-docker/docker$ LUCEE_PASSWORD=12345678 docker-compose up --build --detach --force-recreate

I'd rather use CFConfig for both the passwords, but I could not find a setting to set the server admin one. I'll ask the CFConfig bods. I did find a setting to just disable the login completely (adminLoginRequired), but I suspect that setting is only for ColdFusion, not Lucee. It didn't work for me on Lucee anyhow.

It has written enough for today

I was gonna try to tackle getting CFWheels installed and running in this exercise too, but this article is already long enough and this seems like a good place to pause. Plus I've just spotted someone being wrong on the internet, and I need to go interject there first.

Righto.

--
Adam

Saturday 5 March 2016

CFML: go have a look at a Code Review

G'day:
I've already put more effort into this than a Saturday morning warrants, so I'll keep this short and an exercise in copy and past.

Perennial CFMLer James Mohler has posted some code on Code Review:  "Filtering the attributes for a custom tag". I'll just reproduce my bit of the review, with his code by way of framing.

Go have a look, and assess our code as an exercise.

James's functions:

Original:

string function passThrough(required struct attr)   output="false"  {

    local.result = "";

    for(local.myKey in arguments.attr)    {
        if (variables.myKey.left(5) == "data-" || variables.myKey.left(2) == "on" || variables.myKey.left(3) == "ng-")  {

            local.result &= ' #local.myKey.lcase()#="#arguments.attr[local.myKey].encodeForHTMLAttribute()#"';
        } // end if 
    }   // end for

    return local.result;
}

His reworking:

string function passThrough(required struct attr)   output="false"  {

    arguments.attr.filter(
        function(key, value) { 
            return (key.left(5) == "data-" || key.left(2) == "on" || key.left(3) == "ng-");
        }
    ).each(
        function(key, value)    {
            local.result &= ' #key.lcase()#="#value.encodeForHTMLAttribute()#"';
        }
    );  

    return local.result;
}

Now for my code review (this is a straight copy and paste from my answer):

OK, now that I've had coffee, here's my refactoring:

function extractCodeCentricAttributesToMarkupSafeAttributes(attributes){
    var relevantAttributePattern = "^(?:data-|ng-|on)(?=\S)";

    return attributes.filter(function(attribute){
        return attribute.reFindNoCase(relevantAttributePattern);
    }).reduce(function(attributeString, attributeName, attributeValue){
        return attributeString& ' #attributeName#="#attributeValue#"';
    }, "");
}


Notes on my implementation:
  • I could not get TestBox working on my ColdFusion 2016 install (since fixed), so I needed to use CF11 for this, hence using the function version of encodeForHTMLAttribute(). The method version is new to 2016.
  • we could probably argue over the best pattern to use for the regex all day. I specifically wanted to take a different approach to Dom's one, for the sake of comparison. I'm not suggesting mine is better. Just different. The key point we both demonstrate is don't use a raw regex pattern, always give it a meaningful name.
  • looking at the "single-expression-solution" I have here, the code is quite dense, and I can't help but think Dom's approach to separating them out has merit.

Code review notes:
  • your original function doesn't work. It specifies variables.myKey when it should be local.myKey. It's clear you're not testing your original code, let alone using TDD during the refactoring process. You must have test coverage before refactoring.
  • the function name is unhelpfully non-descriptive, as demonstrated by Dom not getting what it was doing. I don't think my function name is ideal, but it's an improvement. I guess if we knew *why
  • you were doing this, the function name could be improved to reflect that.
  • lose the scoping. It's clutter.
  • lose the comments. They're clutter.
  • don't abbrev. variable names. It makes the code slightly hard to follow.
  • don't have compound if conditions like that. It makes the code hard to read. Even if the condition couldn't be simplified back to one function call and you still needed multiple subconditions: extract them out into meaningful variable names, eg: isDataAttribute || isOnAttribute || isNgAttribute
  • key and value are unhelpful argument names
  • slightly controversial: but unless it's an API intended to be used by third-parties: lose the type checking. It's clutter in one's own code.
  • there's no need for the output modifier for the function in CFScript.
  • don't quote boolean values. It's just false not "false".

Unit tests for this:

component extends="testbox.system.BaseSpec" {

    function beforeAll(){
        include "original.cfm";
        include "refactored.cfm";
        return this;
    }

    function run(testResults, testBox){
        describe("Testing for regressions", function(){
            it("works with an empty struct", function(){
                var testStruct = {};
                var resultFromOriginal = passThrough(testStruct);
                var resultFromRefactored = extractCodeCentricAttributesToMarkupSafeAttributes(testStruct);
                expect(resultFromRefactored).toBe(resultFromOriginal);
            });
            it("works with an irrelevant attribute", function(){
                var testStruct = {notRelevant=3};
                var resultFromOriginal = passThrough(testStruct);
                var resultFromRefactored = extractCodeCentricAttributesToMarkupSafeAttributes(testStruct);
                expect(resultFromRefactored).toBe(resultFromOriginal);
            });
            it("works with each of the relevant attributes", function(){
                var relevantAttributes = ["data-relevant", "onRelevant", "ng-relevant"];
                for (relevantAttribute in relevantAttributes) {
                    var testStruct = {"#relevantAttribute#"=5};
                    var resultFromOriginal = passThrough(testStruct);
                    var resultFromRefactored = extractCodeCentricAttributesToMarkupSafeAttributes(testStruct);
                    expect(resultFromRefactored).toBe(resultFromOriginal);
                }
            });
            it("works with a mix of attribute relevance", function(){
                var testStruct = {notRelevant=7, onRelevant=11};
                var resultFromOriginal = passThrough(testStruct);
                var resultFromRefactored = extractCodeCentricAttributesToMarkupSafeAttributes(testStruct);
                expect(resultFromRefactored).toBe(resultFromOriginal);
            });
            it("works with multiple relevant attributes", function(){
                var testStruct = {"data-relevant"=13, onRelevant=17, "ng-relevant"=19};
                var resultFromOriginal = passThrough(testStruct);
                var resultFromRefactored = extractCodeCentricAttributesToMarkupSafeAttributes(testStruct);
                expect(resultFromRefactored).toBe(resultFromOriginal);
            });
        });

    }

}


Use them. NB: the includes in beforeAll() simply contain each version of the function.

One thing I didn't put as I don't think it's relevant to this code review is that one should be careful with inline function expressions. They're not themselves testable, so if they get more than half a dozen statements or have any branching or conditional logic: extract them into their own functions, and test them separately.

Oh, and in case yer wondering... yes I did write the tests before I wrote any of my own code. And I found a coupla bugs with my planned solution (and assumptions about the requirement) in doing so. Always TDD. Always.

Anyway, all of this is getting in the way of my Saturday.

Righto.

--
Adam

Thursday 6 August 2015

CFML: code challenge from the CFML Slack channel

G'day:
I wasn't gonna write anything today, but then Jessica on Slack (to use her full name) posted a code puzzle on the CFML Slack channel, which I caught whilst I was sardined in a train to Ilford en route home. I nutted out a coupla solutions in my head instead of reading my book, and I saw this as a good opportunity (read: "excuse") to pop down to the local once I got home and squared away a coupla things, and key the code in and see if it worked. I was moderately pleased with the results, and I think I've solved it in an interesting way, so am gonna reproduce here.

Jessica's problem was thus:
so, i am attempting to write a function that lets you set variables specifically in a complex structure [...]
cached:{
    foo: {
        bar: "hi"
    }
}
setProperty("foo.bar", "chicken");
writeDump(cached); // should == cached.foo.bar = chicken

On the Slack channel there was talk of loops and recursion and that sort of thing, which all sounded fine (other people came up with answers, but I purposely did not look at them lest they influenced my own efforts). The more I work with CFML and its iteration methods (map(), reduce(), etc), the more I think actually having to loop over something seems a bit primitive, and non-descriptive. I looked at this for a few minutes... [furrowed my brow]... and thought "you could reduce that dotted path to a reference to the substruct I reckon". There were a few challenges there - if CFML had proper references it'd be easier - but I got an idea of the code in my head, and it seemed nice and easy.

Whilst waiting to Skype with my boy I wrote my tests:

Thursday 16 July 2015

CFML: ColdFusion does some dodgy exception "handling"

G'day:
This was a curly one. I've been doing some testing recently, so I've been giving TestBox a bit of a thrashing. Yesterday I was a bit bemused that some of my tests were failing (that's not actually any sort of surprise given what I'm testing. Ahem), but other tests were erroring. yet they were testing much the same thing, and the condition causing the failure was the same for both the failing and erroring tests. I thought there was something weird afoot with TestBox, so I hit-up Luis about it.



I boiled the repro down to this:

// ErrorInsteadOfFail.cfc

component extends=testbox.system.basespec {

    function run(){
        describe("Replicating issue", function(){

            it("demonstrates a baseline", function(){
                expect(false).toBeTrue();
            });

            it("demonstrates the error", function(){
                var results = [1,2,3];
                results.each(function(result){
                    expect(false).toBeTrue();
                });
            });

        });
    }

}

This is really contrived, so don't pay too much attention to the code. Just focus on these two bits:

  • calling expect() directly in my test;
  • calling expect() within an iteration function, within an inline function expression.

The expectation on each is the same: that false is true. Which it isn't (I'm not going too fast, right? ;-), so the test should fail.

However I get this:


Notice how the second test isn't failing, it's actually erroring.

I raised a ticket for this: TESTBOX-127.

Luis came back to me a coupla hours later with the interesting observation that it seems when an exception is thrown within a closure, then ColdFusion doesn't just let it error, it wraps it up in a different exception, and throws that.

That's a bullshit thing for ColdFusion to be doing. Lucee, btw, does not do this: it behaves properly.

I expanded my tests out to cover a couple more things:

Sunday 19 April 2015

PHP: unit testing private methods (a bit of reflection)

G'day:
Yes, I still do PHP as well as Lucee. Well, indeed, I don't do Lucee, I just seem to "invest" my time blogging about it. I am very much doing PHP all day every day again now.

Recently I revisited the notion of unit testing private methods. I realise a lot of people poopoo this notion as one is only supposed to unit test the interface, so only public methods. However when doing TDD, one needs to maintain one's private methods too, so this means one needs to test the behaviour of the change before making it.


Some argue these days that one should not have private methods: having them suggests bad class composition and a private method should be a public method of another class. I can see where they're coming from, but I also suspect the person deciding this was a Java developer and revels in the theory of OO put into practice, rather than... you know... getting shit done. I am in the "getting shit done" camp. I see no benefit in making classes for the sake of it, and I also like the idea of having code as close as possible to where it'll be used. So private helper methods have their place. And they need to be tested whilst being developed.

Others make the very good point that what actually needs testing is the nature of the impact on the result of the public method is what's important, so a private method ought to be tested "by proxy": call the public method, give it inputs that will invoke the to-be-developed new behaviour, and don't care - at test level - where the code happens to be. I have come to appreciate this idea, and it's how we approach a lot of our testing now.

One hitch with this is sometimes it's difficult to only hit the new functionality without also having to horse around appeasing other bits of functionality on the way through. Mocking makes this a lot easier, but not everything can be mocked, and even mocking stuff can be time and effort consuming. I guess if our code was perfect we'd not have this issue so much, but it's not (we're not), so we need to deal with that reality.

Sometimes it's just bloody easier to test a private method.

Back in my CFML days this was piss-easy. MockBox comes with a method makePublic(), which simply makes a private method public. It achieves this by injecting a proxy into the object under test which is public, and this wraps a call to the private method. One then calls that proxy in one's test.

Using makePublic() is as easy as:

mockbox.makePublic(object, "methodName");

Then continue to call methodName as per usual in one's tests. Cool.

This ain't so easy in PHP. One cannot just inject proxy functions into one's methods. However one can use reflection to make changes to an object's existing methods, including its access restrictions.

So what I set out to do was to make my own makePublic() method.

I arrived at this:

private function makePublic($object, $method){
    $reflectedMethod = new \ReflectionMethod($object, $method);
    $reflectedMethod->setAccessible(true);
    return $reflectedMethod;        
}

(it's private as it's just inline in my test class, the entire code for which can be seen here: SomeClassTest.php SomeClass.php).

This is simple enough. However actually using it is inelegant compared to the MockBox approach:

/**
@covers isIndexedArray
*/
function testIsIndexedArray(){
    $testValue = [53,59,61];
    $exposedMethod = $this->makePublic($this->testObject, 'isIndexedArray');
    $actual = $exposedMethod->invoke($this->testObject, $testValue);
    $this->assertTrue($actual);
}

That's no so clear.

So I decided to abandon that approach, instead coming up with this method:

private function executePrivateMethod($object, $method, $arguments){
    $reflectedMethod = new \ReflectionMethod($object, $method);
    $reflectedMethod->setAccessible(true);

    $result = $reflectedMethod->invoke($object, ...$arguments);
    return $result;        
}

This does the same thing, but it looks far more understandable in the calling code:

/**
@covers isIndexedArray
*/

function testIsIndexedArray_withAssociativeArray(){
    $testValue = ['a'=>67,'b'=>71,'c'=>73];

    $actual = $this->executePrivateMethod($this->testObject, 'isIndexedArray', [$testValue]);
    $this->assertFalse($actual);
}

It's really clear what executePrivateMethod does, I reckon.

I'mm going to show this to the lads at work tomorrow, and see what they think. Note: my position in general will be to test via the public interface, but in those situations where this is going to be a lot of work, or make the testing unclear, hopefully we can use this instead.

Thoughts?

--
Adam

Wednesday 9 July 2014

Some more TestBox testing, case/when for CFML and some dodgy code

G'day:
This is an odd one. I was looking at some Ruby code the other day... well it was CoffeeScript but one of the bits influenced by Ruby, and I was reminded that languages like Ruby and various SQL flavours have - in addition to switch/case constructs - have a case/when construct too. And in Ruby's case it's in the form of an expression. This is pretty cool as one can do this:

myVar = case
    when colour == "blue" then
        "it's blue"
    when number == 1 then
        "it's one"
    else
        "shrug"
end

And depending on the values of colour or number, myVar will be assigned accordingly. I like this. And think it would be good for CFML. So was gonna raise an E/R for it.

But then I wondered... "Cameron, you could probably implement this using 'clever' (for me) use of function expressions, and somehow recursive calls to themselves to... um... well I dunno, but there's a challenge. Do it".

So I set out to write a case/when/then/else/end implementation... as a single UDF. The syntax would be thus:

// example.cfm
param name="URL.number" default="";
param name="URL.colour" default="";

include "case.cfm"

result =
    case()
        .when(URL.number=="tahi")
            .then(function(){return "one"})
        .when(function(){return URL.colour=="whero"})
            .then(function(){return "red"})
        .else(function(){return "I dunno what to say"})
    .end()

echo(result)

This is obviously not as elegant as the Ruby code, but I can only play the hand I am dealt, so it needs to be in familiar CFML syntax.

Basically the construct is this:

case().when(cond).then(value).when(cond).then(value).else(value).end()

Where the condition can be either a boolean value or a function which returns one, and the value is represented as a function (so it's only actually called if it needs to be). And then when()/then() calls can be chained as much as one likes, with only the then() value for the first preceding when() condition that is true being processed. Clear? You probably already understood how the construct worked before I tried to explain it. Sorry.

Anyway, doing the design for this was greatly helped by using the BDD-flavoured unit tests that TestBox provides. I could just write out my rules, then then infill them with tests after that.

So I started with this lot (below). Just a note: this code is specifically aimed at Railo, because a few things I needed to do simply weren't possible with ColdFusion.

// TestCase.cfc
component extends="testbox.system.BaseSpec" {

    function run(){
        describe("Tests for case()", function(){
            describe("Tests for case() function", function(){
                it("compiles when called", function(){})
                it("returns when() function", function(){})
            });
            describe("Tests for when() function", function(){
                it("is a function", function(){})
                it("requires a condition argument", function(){})
                it("accepts a condition argument which is a function", function(){})
                it("accepts a condition argument which is a boolean", function(){})
                it("rejects a condition argument is neither a function nor a boolean", function(){})
                it("returns a struct containing a then() function", function(){})
                it("can be chained", function(){
                })
                it("correctly handles a function returning true as a condition", function(){})
                it("correctly handles a function returning false as a condition", function(){})
                it("correctly handles a boolean true as a condition", function(){})
                it("correctly handles a boolean false as a condition", function(){})
            })
            describe("Tests for then() function", function(){
                it("is a function", function(){})
                it("requires a value argument", function(){})
                it("requires a value argument which is a function", function(){})
                it("returns a struct containing when(), else() and end() functions", function(){})
                it("can be chained", function(){})
                it("executes the value", function(){})
                it("doesn't execute a subsequent value when the condition is already true", function(){})
                it("doesn't execute a false condition", function(){})
            })
            describe("Tests for else() function", function(){
                it("is a function", function(){})
                it("requires a value argument", function(){})
                it("requires a value argument which is a function", function(){})
                it("returns a struct containing an end() function", function(){})
                it("cannot be chained", function(){})
                it("executes when the condition is not already true", function(){})
                it("doesn't execute when the condition is already true", function(){})
            })
            describe("Tests for end() function", function(){
                it("is a function", function(){})
                it("returns the result", function(){})
                it("returns the result of an earlier true condition followed by false conditions", function(){})
                it("returns the result of the first true condition", function(){})
            })
        })
    }
}

TestBox is cool in that I can group the sets of tests with nested describe() calls. This doesn't impact how the tests are run - well as far as it impacts my intent, anyhow - it just makes for clearer visual output, and also helps me scan down to make sure I've covered all the necessary bases for the intended functionality.

I then chipped away at the functionality of each individual sub function, making sure they all worked as I went. I ended up with this test code:

Sunday 6 July 2014

Both ColdFusion and Railo make an incomplete job of validating email addresses

G'day:
A question came up on StackOverflow just now "new to coldfusion. is there any function that defines Email is typed correctly" (how hard, btw, is it to give a coherent title to one's question? [furrow]).

My initial reaction was along the lines of "RTFM, and if you had, you'd see isValid() validates email addresses". However I also know that isValid() is very poorly implemented (see a google of "site:blog.adamcameron.me isvalid" by way of my findings/rantings on the matter).

So I decided to check how reliable it actually is.

The rules for what constitutes a valid email address a quite simply, and a laid out very clearly in various RFCs. Or if one can't be bothered doing that (although the engineers & Adobe and Railo should be bothered), then they're fairly comprehensively defined even on Wikipedia: "Email_address (Syntax)". I found that page by googling for it.

Anyway, I won't bore you by replicating the rules here, but I wrote some TestBox tests to test all the rules, and how isValid() supports them. The answer to this on both ColdFusion (11) and Railo (4.2) is "only to a moderate level". This is kinda what I expect of the Adobe ColdFusion Team (this is the level of quality they seem to aim for, generally, it seems), but am pretty let down by the Railo guys here.

I won't reproduce all the code here as it's long and boring, but it's on GitHub: "TestIsValidEmail.cfc".

Anyhow, the bottom lines are as follows:

ColdFusion:


Railo:


Railo did slightly better, but ran substantially slower (running on same machine, same spec JVM).

I hasten to add that 50 of those tests were testing the validity of individual invalid characters, so that blows the failures out a bit (given they pretty much all failed). But both also failed on some fairly bog-standard stuff.

It's seriously as if it didn't occur to the engineers involved to actually implement the functionality as per the published specification; instead they simply made some shit up, based on what their own personal knowledge of what email addresses kinda look like. This is a bit slack, I'm afraid, chaps. And would just be so easy to do properly / thoroughly.

NB: there are rules around comments in email addresses that I only gave superficial coverage of, but I covered all the other rules at least superficially in these tests.



One new TestBox thing which I learned / noticed / twigged-to today... one can write test cases within loops. One cannot do that with MXUnit! eg:

describe("Tests for invalid ASCII characters", function(){
    for (var char in [" ", "(", ")", ",", ":", ";", "<", ">", "@", "[", "]"]){
        var testAddress = "example#char#example@example.com";
        it("rejects [#char#]: [#testAddress#]", function(){
            expect(
                isValid("email", testAddress)
            ).toBeFalse();
        });
        testAddress = '"example#char#example"@example.com';
        it("accepts [#char#] when quoted: [#testAddress#]", function(){ 
            expect(
                isValid("email", testAddress)
            ).toBeTrue();
        });
    }
});

Here I loop through a bunch of characters, and Here for each character perform two tests. This is possible because testBox uses function expressions for its tests, whereas MXUnit uses declared functions (which cannot be put in loops). This is not a slight on MXUnit at all, because it predates CFML's support for function expressions; it's more just that I'm so used to having to write individual test functions it did not occur to me that I could do this in TestBox until today. I guess there's nothing stopping one from writing one's test functions as function expressions in MXUnit either, that said. But this is the encouraged approach with TestBox.

Nice one, TestBox.

Anyway, this is not helping me prep for my presentation tomorrow. Give me a ballocking if you hear from me again today, OK?

--
Adam

Thursday 13 March 2014

Testbox: using a callback as a mocked method

G'day:
John Whish hit me up the other day, wondering had I looked at a new TestBox (well: at the time it was just for MockBox) feature I'd requested and has been implemented. Rather embarrassedly I had forgotten about it, and had not looked at it. That's a bit disrespectful of Luis and/or Brad (or whoever else put the hard work in to implement it), and for that I apologise. However I have looked at it now.

The feature request is this one, MOCKBOX-8:


Luis, we discussed this briefly @ SotR.

Currently one can specify a expression to use for a $results() value, and the value of that expression is then used as the results for the mocked method.

It would be occasionally handy for some stub code to be run to provide the results when a mocked method is called. This could be effected by being able to pass a callback to $results(), which is then called to provide the results when the mocked method itself is called.

Ideally a mocked method should always be able to be fairly dumb, but sometimes in an imperfect world where the mocked method has other dependencies which cannot be mocked out, an actual usable result is necessary. We've needed to do this about... hmmm... 0.5% of the times in out unit tests I think, so this is quite edge-case-y.

Cheers.
I can't recall what exactly we were needing to do at the time, but basically we were testing one method, and that called a different method which we wanted to mock-out. However we still wanted the mocked method to produce "real" results based on its inputs (for some reason).

MockBox allows to mock a method like this:

someObject.$("methodToMock").$results(0,1,2,3,4,etc);

And each call to methodToMock() will return 0, 1, 2 [etc] for each consecutive call, for as many arguments one cares to pass to the $results() method. Once it's used all the passed-in options it cycles back to the beginning again. This is great, but didn't work for us. What we needed was for the methodToMock() to actually run a stub function when it's called. Hence the enhancement request.

The Ortus guys have implemented this, so here's an example of it in action.

Firstly a MXUnit-compat scenario, running on ColdFusion 9 (so no need for function expressions or "closures" as the TestBox docs tends to refer to them as):

// C.cfc
component {

    public struct function getStructWithUniqueKeys(required numeric iterations){
        var struct = {};
        for (var i=1; i <= iterations; i++){
            struct[getId()] = true;
        }
        return struct;

    }

    private string function getId(){
        // some convoluted way of generating a key which we don't want to test
        return ""; // doesn't matter what it is, we won't be using it
    }

}

Here we have a CFC which has a method we want to test, getStructWithUniqueKeys(). It calls getId() to get a unique ID. For whatever reason we don't want our calls to getStructWithUniqueKeys() to call the real getId() (maybe it requires a DB connection or something?), so we want to mock-out getId(). However we still need it to return unique IDs.

Here's our test CFC:


// TestMxUnitCompat.cfc
component extends="mxunit.framework.TestCase" {

    function beforeTests(){
        variables.testInstance = new C();
        new testbox.system.testing.MockBox().prepareMock(testInstance);
        testInstance.$("getId").$callback(mockedGetId);
    }

    function testGetStructWithUniqueKeys(){
        var iterations    = 10;
        var result        = testInstance.getStructWithUniqueKeys(iterations=iterations);
        assertEquals(iterations, structCount(result), "Incorrect number of struct keys created, implying non-unique results were returned from mock");
    }

    private function mockedGetId(){
        return createUuid();
    }

}

Here I'm using Mockbox to prepare my test object for mocking (which just injects a bunch of MockBox helper methods into it), and then we mock getId() so that instead of calling the real getId() method, we use mockedGetId() as a proxy for it. And mockedGetId() just uses createUuid() to return a unique key.

And, pleasingly, this all works fine:

Wednesday 12 March 2014

Testbox: investigating which method/function/callback gets called when

G'day:
I had a bit of a wrestle with working out a glitch in my TestBox code y/day, and as a result ended up with a test CFC which shows where and when everything seems to run. I'll reproduce it here in case it saves anyone some time.


// TestExecutionSequence.cfc
component extends="testbox.system.testing.BaseSpec" {

    function beforeAll(){
        writeOutput("in beforeAll()<br>");
    }
    function afterAll(){
        writeOutput("in afterAll()<br>");
    }

    function run(){
        writeOutput("top of run()<br>");

        describe("outer describe", function(){
            writeOutput("top of outer describe()'s callback<br>");

            beforeEach(function(){
                writeOutput("in outer describe()'s beforeEach()<br>");
            });

            afterEach(function(){
                writeOutput("in outer describe()'s afterEach()<br>");
            });

            describe("inner describe", function(){
                writeOutput("top of inner describe()'s callback<br>");

                beforeEach(function(){
                    writeOutput("in inner describe()'s beforeEach()<br>");
                });

                afterEach(function(){
                    writeOutput("in inner describe()'s afterEach()<br>");
                });

                it("tests the negative", function(){
                    writeOutput("top of inner describe()'s first it()'s callback<br>");
                    expect(sgn(-1)).toBeLT(0);
                    writeOutput("bottom of inner describe()'s first it()'s callback<br>");
                });

                writeOutput("bottom of inner describe()'s callback<br>");
            });

            it("tests the truth", function(){
                writeOutput("top of first it()'s callback<br>");
                expect(true).toBeTrue();
                writeOutput("bottom of first it()'s callback<br>");
            });

            it("tests falsehood", function(){
                writeOutput("top of second it()'s callback<br>");
                expect(false).notToBeTrue();
                writeOutput("bottom of second it()'s callback<br>");
            });

            writeOutput("bottom of outer describe()'s callback<br>");
        });


        describe("second outer describe", function(){
            writeOutput("top of second outer describe()'s callback<br>");

            it("tests the absolute", function(){
                writeOutput("top of second outer describe()'s first it()'s callback<br>");
                expect(abs(-1)).toBe(1);
                writeOutput("bottom of second outer describe()'s first it()'s callback<br>");
            });

            writeOutput("bottom of second outer describe()'s callback<br>");
        });

        writeOutput("bottom of run()<br>");
    }

}

And here's the output (ignoring the test results, which are immaterial here):

top of run()
   top of outer describe()'s callback
     top of inner describe()'s callback
     bottom of inner describe()'s callback
   bottom of outer describe()'s callback
   top of second outer describe()'s callback
   bottom of second outer describe()'s callback
bottom of run()
in beforeAll()
    in outer describe()'s beforeEach()
        top of first it()'s callback
   
   
bottom of first it()'s callback

    in outer describe()'s afterEach()
    in outer describe()'s beforeEach()
        top of second it()'s callback
   
   
bottom of second it()'s callback
    in outer describe()'s afterEach()
        in inner describe()'s beforeEach()
        in outer describe()'s beforeEach()
            top of inner describe()'s first it()'s callback
   
        bottom of inner describe()'s first it()'s callback
        in inner describe()'s afterEach()
    in outer describe()'s afterEach()
    top of second outer describe()'s first it()'s callback
    bottom of second outer describe()'s first it()'s callback
in afterAll()


(note the indentation is not part of the original output, I just added it to try to add clarity to what's running when. I'm not sure I successed).

There's a few things to note here.

beforeAll()

I expected this to run... before everything. Before anything at all. Not just before the first test (ie: a call to the first it()'s callback). Where it runs is convenient, but I think there should be perhaps a beforeAll() and a beforeTests(), and beforeAll() should truly run before anything else.

Same with afterAll().

beforeAll() and beforeEach()

It seems slightly odd to me that beforeAll() is just a function, whereas beforeEach() is a call to a function which takes a function expression (which is then executed). I certainly didn't guess that, and had to refer to the docs.

Nested describe()

It quite cool how one can nest describe() calls, for subcategorising test output. This makes for a good visual cue when reading the results:


Anyway. That's it. Just a FYI, really.

--
Adam