Skip to content

Conversation

@vera-liu
Copy link
Contributor

@vera-liu vera-liu commented Oct 3, 2016

Done:

  • component tests for default control panels
  • test for fetch datasource metadata endpoint
  • update tests for action creators

Todo:

  • tests for ajax call in action creators

help-needed @ascott

Copy link
Contributor Author

@vera-liu vera-liu Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having trouble figuring this one out. I looked at multiple docs for testing ajax action creators, sinon fakeServer, nock etc. I think http://redux.js.org/docs/recipes/WritingTests.html seems to make more sense. I kind of modified some of it, but this test keeps failing. Saying the actual callCount of dispatch is 1 instead of 5. I figure the dispatch callcount may not take callbacks into account. Still trying to figure this out.

@vera-liu vera-liu force-pushed the vliu_explore_tests branch from b915f78 to 93247a4 Compare October 4, 2016 00:13
@vera-liu vera-liu mentioned this pull request Oct 4, 2016
@vera-liu vera-liu force-pushed the vliu_explore_tests branch from 93247a4 to 6f5e265 Compare October 4, 2016 05:04
@vera-liu vera-liu changed the base branch from alanna-explore-v2-main to master October 4, 2016 18:52
@vera-liu vera-liu force-pushed the vliu_explore_tests branch 2 times, most recently from 5fa4124 to 78bee0d Compare October 4, 2016 19:10
Copy link

@ascott ascott Oct 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if we test that the expected actions were called and not how many times dispatch was called? it looks like that's what the example is doing:

nock('http://example.com/')
      .get('/todos')
      .reply(200, { body: { todos: ['do something'] }})

    const expectedActions = [
      { type: types.FETCH_TODOS_REQUEST },
      { type: types.FETCH_TODOS_SUCCESS, body: { todos: ['do something']  } }
    ]
    const store = mockStore({ todos: [] })

    return store.dispatch(actions.fetchTodos())
      .then(() => { // return of async actions
        expect(store.getActions()).toEqual(expectedActions)
      })
  })
const expectedActions = [
  { type: types.SET_TIME_COLUMN_OPTS },
  { type: types.SET_TIME_GRAIN_OPTS },
  {...},
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it but it says 'Cannot read property 'then' of undefined'. It seems that the ajax call never returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it but it didn't work, it says 'then' can't be called on undefined. I think somehow the ajax request never returned on the nock

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's an example spec that tests fetching from an API that the redux best practices doc references...
https://git.musta.ch/airbnb/airbnb/pull/44253/files#diff-1e402f9922b2b08b8ef90a58502d4c6bR56

maybe we can use that pattern?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asking in #frontend-goalie might be helpful too...

…pache#1205)

* create structure for new forked explore view (apache#1099)

* create structure for new forked explore view

* update component name

* add bootstrap data pattern

* remove console.log

* Associate version to entry files (apache#1060)

* Associate version to entry files

* Modified path joins for configs

* Made changes based on comments

* Created store and reducers (apache#1108)

* Created store and reducers

* Added spec

* Modifications based on comments

* Explore control panel components: Chart control, Time filter, SQL,
GroupBy and Filters

* Modifications based on comments
@vera-liu vera-liu force-pushed the vliu_explore_tests branch 2 times, most recently from b943eac to 2dfac09 Compare October 4, 2016 22:20
@vera-liu vera-liu force-pushed the vliu_explore_tests branch from 2dfac09 to 6f8827d Compare October 4, 2016 22:30
ACCESS_REQUEST_MISSING_ERR = __(
"The access requests seem to have been deleted")
USER_MISSING_ERR = __("The user seems to have been deleted")
DATASOURCE_ACCESS_ERR = __("User does not have access to this datasource")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry forgot to add this for last PR, adding it here

@mistercrunch
Copy link
Member

Build is failing, conflicts, imprecise commit message. I'm closing, you can re-open if needed when ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants