Button router test

[ofhope]

Fixes a todo item in tests.

Changes:

Adding a test to check ButtonOrLink will render in a router and triggers a history change.

I think there could be an opportunity here to add a helper to test-utils perhaps renderWithRoutes or routesRender. Similar to reduxRender.

const subject = () => reduxRender(<Routing />, { store });

Side Note: I also tried installing userEvent a companion package from testing-library
https://testing-library.com/docs/ecosystem-user-event/

But npm run lint had an issue with it when I tried to included it in test-utils

I have verified that this pull request:

• has no linting errors (npm run lint)
• has no test errors (npm run test)
• is from a uniquely-named feature branch and is up to date with the develop branch.
• is descriptively named and links to an issue number, i.e. Fixes #123

[lindapaiste]

Thanks for your insights on this!

We just this week upgraded from react-router v3 to v5 and I’m still figuring out what that means for testing. I can tell that you started working on this before and then we merged in those changes and broke everything 🙃 — sorry!

As part of the upgrade I added a <Router> to the test-utils so that we wouldn’t get errors on <Link> elements for using them outside of a router context. But I’m not sure if the router in render is something that should be optional? Like maybe we only wrap in a <Router> if there is an option { router: true } or something like that. I’m also not sure if it makes more sense to use a MemoryRouter for tests or to use a BrowserRouter with the same history as the app itself. And it gets even more confusing when I look at upgrading to the current v6 with the data routers where the routes are part of the router itself.

As far as this particular error that you are getting, it’s due to the upgrades that we just merged in. The browserHistory is no longer exported by react-router. You need to import it from the browserHistory.js file in the repo.

With this current setup that we have in test-utils, I think your test should work without adding a <Router> in the test (because it’s already added by render) and with examining the browserHistory object that’s created in the browserHistory.js file (because that’s the same history instance that we use in the test router).

[ofhope]

Oh great it's being upgraded! And that Router was added to the renderer. This simplifies things a lot.

I swapped out browser history inside test-utils for createMemoryHistory. It looks like the browserHistory file is now unused.

Should I clean this up and remove the file? Or move the memory history into this file and perhaps rename to history.js

[lindapaiste]

Oh great it's being upgraded! And that Router was added to the renderer. This simplifies things a lot.

I swapped out browser history inside test-utils for createMemoryHistory. It looks like the browserHistory file is now unused.

Should I clean this up and remove the file? Or move the memory history into this file and perhaps rename to history.js

I think that what you have now is great.

You actually cannot name the file history.js, which I learned the hard way! We have a weird module resolution in the webpack config which makes it so that it will resolve imports from the history package to client/history if that file exists.

The browserHistory instance is used in a bunch of Redux actions (which I don't love, but that's a low priority). That's the reason that I used the browserHistory in the test-utils. But we do not actually test that redirect behavior anywhere so probably memoryHistory makes more sense. I mean it's pretty much designed for testing. If our app was perfectly structured then every redirect would access the current history from the router context which would be the memoryHistory during tests and the browserHistory otherwise.

If we ever do want to test the redirects which use the browserHistory directly then we could make the history an overridable option of the test render function. ie. render(<Component/>, { history: browserHistory }). But we'll cross that bridge if or when we come to it.