Too many assumptions about my code. Let me just show it.
Vasyl Boroviak
111

That code looks exactly like I expected, and has exactly the qualities I assumed it would. There is only one small logic branch that needs its own unit tests, because testing the rest of the units would mean unit testing your data fetching API (which I assume is already handled, elsewhere).

The rest of this code needs to be integration tested.

Only part of this function includes non-integration logic and does in fact need unit testing, and unfortunately, it’s tightly coupled to your async APIs, so you can’t unit test it without mocks:

if (await this.haveWriteAccess({dbUser, folderInfo})) {
return await this.uploadToFolder({dbUser, folderInfo, file});
} else {
throw boom.forbidden("No write access to that folder");
}

If haveWriteAcess() can be a pure, synchronous function, you should make it one, and unit test it separately. It may be more convenient for composition to keep it a promise — in which case, it has error branching built-in and you don’t need to branch here. Just reject in haveWriteAccess(), and promote uploadToFolder() to the main pipeline.

The rest of this function is purely integration, and is a poster-child case for a function that is best tested with integration and/or functional testing instead of unit tests.

Mocking exists for the purpose of isolating different units of code and testing them when they’re disconnected. So think carefully about this before you answer:

What’s the point of mocking when you’re specifically testing the integration of several pieces?

Incidentally, if each data-sharing function here could take and return the same object type, like a continuation: {user, folder, file, whatever} this entire function could be written with one asyncPipe():

const asyncPipe = (...fns) => x => (
fns.reduce(async (y, f) => f(await y), x)
);
const uploadFile = asyncPipe(
readUser,
getFolderInfo,
haveWriteAccess,
uploadToFolder,
);
uploadFile({ user, folder, file })
.catch(err => handleError)
;

I’m not saying this is the right approach for your app, but this illustration may clarify which bits need their own unit tests (each argument to asyncPipe()), and that you need some integration tests covering the complete flow.

Now, let’s hit each of your points of concern:

1. It’s backend. There is no Redux.

Redux is just one way to decouple your state logic. Promises are another valid approach.

2. It’s already promises composed together.

Great! You’re 90% there.

3. Nothing is tight coupled because everything is replaceable.

Degree of coupling is a gradient scale, not black & white. There’s room to improve here, as I hope you’ve seen.

4. This is only 9 lines of code. Single piece of logic. Breaking this apart will significantly reduce readability.

This bit is relative. I personally find the asyncPipe() version much more readable, but respect that familiarity bias may be an obstacle for some. See also “Composing Software: An Introduction” for advantages of the compositional style.

5. Integration tests are impossible because: (1) the uploadToFolder can cost up to 9 cents, (2) the getFolderInfo talks to a microservice which is impossible to make available on CI, (3) readUser talks to a Cassandra DB, which is hard to run on a CI.

As you well know, you can test integrations without testing actual API services. E.g., drop in some configuration with test URLs:

{config, user, folder, file} 
6. The functions readUser and getFolderInfo can throw, by design. So, the execution might never get to the haveWriteAccess.

Luckily, promises can branch for error cases, and you can branch a handler on error types. Problem solved.

Like what you read? Give Eric Elliott a round of applause.

From a quick cheer to a standing ovation, clap to show how much you enjoyed this story.