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

Use real data provider for testing #4644

Closed
sebashwa opened this issue Apr 6, 2020 · 19 comments
Closed

Use real data provider for testing #4644

sebashwa opened this issue Apr 6, 2020 · 19 comments

Comments

@sebashwa
Copy link
Contributor

sebashwa commented Apr 6, 2020

Is your feature request related to a problem? Please describe.
I would like to enable my data provider for testing, so I can test my custom components and their side effects better (i.e. requests that are triggered). I'm using jest and react-testing-library and by default when I wrap the components I'd like to test using the <TestContext enableReducers> component, all data provider requests always respond with { data: null }.

Describe the solution you'd like
I think it would be ideal to be able to pass the real data provider to the TestContext component so it will be used by the test application.

Describe alternatives you've considered
I tried to rewrite the TestContext component and used my data provider. It never triggered any requests though and failed inside the react admin data reducer (for a create request inside the addOneRecord function, with newRecord set to null)

I'm not sure why my data provider wasn't triggering any requests and ended up inside the react admin data reducer before doing so. With some guidance why this happened, I would be happy to implement this feature if it is doable.

@Kmaschta
Copy link
Contributor

Kmaschta commented Apr 7, 2020

It's a good idea, feel free to open a PR, I would love to help you on this one :)

@fzaninotto
Copy link
Member

The <TestContext> doesn't deal with dataProvider. You can already write a test with a custom dataProvider by using DataProviderContext:

const TextApp = () => (
<DataProviderContext.Provider value={myDataProvider}>
    <TestContext>
        {({ store, history }) => {
            ...
        }}
    </TestContext>
</DataProviderContext.Provider>
)

See for instance Mutation.spec.js for an example of this in the react-admin codebase.

That being said, this piece of advice should be in the Unit Testing documentation. Marking this issue as Documentation.

@sebashwa
Copy link
Contributor Author

sebashwa commented Apr 7, 2020

Thanks for your input @fzaninotto I managed to write my own TestContext component today which basically does the same thing as you suggest. I forgot to provide the DataProviderContext yesterday, which is why it was not working:

I would have submitted a solution along those lines, but I agree that it is easy enough to wrap the TestContext itself to achieve the same thing:

// imports omitted

const fakeDataProvider = () => Promise.resolve({ data: null });

export default class TestContext extends Component {
    storeWithDefault = null;
    history = null;
    finalDataProvider = convertLegacyDataProvider(fakeDataProvider);

    constructor(props) {
      super(props);
      this.history = props.history || createMemoryHistory();
      const { initialState = {}, enableReducers = false, dataProvider = null, customReducers = {} } = props;

      if (dataProvider) {
        this.finalDataProvider = dataProvider instanceof Function
          ? convertLegacyDataProvider(dataProvider)
          : dataProvider;
      }

      this.storeWithDefault = enableReducers
        ? createAdminStore({
          initialState: merge({}, defaultStore, initialState),
          dataProvider: this.finalDataProvider,
          history: createMemoryHistory(),
          customReducers,
        })
        : createStore(() => merge({}, defaultStore, initialState));
    }

    renderChildren = () => {
      const { children } = this.props;
      return typeof children === "function"
        ? children({
          store: this.storeWithDefault,
          history: this.history,
        })
        : children;
    };

    render() {
      return (
        <Provider store={ this.storeWithDefault }>
          <Router history={ this.history }>
            <DataProviderContext.Provider value={ this.finalDataProvider }>
              { this.renderChildren() }
            </DataProviderContext.Provider>
          </Router>
        </Provider>
      );
    }
}

As you can see I need to use customReducers for my store though. Do you think it's feasible to implement that part inside TestContext?
Are you open to documentation submissions as well? I can imagine to write a passage or two about how to use the <DataProviderContext> in combination with <TestContext>.

@fzaninotto
Copy link
Member

I don't think we should update the TestContext, but we should add a section in the Unit Testing documentation to explain how to pass a dataProvider in tests using the context. And yes, we're open to PRs!

@sebashwa
Copy link
Contributor Author

sebashwa commented Apr 8, 2020

I agree that it is sufficient to add documentation about the usage of DataProviderContext.Provider.
But how should we proceed with custom reducers? Some of my tests fail because I am not able to add necessary initial state for my custom reducers. It is ignored with the following error message:

Unexpected key "someFooState" found in preloadedState argument passed to createStore. Expected to find one of the known reducer keys instead: "admin", "router". Unexpected keys will be ignored.

Should I open a new issue for that one?

@sebashwa
Copy link
Contributor Author

sebashwa commented Apr 8, 2020

On second thought: Why not use TestContext to add the data provider to the context. What's the descision making on what TestContext should offer? TestContext sounds like it offers everything I need for my unit tests, so why not include a dataProvider prop?
Shouldn't sound rude, just asking those questions myself atm. I think from a user perspective that's the more convenient API.

@djhi
Copy link
Collaborator

djhi commented Apr 8, 2020

Some of my tests fail because I am not able to add necessary initial state for my custom reducers.

I actually had the same issue recently. It is indeed a pain when you have custom reducers and we should do something about it.

I personally agree with you about the dataProvider in TestContext as well.

@fzaninotto
Copy link
Member

I'm afraid that adding a DataProviderContext (and i18n) inside TestContext is a breaking change - some existing tests will break.

@sebashwa
Copy link
Contributor Author

sebashwa commented Apr 9, 2020

I see, but how about wrapping the children passed to TestContext inside DataProviderContext.Provider only if the data provider is passed as a dataProvider prop?
I agree that everything should stay the same as before if no dataProvider prop is passed to not break anything.

Do you think passing custom reducers on to createAdminStore() has the potential to break existing tests as well?

@fzaninotto
Copy link
Member

It seems react-admin already offers several utilities for unit testing. TestContext (which should be named TestReduxContext), renderWithRedux, and CoreAdminContext. I'm afraid that by adding a dataProvider property to TextContext, we'll transform it into CoreAdminContext.

I'd like to know more about your testing needs, so that we can design better test utilities.

@ValentinH
Copy link
Contributor

I'm also interesting in writing integration tests for my components used in ReactAdmin. I'm using RTL and msw for this and I'm having a hard time setting up the required context to have everything working.

Here's what I tried so far:

const dataProviders = jsonServerProvider(window.location.origin)

// my msw server setup
const server = setupServer(
  rest.get('/categories', (req, res, ctx) => {
    if (req.url.searchParams.get('q')) {
      return res(
        ctx.set('X-Total-Count', '2'),
        ctx.json([
          { id: '1', name: 'test 1' },
          { id: '2', name: 'test 2' },
        ])
      )
    }
    return res(ctx.set('X-Total-Count', '0'), ctx.json([]))
  })
)

render(
  <CoreAdminContext dataProvider={dataProviders}>
    <MyBulkActionComponent selectedIds={['111', '222']} />
  </CoreAdminContext>
)

MyBulkActionComponent is basically using getList to populate an autocomplete field:

const { ids, data: categories, loading: isLoading } = useGetList<Category>(
    'categories',
    {
      page: 1,
      perPage: 20,
    },
    { field: 'name', order: 'ASC' },
    {
      q: debouncedSearchText,
    }
  )

From what I see, the jsonServerProvider correctly gets the data but the useGetList ids and data are always empty. It seems like the Redux state isn't updated. Am I missing something please?

@ValentinnDimitroff
Copy link
Contributor

Joining the club having to implement own version of the TestContext only because of the customReducers feature. Maybe some extension mode (boolean condition) allowing us to plug in would be quite helpful.

@fzaninotto Are there any future plans on this no matter if it is yes or no and what kind of more info is needed?

@fzaninotto
Copy link
Member

Adding support to customReducers in TestContext and renderWithRedux makes a lot of sense. Would you like to open a PR for that?

@ValentinnDimitroff
Copy link
Contributor

Sure - my pleasure!

@ValentinnDimitroff
Copy link
Contributor

While adding the customReducers option I noticed the ReadME of ra-test package lacks some notes for the renderWithRedux function. Would you like me to add a section about it with some examples?

@djhi
Copy link
Collaborator

djhi commented Mar 19, 2021

Yes, it would be really appreciated. Thanks!

@ValentinnDimitroff
Copy link
Contributor

Also done :)) Willing to fix any change request the you would might have

@ValentinnDimitroff
Copy link
Contributor

Shall we consider this one for closing after #6067 is merged?

@fzaninotto
Copy link
Member

Yes indeed, I'm closing this issue, thanks!

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

No branches or pull requests

6 participants