Baby’s First Pull Request

I have a function that has 100% test coverage. If I create a new function from that one using the promisify method from es6-promisify, and decide to write tests for the new function, what about the new function should I test? The new function has been modified to return a promise, but what about its logic has been changed?

This question bothered me. When I asked it I didn’t have an intuitive understanding of how promises worked, only a superficial and basic one.

So when I decided to contribute to Filer by writing a unit test for a method under it’s new promise-based api, I was a bit stumped. It would have been one thing to just test the output of the method, but I was concerned about the what and why of my test. Exactly what was I testing?

In Filer, its methods are turned into promises through:

FileSystem.prototype.promises[methodName] = promisify(FileSystem.prototype[methodName].bind(fs));

From what I could tell, if there was any new logic added to the promise methods themselves it would be done there. But on closer inspection I realized that the actual conversion is being handled by the promisify method, a method implemented in a dependency called es6-promisify.

I realized that I was technically testing the result of the promisify function and, as such, decided to take a look at the source code for it to see what changes may have been made to its input. It’s a surprisingly small project, and the code that was doing the converting was the following:

Here we are returning a new function that, when called, will return a promise. Unless supplied with a custom implementation of the Promise object, a default ES6 Promise will be returned.

This function was a bit weird to me, it did some things I hadn’t quite seen before. To better understand it I decided to use promisify in an existing program of mine, then printed to the console at various intervals of the function’s execution.

node commands/quicknote.js
the promisified function is running
the original function is about to be called
the callback function is running
the promise has been returned

From the print out, I was about to conclude that when a function created with promisify gets called:

1. All the arguments passed into the function are turned into an array and a callback function is created and appended to the list of arguments.
2. The original asynchronous function is then called with the new list of arguments with the appended callback.
3. When the function has finished executing it will call the callback function, which itself will resolve the promise it is nested inside.

When I decided to start contributing to filer, I created a test to see if a new Promise object was returned through one of the fsPromises methods after creating an issue for it. I decided not to add tests related to the existing fs method implementation for their respective promise-returning versions. I felt that it could be redundant since both the standard and promise version of the filer methods appear to be using the same logic in the implementation.js file. And, based off my investigation into promisify, the method used by filer to convert asynchronous methods into promise-returning ones doesn’t seem to be making changes to the logic either. I’m not 100% confident in that assertion though, since the context of the promise methods seems to be modified in interface.js, so perhaps something like this could bear an impact even if the logic itself isn’t changed.

My test was simple and the implementation for it straight forward. There were different ways I could have approached this test, and at first I thought it might take some work to create it (or at least to find the best way of doing so). However I discovered that chai already provided a means for testing against a promise object, and this alone made the task easy to complete.

I created my pull request. After a bit of time someone was kind enough to suggest a better name for one of my variables. I fully agreed with them since their suggestion made the naming more accurate/clear, and incorporated that fix.

While examining some of the existing pull requests I realized that another user made a mistake that I did and added changes to their package-lock.json file. When it came to my workflow I had the habit of using git add — all to add changes to staging, resulting in unwanted changes being added to my commits. This user likely did the same thing, and I decided to help them out by sharing with them my own solution to the problem.

This exercise took longer than I expected since I was already familiar with git. There were a lot of new concepts in the code base I needed to get used to, and even just adding a simple test required some reading and research on my end. But I had fun, and learned some useful coding techniques in the process. My chase down the proverbial rabbit hole made me realize how much could go into a single line of code, assuming it’s using something from an existing project. I always knew that utilizing existing code could greatly simplify things, but actually looking through through dependencies makes the reality of that concept so much more real.