goatslacker / alt

Isomorphic flux implementation
http://alt.js.org/
3.45k stars 322 forks source link

Understanding unittesting of stores #680

Open weblogixx opened 8 years ago

weblogixx commented 8 years ago

Hi there, I got some problems related to testing alt stores. I am using the altTestingUtils form alt-utils. There, methods seem to return values in different ways when setting store data directly of via setState().

Take the following example:

// The example store:
import alt from './Dispatcher';
import Actions from './Actions';
import Source from './Source';

export class ExampleStore {
  constructor() {

    this.bindActions(Actions);
    this.registerAsync(Source);

    this.state = {
      x: false
    };
  }

  onSetXDirect() {
    this.state.x = true;
  }

  onSetXImmutable() {
    this.setState({ x: true });
  }
}

export default alt.createStore(ExampleStore, 'ExampleStore');

Now, take the following test (import paths changed for better readability):

import AltTestingUtils from 'alt-utils/lib/AltTestingUtils';
import alt from './Dispatcher';
import Actions from './Actions';
import WrappedStore, { ExampleStore } from './Store';

describe('ExampleStore', function () {

  let storeClass;
  beforeEach(function () {
    alt.recycle(WrappedStore);
    storeClass = AltTestingUtils.makeStoreTestable(alt, ExampleStore);
  });

  // This will work without any problems
  describe('when using onSetXDirectly', function () {
    expect(storeClass.state.x).to.be.false;
    storeClass. onSetXDirectly();
    expect(storeClass.state.x).to.be.true;
  });

  // This wont work, as storeClass.state is still the old one
  describe('when using onSetXImmutable', function () {
    expect(storeClass.state.x).to.be.false;
    storeClass. onSetXImmutable();
    expect(storeClass.state.x).to.be.true;
  });

  // This wont work either, thought it would just update the WrappedStore, but no...
  describe('when using onSetXImmutable', function () {
    expect(storeClass.state.x).to.be.false;
    storeClass. onSetXImmutable();
    expect(WrappedStore.getState().x).to.be.true;
  });

  // This will in fact work...
  describe('when using onSetXImmutable with action triggered', function () {
    expect(WrappedStore.getState().x).to.be.false;
    Actions.setXImmutable();
    expect(WrappedStore.getState().x).to.be.true;
  });
});

I would expect that when using the the wrapped store (parsed through AltTestingUtils.makeStoreTestable) it also should get the updated data after using setState().

Also I got another issue with makeStoreTestable. Imagine the following code:

class Example {

  static doRemote() {
    return new Promise((resolve) => {
      window.setTimeout(() => {
        resolve({ id: 5 });
      }, 500);
    });
  }

  doSomething() {
    this.getInstance().doRemote().then(data => {
      this.setState(data);
    });
  }
}

I do not think that this does any harm. I am using a custom doRemote method that can be statically called from within and out of the store. Imagine a sources remote method here that was bound via this.exportAsync() in the constructor of the store.

The code itself works really well. However, if I try to test with makeStoreTestable, it will tell me that doRemote is not accessible. This happens, because makeStoreTestable will exchange the getInstance()-method with an empty function, returning undefined. My current workaround for this is quite ugly:

  let storeClass;
  beforeEach(function () {
    alt.recycle(WrappedStore);
    storeClass = AltTestingUtils.makeStoreTestable(alt, ExampleStore);
    storeClass.getInstance = function () {
      return WrappedStore;
    };
  });

This will work, as it returns the original base class. However, I cannot think that this should be the recommended way to test such stuff.

Hope to get some input, would be really appreciated.

Thanks in advance, Chris