Remove duplication in action tests #139
Conversation
An attempt to remove duplication in action tests, open to opinions.
| @@ -6,6 +6,15 @@ import cfApi from '../../../util/cf_api.js'; | |||
| import serviceActions from '../../../actions/service_actions.js'; | |||
| import { serviceActionTypes } from '../../../constants.js'; | |||
|
|
|||
| function assertAction(spy, type, params) { | |||
| expect(spy).toHaveBeenCalledOnce(); | |||
| let arg = spy.getCall(0).args[0]; | |||
There was a problem hiding this comment.
change arg to actualOrgGuidArg or something that lets the reader know that the arg being assigned should the org.
There was a problem hiding this comment.
my only worry is the indices used for getCall and args i really doubt the order of the call or args will ever matter (since we are only expecting one call and the call will have just one arg for now for these org calls). but i'm just thinking how this will be used in case some other developer comes along trying to add tests.
There was a problem hiding this comment.
with that said. i'm fine with it (the index hard coding) for now, and we can worry about it later when that time comes.
| @@ -17,17 +18,40 @@ describe('serviceActions', function() { | |||
| sandbox.restore(); | |||
| }); | |||
|
|
|||
| function setupViewSpy() { | |||
| return sandbox.spy(AppDispatcher, 'handleViewAction'); | |||
There was a problem hiding this comment.
if you find yourself referring to the string 'handleViewAction' or 'handleServerAction' elsewhere in the code, I would make those constants. i'm not sure if you do use them more than here. i remember i did that in the angular stuff and wished i didn't. too many places to find and replace when refactoring. your call on this one.
I tried to allow the failure by putting it inthe the build matrix, but this failed so just doing this for now.
Remove duplication in action tests
An attempt to remove duplication in action tests, open to
opinions.
WIP, wanted feedback