FP Unit Testing in Node - Part 4: Concurrency, Compose, and Coverage

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.

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 webservice, 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 possiblity 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?

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 rootbeer 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 interchangly. 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 pragmmatic 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 immeidately 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 propogate 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 tincy 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
          )
        )
      )
    ))