Functional Programming Unit Testing in Node – Part 4

Concurrency, Compose, and Coverage

Welcome to Part 4 where we show how to do concurrency which is a lot easier to get “for free” using pure functions, we compose both async and synchronous functions, we and utilize a test coverage report to where next to focus our refactoring and testing efforts.

Contents

This is a 6 part series on refactoring imperative code in Node to a functional programming style with unit tests. You are currently on Part 4.

Compose Again

Assuming getUserEmail succeeded, we’ll have the user’s information so we can send an email. We now need to read the text email template to inject that information into. We’ll compose that readFile function we wrote. The existing code is imperatively inside the getUserEmail‘s then:

...
userModule.getUserEmail(get('cookie.sessionID', req))
    .then(value => {
        fs.readFile('./templates/email.html', 'utf-8', (err, template) => {
            if (err) {
                console.log(err)
                    err.message = 'Cannot read email template'
                    err.httpStatusCode = 500
                    return next(err)
            }
...

Let’s fix that and compose them together #connectDemTrainTrax:

...
userModule.getUserEmail(get('cookie.sessionID', req))
.then(userInfo => readEmailTemplate(fs)))
.then(template => ...)
...

Great! We even removed all the error handling as that’s built into our pure readEmailTemplate function.

Parallelism, Not Concurrency (Who Cares)

However, that’s one new problem; we need userInfo later on once we’ve gotten all the email info setup and ready. Since it’s only in scope for this function it’s now gone. One of JavaScript’s most powerful and taken for granted features, closures, we just threw out the window to remain “pure” for purity’s sake.

We can fix it, though, with one of JavaScript’s other features: non-blocking I/O. We can return 3 Promises and wait for all 3 to complete, and use all 3 values in the same function. It doesn’t matter if one takes longer than the others; Promise.all will wait for all 3 to be done, then give us an Array with all 3 values in order. If even 1 has an error, it’ll just pop out the .catch. This has 2 bad problems, but we’ll tackle that in another article. This also has the benefit of being faster in that we don’t have to wait for each in line, they all happen “at the same time Node style” which is not the same as “happening at the same time Elixir/Erlang or Go style” but that’s ok, we can get into the same dance club.

For now, we’ll refactor to:

...
Promise.all([
      getUserEmail(get('cookie.sessionID', req)),
      readEmailTemplate(readFile),
      mapFilesToAttachments(filterCleanFiles(get('files', req)))
    ])
    .then( ([userEmailAddress, emailTemplate, fileAttachments]) => ...)
...

Now we’re talking. Loading from an external web service, reading from a local file, and a synchronous function call all can happen “at the same time”, and we don’t have to worry about how long each one takes. We use Array Destructuring to get our arguments out. Note they come in the same order we put the functions into the Promise.all.

We now have our user’s email address, the text template to inject information into, along with the file attachments used for both in the same function scope.

Synchronous Compose

One thing to nitpick. Sometimes you refactor FP code for readability purposes, not just for the mathematical purity reasons. In this case, check out the 3 levels of nesting:

mapFilesToAttachments(filterCleanFiles(get('files', req)))

In imperative code, if you see if/then statements nested more than 2 levels deep, that tends raise concern. Developers are sometimes fine with creating that code to ensure they truly understand the different cases in playing with ideas, but once complete, they don’t like LEAVING it that way. Nested if statements are hard to read and follow. If you DO follow them, you can sometimes get a rush or high in “figuring it out”. That’s not the goal, though; nested if’s are considered bad practice.

For FP, deeply nested functions like this have the same problem. It’s compounded by the fact we attempted to use verbose names for the functions to make what they do more clear vs. short names. This ends up making the problem worse.

For Promises, it’s not so bad; you just shove them in the .then. But what about synchronous code?

You have 2 options:

  1. Simply wrap in them in a Promise; most promises except for a couple of edge cases are fine getting a return value of a Promise or a value as long as the value isn’t an Error.
  2. Use Lodash’ flow function, or Ramda’s compose.
  3. Use the pipeline operator.

Sadly, at the time of this writing, the pipeline operator is only at Stage 1 for JavaScript, meaning it’s not even considered a possibility for inclusion in the ECMA Standard yet. None of this code is asynchronous so we’ll use the Lodash flow (I like Ramda’s compose name better).

Let’s put the functions in order, just like we would with a Promise chain:

const filterCleanFilesAndMapToAttachments = flow([
  get('files'),
  filterCleanFiles,
  mapFilesToAttachments
])

Note the use of get('files'). The get function takes 2 arguments, but we only supply 1. We know it’s curried by default, meaning it’ll be a partial application if we just say get('files'); it’s waiting for the 2nd argument. Once it gets that, it’ll search for the ‘files’ property on it, else give undefined. If it DOES find undefined, filterCleanFiles will just spit out an empty Array, and mapFilesToAttachments will spit out an empty Array when you give it an empty Array. Otherwise, they’ll get the good Array full of files, and both of those functions will do their thang.

See how we use curried functions that create partial applications to help compose other functions? I know… for you guys, not a good pickup line, but you never know, she me might be a Data Scientist who digs Scala. Or she’s lit and you look good and anything you say doesn’t really matter at that point. Either way, it’s alllll good.

Now to use that composed function, we take what we had:

Promise.all([
  getUserEmail(get('cookie.sessionID', req)),
  readEmailTemplate(readFile),
  mapFilesToAttachments(filterCleanFiles(get('files', req)))
])

And replace it with our composed function:

Promise.all([
  getUserEmail(get('cookie.sessionID', req)),
  readEmailTemplate(readFile),
  filterCleanFilesAndMapToAttachments(req)
])

Much better eh? Speaking of lit, I’m feeling that hard root beer right now, but I STILL remember we need to unit test our composed function. Let’s do that… and with confidence because we already have 3 unit tested pure functions, and we composed them together with a Lodash pure function. DAT CONFIDENCE BUILDING! Also, you MAY have to install config and nodemailer: npm i config nodemailer and then require them up top. Also, depending on the order of functions, you may have to move some functions around giving while we’re creating pure functions, they’re defined IN an imperative way, and so order matters. i.e. you have to create the const app = express() first before you can app.post.

describe('filterCleanFilesAndMapToAttachments when called', ()=> {
    it('should give an attachment from a request with clean file', ()=>{
        const reqStub = {files: [{scan: 'clean', originalname: 'so fresh', path: '/o/m/g'}]}
        const result = filterCleanFilesAndMapToAttachments(reqStub)
        expect(result[0].filename).to.equal('so fresh')
    })
    it('should give an empty Array with no files', ()=>{
        const reqStub = {files: [{scan: 'dirty south', originalname: 'so fresh', path: '/o/m/g'}]}
        const result = filterCleanFilesAndMapToAttachments(reqStub)
        expect(result).to.be.empty
    })
    it('should give an empty Array with undefined', ()=>{
        const result = filterCleanFilesAndMapToAttachments(undefined)
        expect(result).to.be.empty
    })
})

:: chink chunk :: NIIIIICCCE!

Composing the Promise Way

You can also just compose the Promise way, and they’ll work for Promise based functions as well as synchronous ones allowing you to use interchangeably. Let’s first delete all the no-longer-needed imperative code:

let attachments = []
files.map(file => {
  if (file.scan === 'clean') {
    attachments.push({ filename: file.originalname, path: file.path })
  }
})

value.attachments = attachments
req.attachments = attachments

And we’ll take the remaining mix of synchronous and imperative code, and one by one wire together:

let emailBody = Mustache.render(template, value)
let emailService = config.get('emailService')
const transporter = nodemailer.createTransport({
  host: emailService.host,
  port: emailService.port,
  secure: false,
})

const mailOptions = {
  from: emailService.from,
  to: emailService.to,
  subject: emailService.subject,
  html: emailBody,
  attachments: attachments,
}

transporter.sendMail(mailOptions, (err, info) => {
  if (err) {
    err.message = 'Email service unavailable'
    err.httpStatusCode = 500
    return next(err)
  } else {
    return next()
  }
})

You hopefully are getting trained at this point to start noticing “globals in my function”. Note our current line of code is:

let emailBody = Mustache.render(template, value)

But nowhere in the function arguments do we pass the render function to use. Let’s quickly modify the ever-growing Express route function signature from:

const sendEmail = curry((readFile, config, createTransport, getUserEmail, req, res, next) =>

to:

const sendEmail = curry((readFile, config, createTransport, getUserEmail, render, req, res, next) =>

We’re already in a Promise at this point, so we can return a value here, or a Promise and we’ll be sure we can add another .then if we need to. One trick VSCode, a free text and code editor by Microsoft, has is highlighting variables. Before we shove this rendered email template variable in the Monad train, let’s see if anyone down the tracks needs it. We’ll select the whole variable, and watch how VSCode will highlight usage of it as well:

Crud… it’s a ways down, AND it’s mixed in with this emailService thing. Let’s highlight him and see where he’s grouped:

This’ll be tricky. Good news, rendering the email and loading the email service configuration can be done at the same time. Let’s keep that INSIDE the Promise now until we feel comfortable we no longer need the userEmailAddress, emailTemplate, fileAttachments in scope. A lot more pragmatic people would be fine with keeping the code this way, and using JavaScript’s built in feature of closures, and move on with life. However, imperative code is harder to test, and results in LONGER code vs. smaller, pure functions that are easier to test. You don’t always START there, though. It’s fine to write imperative, then write “kind of pure” and keep refactoring your way there. That’s part of learning, figuring our the idea of how your code should work, or both.

...
.then( ([userEmailAddress, emailTemplate, fileAttachments]) => {
    return Promise.all([
      render(render, template, userEmailAddress),
      getEmailService(config)
    ])
    .then( ([emailBody, emailService]) => ...
...

And we’ll clean up the code below to use our pure functions first imperatively:

...
.then( ([emailBody, emailService]) => {
  const transportObject = createTransportObject(emailService.host, emailBody.port)
  const transport = createTransport(transportObject)
  const sendEmailFunction = transport.sendEmail
  const mailOptions = createMailOptions(
    emailService.from,
    emailService.to,
    emailService.subject,
    emailBody,
    fileAttachments
  )
  return sendEmailSafe(sendEmailFunction, mailOptions)
})

… and then refactor to more functional:

...
.then( ([emailBody, emailService]) =>
  sendEmailSafe(
    createTransport(
      createTransportObject(emailService.host, emailBody.port)
    ).sendEmail,
    createMailOptions(
      emailService.from,
      emailService.to,
      emailService.subject,
      emailBody,
      fileAttachments
    )
  )
})
...

Note the fileAttachments comes from the scope higher up. The sendEmailSafe function requires a nodemailer transport. We create that from our function that creates the Object from the emailService. Once created we need that sendEmail function to pass it to the sendEmailSafe so we just immediately go .sendEmail in the first parameter. The createMailOptions is another function that simply creates our Object from the emailService object, the rendered via Mustache emailBody, and the virus scanned fileAttachements. One last touch is to remove the squiggly braces {} as we’re no longer writing imperative code, and the return statement as Arrow functions have an implicit return when you remove the squiggly braces.

This last part is left over from the callback:

), reason => {
      return next(reason)
    })

Typically you defer Promise error handling higher up the call stack; meaning, “let whoever is calling me deal with error handling since Promises that call Promises have their errors propagate up”. That’s fine, so… we’ll delete it.

After all that refactoring, here’s what we’re left with:

const sendEmail = curry((readFile, config, createTransport, getUserEmail, render, req, res, next) =>
    Promise.all([
      getUserEmail(get('cookie.sessionID', req)),
      readEmailTemplate(readFile),
      filterCleanFilesAndMapToAttachments(req)
    ])
    .then( ([userEmailAddress, emailTemplate, fileAttachments]) => 
      Promise.all([
          render(render, template, userEmailAddress),
          getEmailService(config)
        ])
        .then( ([emailBody, emailService]) =>
          sendEmailSafe(
            createTransport(
              createTransportObject(emailService.host, emailBody.port)
            ).sendEmail,
            createMailOptions(
              emailService.from,
              emailService.to,
              emailService.subject,
              emailBody,
              fileAttachments
            )
          )
        )
    ))

Coverage Report

Let’s unit test it; it’ll be hard because we have a lot of stubs, but we can borrow from the ones we’ve already created in the other tests. I’m not going to DRY the code at all in the tests as that would require too much brainpower at this point, but when you get an Agile Sprint or time to pay down technical debt, this is one of the stories/tasks you add to that list.

… before we do, let’s run a coverage report to see how much work we have cut out for us (we’re ignoring my fake npm module and the user stuff for now). Run npm run coverage && open coverage/lcov-report/index.html:

And the details around our particular function:

Status Quo at This Point

Wonderful; the only thing we need to test is the composition of those functions in Promise.all. Rather than create 20 billion stubs, and ensure they’re setup “just so” so the sendEmail unit test passes or fails, we’ll continue to our strategy of pulling out teency pieces, wrapping them in pure functions, testing those, repeat. Let’s start with the first Promise.all:

const getSessionIDFromRequest = get('cookie.sessionID')
const getEmailTemplateAndAttachments = curry((getUserEmail, readFile, req) =>
  Promise.all([
    getUserEmail(getSessionIDFromRequest(req)),
    readEmailTemplate(readFile),
    filterCleanFilesAndMapToAttachments(req)
  ]))

Then we’ll unit test the getEmailTemplateAndAttachments (he’ll end up ensuring we’ve tested the new getSessionIDFromRequest):

describe('getEmailTemplateAndAttachments when called', ()=> {
    const reqStub = {
        cookie: { sessionID: '1' },
        files: [{scan: 'clean', originalname: 'so fresh', path: '/o/m/g'}]
    }
    const getUserEmailStub = () => 'email'
    const readFileStub = (path, encoding, callback) => callback(undefined, 'email')
    const readFileStubBad = (path, encoding, callback) => callback(new Error('b00mz'))
    it('should succeed with good stubs', ()=> {
        return expect(
            getEmailTemplateAndAttachments(getUserEmailStub, readFileStub, reqStub)
        ).to.be.fulfilled
    })
    it('should succeed resolve to having an email', ()=> {
        return getEmailTemplateAndAttachments(getUserEmailStub, readFileStub, reqStub)
        .then( ([userEmail, emailBody, attachments]) => {
            expect(userEmail).to.equal('email')
        })
    })
    it('should fail if reading file fails', ()=> {
        return expect(
            getEmailTemplateAndAttachments(getUserEmailStub, readFileStubBad, reqStub)
        ).to.be.rejected
    })
})

And we’ll then swap it out for the raw Promise.all:

const sendEmail = curry((readFile, config, createTransport, getUserEmail, render, req, res, next) =>
    getEmailTemplateAndAttachments(getUserEmail, readFile, req)
    .then( ([userEmailAddress, emailTemplate, fileAttachments]) => 
      Promise.all([
          render(render, template, userEmailAddress),
          getEmailService(config)
        ])
        .then( ([emailBody, emailService]) =>
          sendEmailSafe(
            createTransport(
              createTransportObject(emailService.host, emailBody.port)
            ).sendEmail,
            createMailOptions(
              emailService.from,
              emailService.to,
              emailService.subject,
              emailBody,
              fileAttachments
            )
          )
        )
    ))

… and then re-run coverage. Just run npm run coverage and you can refresh the coverage in the browser:

As you can see, coverage isn’t going to let us off that easy. That’s ok, we can re-use these stubs for the final battle. Let’s do the last Promise.all.

describe('renderEmailAndGetEmailService when called', ()=> {
    const configStub = { has: stubTrue, get: () => 'email service' }
    const renderStub = stubTrue
    const renderStubBad = () => { throw new Error('intentionally failed render')}
    it('should work with good stubs', ()=> {
        return expect(
            renderEmailAndGetEmailService(configStub, renderStub, 'template', 'user email')
        ).to.be.fulfilled
    })
    it('should resolve to an email', ()=> {
        return renderEmailAndGetEmailService(configStub, renderStub, 'template', 'user email')
        .then( ([emailRendered, emailService]) => {
            expect(emailRendered).to.equal(true)
        })
    })
    it('should fail if rendering fails', ()=> {
        return expect(
            renderEmailAndGetEmailService(configStub, renderStubBad, 'template', 'user email')
        ).to.be.rejected
    })
})

And swap it out:

const sendEmail = curry((readFile, config, createTransport, getUserEmail, render, req, res, next) =>
    getEmailTemplateAndAttachments(getUserEmail, readFile, req)
    .then( ([userEmailAddress, emailTemplate, fileAttachments]) => 
      renderEmailAndGetEmailService(config, render, emailTemplate, userEmailAddress)
      .then( ([emailBody, emailService]) =>
        sendEmailSafe(
          createTransport(
            createTransportObject(emailService.host, emailBody.port)
          ).sendEmail,
          createMailOptions(
            emailService.from,
            emailService.to,
            emailService.subject,
            emailBody,
            fileAttachments
          )
        )
      )
    ))

4 Replies to “Functional Programming Unit Testing in Node – Part 4”

  1. Hey

    Unfortunately your code is broken. Function getEmailService is returning Maybe and you are not handling that Maybe anywhere. In your sendEmail function you treat emailService as a regular object but it will be either Just or Nothing. I was expecting this to be uncovered somehow… maybe with integration tests but in your integration test you simply copy’n paste (with minor modifications) sendEmailSafe from server.js to sandbox.js so you are testing only sending functionality not full end2end behaviour (which is actually not bad albeit it doesn’t allow one to uncover the bug).
    One solution would be to simply spy createTransport function to check if it receives a valid parameter (an object with host, port and and secure:false)

    My biggest concern regarding your approach is that it is too much focused on particular functions (units) not on behaviour. It is a matter of TDD vs BDD

  2. You’re right, thanks for the find! Nice catch. I’ll fix the post.

    Your mind is in the RIGHT place, this is exactly the weakness unit tests have whether they use FP programming with stubs, or non-FP with mocks and stubs: if the stub is wrong, and the test passes, it’s a false positive despite 101% test coverage. This is exactly the reason for integration tests.

    I’d caution against spy, though, as he’s just a mock that can act as a stub; i.e. he is explicitly made for testing side effects. That’s the point of this whole series; to avoid mocks, to avoid creating side effects.

    “…too much focused on particular functions (units) not on behaviour”

    YES! Absolutely. You want to test units first. If those fail, you’re behavior will fail. Units are faster to run with stubs that have no time, no side effects like network calls, reading disk, etc. AND can be run concurrently. Once those pass, then you run the property tests, like jsverify, which are longer, random, and tackle every avenue of positive/negative tests around data types. Finally, you do the integration tests to ensure your actual interfaces are correct. Now, some of those can be mocked (supertest, nock, etc), but even if your local server just returns stubbed data, you can guarantee your contracts are correct. What you found is exactly what we want to find in integration tests… they are just longer and probably won’t work if your unit tests fail.

    Unit (like these posts show), property testing, integration testing, and if you have UI stuff, Functional Testing. … I only focused on unit to show people how they don’t need mocks.

    …. now, do we HAVE to use integration testing to surface these issues (beyond hiring you full time to code with me, heh)?

    No, there are 2 other options:
    1. Strong Typing
    2. Total Functions

    Types would of caught that error before I even attempted to run the tests, in a second whether Flow or TypeScript . This is another cost of dynamic languages: propertyThatExists.somethingThatDoesnt === undefined, no error.

    The second option is to use total functions. If we did, createMailOptions would returned an Error or rejected the Promise after validating the from was undefined.

  3. Hey

    I am waiting for the update of this post. Regarding your comment: I fully support having tests like unit, property, integration, integrated (e2e) tests. Unfortunately these terms are very often understood in a different ways by different people…
    My biggest concern regarding what you showed in this series is focusing on building a set of tests for EACH individual function.
    I prefer to employ hexagonal architecture (ports and adapters) and build a set of unit tests for given “functionality” (input port functionality) instead of tests each “internal function” individually.
    In this case i would only create a set of unit tests (with stubs and spies as test doubles for external services like reading template, getUserEmail, creating transport ) for sendEmail function (having the same signature as the one you have). It would allow me to cover all scenarios (based on different input/different stubs) of this function by asserting sendEmail side effects (if we have any ) and its return value. I will not test any of internal functions like : getEmailTemplateAndAttachments, renderEmailAndGetEmailService, filterCleanFilesAndMapToAttachments, createMailOptions and more. At the current stage these are not my “units of reusability” – they are only “helpers” (albeit very useful) for building sendEmail function. All that allows me to easily refactor sendEmail – as long as all tests for sendEmail are passing i can ruthlessly refactor it (yes i will drive it to extract as many pure functions as possible).

    After i am done with these unit tests, i would build some integration tests for code communicating with outside world (can use nock or real implementations) and at the end i would build a single e2e test which would execute sendEmail function with real adapters passed.

  4. …focusing on building a set of tests for EACH individual function…

    Yep, agreed. If you follow Kent Beck’s TDD ideas, you should only be testing the feature/public API, like you said sendEmail. If you setup a few good stubs for that, you’d get 100% coverage as well and could move onto the other tests. Then if you changed the implementation, you wouldn’t have to change all the tests I wrote for the individual units.

    I’m not covering TDD, though, just why pure functions are cool: only need stubs, easy for you to test your ideas, and you have massive confidence when you wire them together. Nothing wrong, everything right with deleting these tests once your public interface is legit. Not a panacea, though, for sure.

Comments are closed.