5
\$\begingroup\$

I'd really like someone to sanity check my approach for unit testing the summarise() function and mocking its dependencies.

Background

  • Each option has a set of values, which come from the app state
  • Options also each have a config, which are defined in configs.js (potentially a large list, with 'dynamic' data-like variations)

The tests

I'm testing the summarise function. To isolate my tests, I've mocked the option configs. This means I don't have to couple the test to certain option configs, which allows settings to be changed freely.

My tests work, but I feel it has a few issues:

  1. I can't directly spy on a config object, so I've exported a function getOptionConfig() which I can spy on. I feel like it would be cleaner to avoid an API; and just spy on the object if possible.
  2. I feel it'd be much cleaner to pass the _optionConfigs object to summarise() (to avoid mocks completely). However, this would only pass the same issue to any functions which call summarise().
  3. Because I'm forcing a return value in the spies, I'm not testing the parameter that I'm passing to getOptionConfig(). Is this bad?

configs.js

const _optionConfigs = {
  exampleOption: {
    type: 'red',
  },
  exampleOption2: {
    type: 'blue',
  },
  exampleOption3: {
    type: 'red',
    exclude: true,
  },
  exampleOption4: {
    type: 'red',
  },
  // ... the list goes on...
};

export const getOptionConfig(id) {
  return _optionConfigs[id];
}

summarise.js

import { getOptionConfig } from './config';

/**
 * Summarise an option's values
 * @param {string} optionName - an option name, used for referencing its config data
 * @param {array} optionValues - application state. values of a particular option
 * @returns {string} a summary of the option's values
 */
export const summarise(optionName, optionValues) {
  const optionConfig = getOptionConfig(optionName);
  if (optionConfig.exclude) {
    return '';
  }
  if (optionConfig.type === 'red') {
    return optionValues.map(value => value + ' with a dash of red').join(', ');
  } else (optionConfig.type === 'blue') {
    return optionValues.map(value => value + ' with a bit of blue').join(', ');
  }
}

summarise.test.js

// ENV - jasmine

import * as configs from './configs';

describe('summarise', () => {
  it('ignores when excluded', () => {
    spyOn(configs, 'getOptionConfig').and.returnValue({
      exclude: true,
      type: 'blue',
    });
    const summary = summarise('testOption3');
    expect(summary).toBe('');
  });
  it('summarises blue types', () => {
    spyOn(configs, 'getOptionConfig').and.returnValue({
      type: 'blue',
    });
    const summary = summarise('testOption2', [
      'Value 1',
      'Value 2',
    ]);
    expect(summary).toBe('Value 1 with a bit of blue, Value 2 with a bit of blue');
  });
  it('summarises red types', () => {
    spyOn(configs, 'getOptionConfig').and.returnValue({
      type: 'red',
    });
    const summary = summarise('testOption', [
      'Value 1',
      'Value 2',
    ]);
    expect(summary).toBe('Value 1 with a dash of red, Value 2 with a dash of red');
  });
});
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I agree with point number 2, just passing in the config is better (i.e. dependency injection).

You could do it like this:

// Definition of summarize:
export const summarize(config, optionName, optionValues){
    // stuff
}

// In some other file:
const summarizeWithConfig = require('./summarize');
const config = require('./config');

// Create a new function which has it's first argument bound to the config
let summarize = summarizeWithConfig.bind(null, config);

Now summarize can be used like before, and in your tests you can simply test the unbound function.

This kind of pattern should be used in all your code, and will make it a lot easier to test. However, binding up functions like this becomes a little tedious (with the extra name "summarizeWithConfig", the ugly bind syntax, etc), so I would really recommend using classes instead, and have the constructors of those classes as injection points.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.