Generate a URL, shorten it, insert it in a tex file and compile those tex files

I have written a NodeJS module and its tests to generate a URL, shorten it with Bitly, insert the short link in a tex file, compile these tex files to PDF and then merge them into one.

The code is working but it looks confusing and the tests are too hard to follow in my opinion.

I'm not sure if I should mock fs to avoid writing to disk or just let the module write during the tests and cleanup after all the tests.

I'm wondering if I shouldn't have dummy tex files in my fixtures and use these instead of external tex files that aren't included in the repo.

Here is the code:

lib/Package.js

```
// A package represents what is sent in the mail. It contains a letter and a resume, and provides its name, short_url
// and long_url.
var crypto = require('crypto');
var fs = require('fs');
var async = require('async');
var helpers = require('./helpers');
var Bitly = require('bitly');
var path = require('path');
var exec = require('child_process').exec;
var changeCase = require('change-case');

module.exports = Package;

function Package(config) {
var self = this;
var defaults = {
config_files: {
config: 'config.json',
secrets: 'secrets.json'
}
};

self.recipient = config.recipient;
self.files = config.files;

self.config_files = config.config_files || defaults.config_files;
}

// The init method should be called just after Package is intantiated. It runs the asynchronous code needed to
// populate various Package's properties.
// This avoids putting the async code in the constructor, saving us from a messy instantiation
Package.prototype.init = function(done) {
var self = this;
async.auto({
populateName: function(next) {
self._initName(next);
},
populateConfig: function(next) {
self._initConfig(next);
},
populateLongUrl: ['populateConfig', function(next) {
self._initLongUrl();
next();
}],
populateShortUrl: ['populateLongUrl', function(next) {
self._initShortUrl(next);
}],

I spent a good time reading your code, only found minor stuff:

-
You have code similar to this a few times:

exec(command, options, function(err) {
  if (err) return next(err);
  return next(null);
});

since you never check for the actual 'nullness' of err, you could just write

return exec(command, options, next);

-
I understand the following code is self documenting, however I still feel it's a bit overkill:

Package.prototype._generateLongUrl = function() {
  var self = this;
  var protocol = 'https://';
  var aws_fqdn = '.s3.amazonaws.com/';
  var package_name = self.name;
  var archive_ext = '.tar.gz';

  return protocol + self.config.s3_bucket + aws_fqdn + package_name + archive_ext;
};

compared to

Package.prototype._generateLongUrl = function() {
  return `https://${self.config.s3_bucket}.s3.amazonaws.com/${self.name}.tar.gz`;
}

-
This might be a lack imagination on my part, but if (typeof(self.compiled_files) === 'undefined' && !self.compiled_files) if you check for undefined, then the second clause can be dropped? There is no way that self.compile_files can be true ? Peronally I would go for self.compiled_files = self.compiled_files || {};

-
I initially resolved not to mention it, but I cracked. Please use consistently lowerCamelCase everywhere. compiled_files -> compiledFiles

-
In _mergePdf, the following

var parsed_resume_path = path.parse(self.compiled_files.resume);
var resume_path = path.resolve(parsed_resume_path.dir + path.sep + parsed_resume_path.base);

could be

var resumePath = path.resolve(self.compiled_files.resume);

All in all, I think your variable are wordy, but then again your code is pretty easy to follow. I think your code will be even easier to follow with the first observation and in some cases I think you are trying too hard like in my last point.

StackExchange Code Review Q#106641, answer score: 4