Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Enhancement for testAction helper method #939

Closed
kingstenbanh opened this issue Sep 10, 2017 · 7 comments
Closed

Proposal: Enhancement for testAction helper method #939

kingstenbanh opened this issue Sep 10, 2017 · 7 comments
Labels
documentation Improvements or additions to documentation

Comments

@kingstenbanh
Copy link

kingstenbanh commented Sep 10, 2017

What problem does this feature solve?

The current API for the testAction method helps reducing the need to stub out the action context (state, rootState, commit, etc). However, the recommended method assumes that each action will only trigger a state mutation (commit) and access previous data (state), which is not true in real-world and large-scale project. In my project, an action might need to dispatch another action, access the state of another module (rootState), or some getters. The current method doesn't give us the full access of the action context resulting in a lot of mocking in writing tests. I bet a lot of user out there are experiencing the same problem.

What does the proposed API look like?

The proposed API will create a similar signature with the way an action is created.

const actions = {
   [CREATE_USER](context, payload) {
      // do something here
   },
};

// original testAction
const testAction = (action, payload, state, expectedMutation, done) => {
  ...
};

// proposed testAction
const testAction = (action, context, payload, expectedMutations, done) => {
  ...
};

Here is the proposed testAction.

const testAction = (
    action,
    {
        state,
        rootState,
        getters,
    },
    payload,
    expectedMutations,
    done
) => {
    let count = 0;

    // mock commit
    const commit = (type, commitPayload) => {
        const mutation = expectedMutations[count];

        try {
            expect(mutation.type).to.equal(type);

            if (commitPayload) {
                expect(mutation.payload).to.deep.equal(commitPayload);
            }
        } catch (error) {
            done(error);
        }

        count  ;

        if (count >= expectedMutations.length) {
            done();
        }
    };

    // mock dispatch
    const dispatch = (type, dispatchPayload) => {
        const mutation = expectedMutations[count];

        try {
            expect(mutation.type).to.equal(type);

            if (dispatchPayload) {
                expect(mutation.payload).to.deep.equal(dispatchPayload);
            }
        } catch (error) {
            done(error);
        }

        count  ;

        if (count >= expectedMutations.length) {
            done();
        }
    };

    
    const context = {
        state,
        rootState,
        commit,
        dispatch,
        getters,
    };

    // call the action with mocked store and arguments
    action(context, payload);

    // check if no mutations should have been dispatched
    if (expectedMutations.length === 0) {
        expect(count).to.equal(0);
        done();
    }
};

export default testAction;
@kingstenbanh kingstenbanh changed the title Enhancement for testAction [Proposal] - Enhancement for testAction Sep 10, 2017
@kingstenbanh kingstenbanh changed the title [Proposal] - Enhancement for testAction Proposal: Enhancement for testAction helper method Sep 10, 2017
@ktsn ktsn added the documentation Improvements or additions to documentation label Sep 10, 2017
@bartlomieju
Copy link

Thumps up, I thinks this is a great idea! I've found myself in similar situation, thinking how to test actions that dispatch other actions. However, IMHO, I think dispatched actions shouldn't go to expectedMutations but rather dedicated expectedActions.

@egardner
Copy link

@kingstenbanh I was looking for the exact information you are proposing to add in the Vuex docs (I even posed in the Vuejs forum in hopes of finding a solution). Using your proposed testAction helper, I was able to adequately test the actions in my app that dispatch other actions.

Since the Vuex docs explicitly recommend composing actions from other actions using dispatch, I think that your example here would be a useful addition.

One other thing the docs recommend is returning Promises in actions since they can be asynchronous. Any ideas on how the a test for async Vuex actions would look?

Thanks for posting, I found this thread very useful.

@ghost
Copy link

ghost commented Mar 29, 2018

One other thing the docs recommend is returning Promises in actions since they can be asynchronous. Any ideas on how the a test for async Vuex actions would look?

@egardner You could try using spies:

const dispatch = sinon.spy();
const state = {
  // ...
};

actions.myAction({ dispatch, state }, someParameter)
  .then(() => {
    expect(dispatch.args).to.deep.equal([
      [ 'firstDispatchedAction' ],
      [ 'actionWithParameter', someParameter ],
    ]);
  });

@ghost
Copy link

ghost commented Mar 29, 2018

I have created a pull request to add spies as an alternative to the testAction helper: https://github.com/vuejs/vuex/pull/1209/files

@ghost
Copy link

ghost commented May 16, 2018

Enhancment that comes handy when want to make sure if no extra commits were made

let stubbedCommit = sinon.spy(commit);

action({ commit: stubbedCommit, state, ....etc }, ...args);
	
// assert exact number of commits in the action
expect(stubbedCommit.callCount).to.equal(expectedMutations.length);

// moved this one outside commit function so previous runs
if (count >= expectedMutations.length && done) {
	done();
}

@filipalacerda
Copy link

We've bumped into the same problem, we added support for actions like this: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/helpers/vuex_action_helper.js#L21

(Still needs some improvements, but it might be usefull for anyone else)

@kiaking
Copy link
Member

kiaking commented Apr 23, 2020

Closing due to inactivity. Docs have been updated few times, and if still need to discuss about this topic, we should open another issue referencing the latest docs.

@kiaking kiaking closed this as completed Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

6 participants