Writing Tests - Stubbing composed selectors

[tomatau]

Currently we have some complex logic around state tested in one selector, all is good there:

const complexSelector = createSelector(
   someSimpleSelectors,
   (a,b,c) => // does complex logic and returns one of a few simple values
)

The tests for this are good, we have many permutations of state fed in, and a few simple returned values.

This complex selector is then re-used by other selectors!

const otherSelector = createSelector(
   selectors.complexSelector, selectors.anotherSelector,
   (complexResult, anotherValue) => // uses the simplified values from complex state
)

Instead of having to put all of the permutations of state into otherSelector (that we've already tested), we were hoping to stub out complexSelector for this test. This would allow us to mock the simple return values of complexSelector when testing otherSelector!

One problem here is that when we sinon.stub(selectors, 'complexSelector'), it seems to not be the same reference to the complexSelector function that is used by otherSelector!

We've tried wrapping all the selectors in an object and exporting that, but same deal... It's like createSelector is actually creating new references to the select functions you give it... making them un-stubbable (is that a word?)...

Our only solution to this so far has been to just resort to simple state select functions, like so:

const otherSelector = (state) => {
   const complexResult = selectors.complexSelector(state);
   const anotherValue = selectors.anotherSelector(state);
   // uses the simplified values from complex state
}

And then stubbing works fine.

This feels like going backwards though... is there any way to get the stubbing to work for selectors given to createSelector?

Maybe a PR to expose the dependendeded on selector functions when using createSelector, so that they can be stubbed?

[ellbee]

Isn't this going to be the case for any parameter that is a function that you try to stub out like this? E.g.

const sinon  = require('sinon')
const fns = {
  fn1: () => 100,
  fn2: () => 200
}

function testFn(fn1, fn2) {
  return () => fn1() + fn2()
}

const test = testFn(fns.fn1, fns.fn2)

sinon.stub(fns, 'fn1', () => 10000)
console.log('testFn', test()) // 300

Are you sure stubbing is the right approach in your scenario?

[tomatau]

Stubbing is most certainly the best way to avoid duplicating test logic for composed selectors here.

As I said, I'd be happy to PR to expose the arguments for stubbing:

const fns = {
  fn1: () => 100,
  fn2: () => 200,
}

function testFn(fn1, fn2) {
  const funk = () => funk.fn1() + funk.fn2()
  funk.fn1 = fn1
  funk.fn2 = fn2
  return funk
}

const test = testFn(fns.fn1, fns.fn2)

sinon.stub(test, 'fn1', () => 10000)
console.log('testFn', test()) // 10200

Do you have any suggestions?

[ellbee]

Sorry, I didn't explain myself at all well there.

What I meant was that I don't understand why stubbing would be better than passing otherSelector state that is known to return a certain value from complexSelector. E.g.

// test complexSelector which returns either 1, 2, 3, or 4

// save off a single instance of state that returns a particular value
const complexState1 = {a: 100, b: 200}
assert(complexSelector(complexState1)).toBe(1)
// don't need to bother saving again
assert(complexSelector({a: 300, b: 400})).toBe(1)
assert(complexSelector({a: 400, b: 500})).toBe(1)
assert(complexSelector({a: 400, b: 600})).toBe(1)

...

const complexState4 = {a: 1000, b: 2000}
assert(complexSelector(complexState4)).toBe(4)
assert(complexSelector({a: 3000, b: 4000})).toBe(4)
assert(complexSelector({a: 4000, b: 5000})).toBe(4)
assert(complexSelector({a: 4000, b: 6000})).toBe(4)

 // equivalent of stubbing complexSelector with return value 1
assert(otherSelector({ ...complexState1, {c: 999})).toBe(100)
 // equivalent of stubbing complexSelector with return value 2
assert(otherSelector({ ...complexState2, {c: 999})).toBe(200)
 // equivalent of stubbing complexSelector with return value 3
assert(otherSelector({ ...complexState3, {c: 999})).toBe(300)
 // equivalent of stubbing complexSelector with return value 4
assert(otherSelector({ ...complexState4, {c: 999})).toBe(400)

[tomatau]

Thanks for expanding on your suggestion, I understand this solution, but to me, this has 2 problems:

• unclear when reading the tests back.
• focusing on only a few select permutations of the state in order to confine duplication to a limited set.

Stubbing is really nice solution here -- by taking advantage of composition, the state permutations and test logic can be reduced to one place. This would make them both easier to maintain and to read.

[ellbee]

Sorry, but I am going to close this out as I'm not convinced of the value of the suggested change. I guess this is somewhat subjective but I actually disagree with each of your points (I think testing with the actual state is clearer, I don't see any duplication and I don't think stubbing is nice here).

[tomatau]

No probs, happy to explain how it is duplication if you like ;)

[stevenmusumeche]

Just to chime in here, I agree with @tomatau and would love to be able to stub the composed selectors so that I could isolate the behavior of each selector without depending on the behavior of one of the "source" selectors.

[notmlee]

I'm surprised this was shut down. It seems like the recommendation here is that if you want to test a selector composed of other composed selectors (e.g. deeply composed selectors?), you have to provide the requisite state for each of the dependent composed selectors.

This seems like an extremely onerous way to test deeply composed selectors. In the worst case, we'll basically be expected to mock the entirety of state to test one selector, including the variations of state necessary to change the output of one of the underlying selectors just so we can test the interaction of a composed selector with the rest of the application.

Edit: I fully retract this. Like it always is, the issue was elsewhere. :feelsbadman:

[stevenmusumeche]

@notmlee I'm still unable to stub selectors that feed into a composed selector. What was your solution?

[notmlee]

I have not yet been required to do that, so I don't have a good answer for you @stevenmusumeche.

The general strategy from our short use of reselect:

• If you need to test the I/O of a composed selector, use .resultFunc and provide the mock inputs directly rather than stubbing the individual selectors.
• If you need to test the usage of a composed selector, stub the composed selector directly. This won't work depending on how your modules are organized (i.e. if both the consumer and composed selector are initialized in the same module/file due to how createSelector works internally as Ellbee described earlier. This was the issue I was facing in my previous post)

I'm sure there's a scenario where one may need to stub the input selectors, but so far we've been spared.

[rickhanlonii]

It sounds like what you really want to do is run the last arugment of createSelector (called the result function) with values you expect complexSelector to return, and check the result function logic.

You can do that using otherSelector.resultFunc()

So instead of

sinon.stub(selectors, 'complexSelector', () => 'mockValue1');
sinon.stub(selectors, 'anotherSelector', () => 'mockValue2');

const result = otherSelector({foo: 'bar'});
assert(result).toBe('expectedResult');

you can do

const result = otherSelector.resultFunc('mockValue1', 'mockValue2');
assert(result).toBe('expectedResult')

This allows you to pass values directly to the last argument of createSelector, no stubbing necessary

[jimbol]

I think the missing piece here is that it matters which dependencies a selector uses. For example, if we have two selectors that use the same resultFunc...

const lookUp = (hash, ids) => ids.map((id) => hash[id]);

const getMyValues = createSelector(
  getMyHash,
  getMyIds,
  lookUp,
);

const getYourValues = createSelector(
  getYourHash,
  getYourIds,
  lookUp,
);

...testing the resultFunc isn't enough. We actually need to test that the first two functions are what we expect them to be. We can do this with an integration test (passing in the entire state into the whole selector) but, it forces us to essentially re-test getMyHash, getMyIds.

Desired API

We've been running into this problem. The direction I'm leaning is to expose the dependencies so that we can test them with a simple equality check.

it('uses the dependencies we expect', () => {
  // We can expose `args`
  const dependencies = getMyValues.args;

  expect(dependencies).to.deep.equal([
    getMyHash,
    getMyIds,
  ]);
});

This way we can test which dependencies are used. Then go and test our resultFunc seperately.

How do we accomplish this?

We can wrap reselect like so to expose those values.

import { createSelector } from 'reselect';

export const createWrappedSelector = function (...args) {
  const selOriginal = createSelector(...args);

  const sel = function (state, props) {
    return selOriginal(state, props);
  };

  sel.resultFunc = selOriginal.resultFunc;
  sel.args = args.slice(0, args.length - 1);
  return sel;
};

and use createWrappedSelector instead of createSelector.

const getMyValues = createWrappedSelector(
  getMyHash,
  getMyIds,
  lookUp,
);

[esbeto]

Hey, I know this has been closed. But does anybody have any way to stub the createSelector function with the memoization disabled?